Skip to content

Commit

Permalink
cql3/alter_keyspace_statement: Do not allow for change of RF by more …
Browse files Browse the repository at this point in the history
…than 1

We want to ensure that when the replication factor
of a keyspace changes, it changes by at most 1 per DC
if it uses tablets. The rationale for that is to make
sure that the old and new quorums overlap by at least
one node.

After these changes, attempts to change the RF of
a keyspace in any DC by more than 1 will fail.
  • Loading branch information
dawmd authored and ptrsmrn committed May 7, 2024
1 parent 5210bc9 commit b2c026b
Showing 1 changed file with 27 additions and 3 deletions.
30 changes: 27 additions & 3 deletions cql3/statements/alter_keyspace_statement.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
*/

#include <boost/range/algorithm.hpp>
#include <fmt/format.h>
#include <seastar/core/coroutine.hh>
#include <stdexcept>
#include "alter_keyspace_statement.hh"
#include "prepared_statement.hh"
#include "service/migration_manager.hh"
Expand Down Expand Up @@ -40,10 +42,21 @@ future<> cql3::statements::alter_keyspace_statement::check_access(query_processo
return state.has_keyspace_access(_name, auth::permission::ALTER);
}

static bool validate_rf_difference(const std::string_view curr_rf, const std::string_view new_rf) {
auto to_number = [] (const std::string_view rf) {
int result;
// We assume the passed string view represents a valid decimal number,
// so we don't need the error code.
(void) std::from_chars(rf.begin(), rf.end(), result);
return result;
};

// We want to ensure that each DC's RF is going to change by at most 1
// because in that case the old and new quorums must overlap.
return std::abs(to_number(curr_rf) - to_number(new_rf)) <= 1;
}

void cql3::statements::alter_keyspace_statement::validate(query_processor& qp, const service::client_state& state) const {
// TODO: when processing a tablet-enabled keyspace,
// we must check if the new RF differs by at most 1 from the old RF,
// and fail the query if that's not the case
auto tmp = _name;
std::transform(tmp.begin(), tmp.end(), tmp.begin(), ::tolower);
if (is_system_keyspace(tmp)) {
Expand All @@ -57,6 +70,17 @@ void cql3::statements::alter_keyspace_statement::validate(query_processor& qp, c
}
try {
auto ks = qp.db().find_keyspace(_name);

if (ks.get_replication_strategy().uses_tablets()) {
const std::map<sstring, sstring>& current_rfs = ks.metadata()->strategy_options();
for (const auto& [new_dc, new_rf] : _attrs->get_replication_options()) {
auto it = current_rfs.find(new_dc);
if (it != current_rfs.end() && !validate_rf_difference(it->second, new_rf)) {
throw exceptions::invalid_request_exception("Cannot modify replication factor of any DC by more than 1 at a time.");
}
}
}

data_dictionary::storage_options current_options = ks.metadata()->get_storage_options();
data_dictionary::storage_options new_options = _attrs->get_storage_options();
if (!qp.proxy().features().keyspace_storage_options && !new_options.is_local_type()) {
Expand Down

0 comments on commit b2c026b

Please sign in to comment.