Skip to content

Commit

Permalink
migration_manager: do not allow creating keyspace with arbitrary time…
Browse files Browse the repository at this point in the history
…stamp

This was needed to fix issue scylladb#2129 which was only manifest itself with
auto_bootstrap set to false. The option is ignored now and we always
wait for schema to synch during boot.
  • Loading branch information
Gleb Natapov committed Dec 30, 2021
1 parent 85f5277 commit 2054082
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 23 deletions.
2 changes: 1 addition & 1 deletion alternator/executor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4050,7 +4050,7 @@ future<> executor::create_keyspace(std::string_view keyspace_name) {
}
auto opts = get_network_topology_options(_gossiper, rf);
auto ksm = keyspace_metadata::new_keyspace(keyspace_name_str, "org.apache.cassandra.locator.NetworkTopologyStrategy", std::move(opts), true);
return _mm.announce_new_keyspace(ksm, api::new_timestamp());
return _mm.announce_new_keyspace(ksm);
});
}

Expand Down
4 changes: 1 addition & 3 deletions auth/service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,7 @@ future<> service::create_keyspace_if_missing(::service::migration_manager& mm) c
opts,
true);

// We use min_timestamp so that default keyspace metadata will loose with any manual adjustments.
// See issue #2129.
return mm.announce_new_keyspace(ksm, api::min_timestamp);
return mm.announce_new_keyspace(ksm);
}

return make_ready_future<>();
Expand Down
2 changes: 1 addition & 1 deletion cql3/statements/create_keyspace_statement.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ future<std::pair<::shared_ptr<cql_transport::event::schema_change>, std::vector<
std::vector<mutation> m;

try {
m = qp.get_migration_manager().prepare_new_keyspace_announcement(_attrs->as_ks_metadata(_name, tm), api::new_timestamp());
m = qp.get_migration_manager().prepare_new_keyspace_announcement(_attrs->as_ks_metadata(_name, tm));

ret = ::make_shared<event::schema_change>(
event::schema_change::change_type::CREATED,
Expand Down
6 changes: 2 additions & 4 deletions db/system_distributed_keyspace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -239,15 +239,13 @@ future<> system_distributed_keyspace::start() {
return futurize_invoke(std::move(func)).handle_exception_type([] (exceptions::already_exists_exception& ignored) { });
};

// We use min_timestamp so that the default keyspace metadata will lose with any manual adjustments.
// See issue #2129.
co_await ignore_existing([this] {
auto ksm = keyspace_metadata::new_keyspace(
NAME,
"org.apache.cassandra.locator.SimpleStrategy",
{{"replication_factor", "3"}},
true /* durable_writes */);
return _mm.announce_new_keyspace(ksm, api::min_timestamp);
return _mm.announce_new_keyspace(ksm);
});

co_await ignore_existing([this] {
Expand All @@ -256,7 +254,7 @@ future<> system_distributed_keyspace::start() {
"org.apache.cassandra.locator.EverywhereStrategy",
{},
true /* durable_writes */);
return _mm.announce_new_keyspace(ksm, api::min_timestamp);
return _mm.announce_new_keyspace(ksm);
});

for (auto&& table : ensured_tables()) {
Expand Down
13 changes: 4 additions & 9 deletions service/migration_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -633,22 +633,17 @@ future<> migration_manager::announce_keyspace_update(lw_shared_ptr<keyspace_meta
return announce(prepare_keyspace_update_announcement(std::move(ksm)));
}

future<>migration_manager::announce_new_keyspace(lw_shared_ptr<keyspace_metadata> ksm)
{
return announce_new_keyspace(ksm, api::new_timestamp());
}

std::vector<mutation> migration_manager::prepare_new_keyspace_announcement(lw_shared_ptr<keyspace_metadata> ksm, api::timestamp_type timestamp) {
std::vector<mutation> migration_manager::prepare_new_keyspace_announcement(lw_shared_ptr<keyspace_metadata> ksm) {
auto& proxy = _storage_proxy;
auto& db = proxy.get_db().local();

db.validate_new_keyspace(*ksm);
mlogger.info("Create new Keyspace: {}", ksm);
return db::schema_tables::make_create_keyspace_mutations(ksm, timestamp);
return db::schema_tables::make_create_keyspace_mutations(ksm, api::new_timestamp());
}

future<> migration_manager::announce_new_keyspace(lw_shared_ptr<keyspace_metadata> ksm, api::timestamp_type timestamp) {
return announce(prepare_new_keyspace_announcement(std::move(ksm), timestamp));
future<> migration_manager::announce_new_keyspace(lw_shared_ptr<keyspace_metadata> ksm) {
return announce(prepare_new_keyspace_announcement(std::move(ksm)));
}

future<> migration_manager::announce_new_column_family(schema_ptr cfm)
Expand Down
4 changes: 1 addition & 3 deletions service/migration_manager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,7 @@ public:

future<> announce_new_keyspace(lw_shared_ptr<keyspace_metadata> ksm);

future<> announce_new_keyspace(lw_shared_ptr<keyspace_metadata> ksm, api::timestamp_type timestamp);

std::vector<mutation> prepare_new_keyspace_announcement(lw_shared_ptr<keyspace_metadata> ksm, api::timestamp_type timestamp);
std::vector<mutation> prepare_new_keyspace_announcement(lw_shared_ptr<keyspace_metadata> ksm);


// The timestamp parameter can be used to ensure that all nodes update their internal tables' schemas
Expand Down
3 changes: 1 addition & 2 deletions table_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@ future<> table_helper::setup_keyspace(cql3::query_processor& qp, const sstring&
std::map<sstring, sstring> opts;
opts["replication_factor"] = replication_factor;
auto ksm = keyspace_metadata::new_keyspace(keyspace_name, "org.apache.cassandra.locator.SimpleStrategy", std::move(opts), true);
// We use min_timestamp so that default keyspace metadata will loose with any manual adjustments. See issue #2129.
qp.get_migration_manager().announce_new_keyspace(ksm, api::min_timestamp).get();
qp.get_migration_manager().announce_new_keyspace(ksm).get();
}

qs.get_client_state().set_keyspace(db.real_database(), keyspace_name);
Expand Down

0 comments on commit 2054082

Please sign in to comment.