Skip to content

Commit

Permalink
db: system_distributed_keyspace: use current time when creating mutat…
Browse files Browse the repository at this point in the history
…ions in `start()`

When creating or updating internal distributed tables in
`system_distributed_keyspace::start()`, hardcoded timestamps were used.

There two reasons for this:
- to protect against issue scylladb#2129, where nodes would start without
  synchronizing schema with the existing cluster, creating the tables
  again, which would override any manual user changes to these tables.
  The solution was to use small timestamps (like api::min_timestamp) - the
  user-created schema mutations would always 'win' (because when they were
  created, they used current time).
- to eliminate unnecessary schema sync. If two nodes created these
  tables concurrently with different timestamps, the schemas would
  formally be different and would need to merge. This could happen
  during upgrades when we upgraded from a version which doesn't have
  these tables or doesn't have some columns.

The scylladb#2129 workaround is no longer necessary: when nodes start they always
have to sync schema with existing nodes; we also don't allow
bootstrapping nodes in parallel.

The second problem would happen during parallel bootstrap, which we
don't allow, or during parallel upgrade. The procedure we recommend is
rolling upgrade - where nodes are upgraded one by one. In this case only
one node is going to create/update the tables; following upgraded nodes
will sync schema first and notice they don't need to do anything. So if
procedures are followed correctly, the workaround is not needed. If
someone doesn't follow the procedures and upgrades nodes in parallel,
these additional schema synchronizations are not a big cost, so the
workaround doesn't give us much in this case as well.

When schema changes are performed by Raft group 0, certain constraints
are placed on the timestamps used for mutations. For this we'll need to
be able to use timestamps which are generated based on current time.
  • Loading branch information
kbr-scylla authored and syuu1228 committed Jan 26, 2022
1 parent 0ce8aa5 commit c61cb6b
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions db/system_distributed_keyspace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ static future<> add_new_columns_if_missing(replica::database& db, ::service::mig
if (updated) {
schema_ptr table = b.build();
try {
co_return co_await mm.announce(co_await mm.prepare_column_family_update_announcement(table, false, std::vector<view_ptr>(), api::timestamp_type(1)));
co_return co_await mm.announce(co_await mm.prepare_column_family_update_announcement(table, false, std::vector<view_ptr>(), api::new_timestamp()));
} catch (...) {}
}
} catch (...) {
Expand Down Expand Up @@ -285,7 +285,7 @@ future<> system_distributed_keyspace::start() {
auto m = co_await map_reduce(tables,
/* Mapper */ [this] (auto&& table) -> future<std::vector<mutation>> {
try {
co_return co_await _mm.prepare_new_column_family_announcement(std::move(table), api::min_timestamp);
co_return co_await _mm.prepare_new_column_family_announcement(std::move(table), api::new_timestamp());
} catch (exceptions::already_exists_exception&) {
co_return std::vector<mutation>();
}
Expand Down

0 comments on commit c61cb6b

Please sign in to comment.