From 50095cc3a541f226d8a3cd190d64548dd89ac9df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Botond=20D=C3=A9nes?= Date: Fri, 21 Apr 2023 14:08:56 +0300 Subject: [PATCH] Merge 'db: system_keyspace: use microsecond resolution for group0_history range tombstone' from Kamil Braun in `make_group0_history_state_id_mutation`, when adding a new entry to the group 0 history table, if the parameter `gc_older_than` is engaged, we create a range tombstone in the mutation which deletes entries older than the new one by `gc_older_than`. In particular if `gc_older_than = 0`, we want to delete all older entries. There was a subtle bug there: we were using millisecond resolution when generating the tombstone, while the provided state IDs used microsecond resolution. On a super fast machine it could happen that we managed to perform two schema changes in a single millisecond; this happened sometimes in `group0_test.test_group0_history_clearing_old_entries` on our new CI/promotion machines, causing the test to fail because the tombstone didn't clear the entry correspodning to the previous schema change when performing the next schema change (since they happened in the same millisecond). Use microsecond resolution to fix that. The consecutive state IDs used in group 0 mutations are guaranteed to be strictly monotonic at microsecond resolution (see `generate_group0_state_id` in service/raft/raft_group0_client.cc). Fixes #13594 Closes #13604 * github.com:scylladb/scylladb: db: system_keyspace: use microsecond resolution for group0_history range tombstone utils: UUID_gen: accept decimicroseconds in min_time_UUID (cherry picked from commit 10c1f1dc80afec530dd24bd11ca74cfbe0eb0f99) --- db/system_keyspace.cc | 8 ++++---- utils/UUID_gen.hh | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/db/system_keyspace.cc b/db/system_keyspace.cc index 42f99ac77f..ad61fde923 100644 --- a/db/system_keyspace.cc +++ b/db/system_keyspace.cc @@ -3347,11 +3347,11 @@ mutation system_keyspace::make_group0_history_state_id_mutation( using namespace std::chrono; assert(*gc_older_than >= gc_clock::duration{0}); - auto ts_millis = duration_cast(microseconds{ts}); - auto gc_older_than_millis = duration_cast(*gc_older_than); - assert(gc_older_than_millis < ts_millis); + auto ts_micros = microseconds{ts}; + auto gc_older_than_micros = duration_cast(*gc_older_than); + assert(gc_older_than_micros < ts_micros); - auto tomb_upper_bound = utils::UUID_gen::min_time_UUID(ts_millis - gc_older_than_millis); + auto tomb_upper_bound = utils::UUID_gen::min_time_UUID(ts_micros - gc_older_than_micros); // We want to delete all entries with IDs smaller than `tomb_upper_bound` // but the deleted range is of the form (x, +inf) since the schema is reversed. auto range = query::clustering_range::make_starting_with({ diff --git a/utils/UUID_gen.hh b/utils/UUID_gen.hh index d2a69d1e6f..9f032e7620 100644 --- a/utils/UUID_gen.hh +++ b/utils/UUID_gen.hh @@ -289,7 +289,7 @@ public: * Warning: this method should only be used for querying as this * doesn't at all guarantee the uniqueness of the resulting UUID. */ - static UUID min_time_UUID(milliseconds timestamp = milliseconds{0}) + static UUID min_time_UUID(decimicroseconds timestamp = decimicroseconds{0}) { auto uuid = UUID(create_time(from_unix_timestamp(timestamp)), MIN_CLOCK_SEQ_AND_NODE); assert(uuid.is_timestamp());