bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

features: assume MC_SSTABLE and UNBOUNDED_RANGE_TOMBSTONES are always enabled

Browse Source 
These features have been around for over 2 years and every reasonable
deployment should have them enabled.

The only case when those features could be not enabled is when the user
has used enable_sstables_mc_format config flag to disable MC sstable
format. This case has been eliminated by removing
enable_sstables_mc_format config flag.

Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>
This commit is contained in:
Piotr Jastrzebski
2021-03-26 10:22:46 +01:00
parent 3552e99ce7
commit 1bdcef6890
7 changed files with 5 additions and 155 deletions
Download Patch File Download Diff File Expand all files Collapse all files

97
cql3/statements/delete_statement.cc
View File

@@ -84,98 +84,6 @@ void delete_statement::add_update_for_key(mutation& m, const query::clustering_r
namespace raw {
namespace {
using namespace expr;
/// If oper.lhs is a single column, returns it; otherwise, returns null.
const column_definition* single_column(const binary_operator& oper) {
if (auto c = std::get_if<column_value>(oper.lhs.get())) {
return c->col;
}
return nullptr;
}
/// True iff expr bounds clustering key from both above and below OR it has no clustering-key bounds at all.
/// See #6493.
bool bounds_ck_symmetrically(const expression& expr) {
/// A visitor to find out if CK boundedness is symmetric.
class boundedness_tracker {
using boundedness_bitvector = int; // Combined using binary OR.
const boundedness_bitvector UPPER=1, LOWER=2;
/// Individual bounds collected from the visiting expression. May have a nullptr entry, which
/// represents multi-column bounds encountered.
std::unordered_map<const column_definition*, boundedness_bitvector> _found_bounds;
bool _shortcircuit = false; ///< When true, cease all further visiting and declare boundedness symmetric.
public:
/// True iff the nodes visited so far do bound the CK symmetrically.
bool result() const {
return _shortcircuit ||
// Since multi-column comparisons can't be mixed with single-column ones, _found_bounds will
// either have a single entry with key nullptr or one entry per restricted column.
boost::algorithm::all_of_equal(_found_bounds | boost::adaptors::map_values, UPPER | LOWER);
}
/// Updates state for a boolean expression.
void operator()(bool b) {
// b==true doesn't change the current state; b==false shortcircuits the entire expression to empty set.
if (!b) {
_shortcircuit = true;
}
}
/// Updates state for a binary-operator expression.
void operator()(const binary_operator& oper) {
if (std::holds_alternative<token>(*oper.lhs)) {
return;
}
// The rules of multi-column comparison imply that any multi-column expression sets a bound for the
// entire clustering key. Therefore, we represent any such expression with special pointer value
// nullptr.
auto col = single_column(oper);
if (col && !col->is_clustering_key()) {
return;
}
if (oper.op == oper_t::EQ) {
_found_bounds[col] = UPPER | LOWER;
} else if (oper.op == oper_t::LT || oper.op == oper_t::LTE) {
_found_bounds[col] |= UPPER;
} else if (oper.op == oper_t::GTE || oper.op == oper_t::GT) {
_found_bounds[col] |= LOWER;
}
}
/// Updates state for a conjunction.
void operator()(const conjunction& conj) {
for (const auto& child : conj.children) {
std::visit(*this, child);
if (_shortcircuit) {
break;
}
}
}
void operator()(const column_value&) {
throw std::logic_error("Column encountered outside binary_operator");
}
void operator()(const column_value_tuple&) {
throw std::logic_error("Column tuple encountered outside binary_operator");
}
void operator()(const token&) {
throw std::logic_error("Token function encountered outside binary_operator");
}
} tracker;
std::visit(tracker, expr);
return tracker.result();
}
} // anonymous namespace
::shared_ptr<cql3::statements::modification_statement>
delete_statement::prepare_internal(database& db, schema_ptr schema, variable_specifications& bound_names,
std::unique_ptr<attributes> attrs, cql_stats& stats) const {
@@ -200,11 +108,6 @@ delete_statement::prepare_internal(database& db, schema_ptr schema, variable_spe
}
prepare_conditions(db, *schema, bound_names, *stmt);
stmt->process_where_clause(db, _where_clause, bound_names);
if (!db.supports_infinite_bound_range_deletions() &&
!bounds_ck_symmetrically(stmt->restrictions().get_clustering_columns_restrictions()->expression)) {
throw exceptions::invalid_request_exception(
"A range deletion operation needs to specify both bounds for clusters without sstable mc format support");
}
if (has_slice(stmt->restrictions().get_clustering_columns_restrictions()->expression)) {
if (!schema->is_compound()) {
throw exceptions::invalid_request_exception("Range deletions on \"compact storage\" schemas are not supported");

5
database.cc
View File

@@ -371,11 +371,6 @@ database::database(const db::config& cfg, database_config dbcfg, service::migrat
_row_cache_tracker.set_compaction_scheduling_group(dbcfg.memory_compaction_scheduling_group);
_infinite_bound_range_deletions_reg = _feat.cluster_supports_unbounded_range_tombstones().when_enabled([this] {
dblog.debug("Enabling infinite bound range deletions");
_supports_infinite_bound_range_deletions = true;
});
setup_scylla_memory_diagnostics_producer();
}

5
db/sstables-format-selector.cc
View File

@@ -45,7 +45,6 @@ sstables_format_selector::sstables_format_selector(gms::gossiper& g, sharded<gms
: _gossiper(g)
, _features(f)
, _db(db)
, _mc_feature_listener(*this, sstables::sstable_version_types::mc)
, _md_feature_listener(*this, sstables::sstable_version_types::md)
{ }
@@ -75,7 +74,6 @@ future<> sstables_format_selector::do_maybe_select_format(sstables::sstable_vers
future<> sstables_format_selector::start() {
assert(this_shard_id() == 0);
return read_sstables_format().then([this] {
_features.local().cluster_supports_mc_sstable().when_enabled(_mc_feature_listener);
_features.local().cluster_supports_md_sstable().when_enabled(_md_feature_listener);
return make_ready_future<>();
});
@@ -100,9 +98,6 @@ future<> sstables_format_selector::select_format(sstables::sstable_version_types
_selected_format = format;
return _db.invoke_on_all([this] (database& db) {
db.set_format(_selected_format);
if (_selected_format >= sstables::sstable_version_types::mc) {
_features.local().support(gms::features::UNBOUNDED_RANGE_TOMBSTONES);
}
});
}

3
db/sstables-format-selector.hh
View File

@@ -61,10 +61,9 @@ class sstables_format_selector {
seastar::named_semaphore _sem = {1, named_semaphore_exception_factory{"feature listeners"}};
seastar::gate _sel;
feature_enabled_listener _mc_feature_listener;
feature_enabled_listener _md_feature_listener;
sstables::sstable_version_types _selected_format = sstables::sstable_version_types::la;
sstables::sstable_version_types _selected_format = sstables::sstable_version_types::mc;
future<> select_format(sstables::sstable_version_types new_format);
future<> read_sstables_format();

14
gms/feature_service.cc
View File

@@ -46,12 +46,12 @@ constexpr std::string_view features::STREAM_WITH_RPC_STREAM = "STREAM_WITH_RPC_S
constexpr std::string_view features::ROW_LEVEL_REPAIR = "ROW_LEVEL_REPAIR";
constexpr std::string_view features::TRUNCATION_TABLE = "TRUNCATION_TABLE";
constexpr std::string_view features::CORRECT_STATIC_COMPACT_IN_MC = "CORRECT_STATIC_COMPACT_IN_MC";
constexpr std::string_view features::UNBOUNDED_RANGE_TOMBSTONES = "UNBOUNDED_RANGE_TOMBSTONES";
constexpr std::string_view features::MC_SSTABLE = "MC_SSTABLE_FORMAT";
// Up-to-date features
constexpr std::string_view features::UDF = "UDF";
constexpr std::string_view features::MC_SSTABLE = "MC_SSTABLE_FORMAT";
constexpr std::string_view features::MD_SSTABLE = "MD_SSTABLE_FORMAT";
constexpr std::string_view features::UNBOUNDED_RANGE_TOMBSTONES = "UNBOUNDED_RANGE_TOMBSTONES";
constexpr std::string_view features::VIEW_VIRTUAL_COLUMNS = "VIEW_VIRTUAL_COLUMNS";
constexpr std::string_view features::DIGEST_INSENSITIVE_TO_EXPIRY = "DIGEST_INSENSITIVE_TO_EXPIRY";
constexpr std::string_view features::COMPUTED_COLUMNS = "COMPUTED_COLUMNS";
@@ -74,9 +74,7 @@ feature_config::feature_config() {
feature_service::feature_service(feature_config cfg) : _config(cfg)
, _udf_feature(*this, features::UDF)
, _mc_sstable_feature(*this, features::MC_SSTABLE)
, _md_sstable_feature(*this, features::MD_SSTABLE)
, _unbounded_range_tombstones_feature(*this, features::UNBOUNDED_RANGE_TOMBSTONES)
, _view_virtual_columns(*this, features::VIEW_VIRTUAL_COLUMNS)
, _digest_insensitive_to_expiry(*this, features::DIGEST_INSENSITIVE_TO_EXPIRY)
, _computed_columns(*this, features::COMPUTED_COLUMNS)
@@ -96,8 +94,6 @@ feature_service::feature_service(feature_config cfg) : _config(cfg)
feature_config feature_config_from_db_config(db::config& cfg, std::set<sstring> disabled) {
feature_config fcfg;
fcfg._masked_features.insert(sstring(gms::features::UNBOUNDED_RANGE_TOMBSTONES));
fcfg._disabled_features = std::move(disabled);
if (!cfg.enable_sstables_md_format()) {
@@ -168,18 +164,18 @@ std::set<std::string_view> feature_service::known_feature_set() {
gms::features::ROW_LEVEL_REPAIR,
gms::features::TRUNCATION_TABLE,
gms::features::CORRECT_STATIC_COMPACT_IN_MC,
gms::features::UNBOUNDED_RANGE_TOMBSTONES,
gms::features::MC_SSTABLE,
// Up-to-date features
gms::features::VIEW_VIRTUAL_COLUMNS,
gms::features::DIGEST_INSENSITIVE_TO_EXPIRY,
gms::features::COMPUTED_COLUMNS,
gms::features::NONFROZEN_UDTS,
gms::features::UNBOUNDED_RANGE_TOMBSTONES,
gms::features::HINTED_HANDOFF_SEPARATE_CONNECTION,
gms::features::PER_TABLE_PARTITIONERS,
gms::features::PER_TABLE_CACHING,
gms::features::LWT,
gms::features::MC_SSTABLE,
gms::features::MD_SSTABLE,
gms::features::UDF,
gms::features::CDC,
@@ -251,9 +247,7 @@ db::schema_features feature_service::cluster_schema_features() const {
void feature_service::enable(const std::set<std::string_view>& list) {
for (gms::feature& f : {
std::ref(_udf_feature),
std::ref(_mc_sstable_feature),
std::ref(_md_sstable_feature),
std::ref(_unbounded_range_tombstones_feature),
std::ref(_view_virtual_columns),
std::ref(_digest_insensitive_to_expiry),
std::ref(_computed_columns),

10
gms/feature_service.hh
View File

@@ -76,9 +76,7 @@ public:
private:
gms::feature _udf_feature;
gms::feature _mc_sstable_feature;
gms::feature _md_sstable_feature;
gms::feature _unbounded_range_tombstones_feature;
gms::feature _view_virtual_columns;
gms::feature _digest_insensitive_to_expiry;
gms::feature _computed_columns;
@@ -99,10 +97,6 @@ public:
return bool(_udf_feature);
}
const feature& cluster_supports_mc_sstable() const {
return _mc_sstable_feature;
}
const feature& cluster_supports_md_sstable() const {
return _md_sstable_feature;
}
@@ -123,10 +117,6 @@ public:
return _digest_for_null_values_feature;
}
const feature& cluster_supports_unbounded_range_tombstones() const {
return _unbounded_range_tombstones_feature;
}
const feature& cluster_supports_view_virtual_columns() const {
return _view_virtual_columns;
}

26
test/boost/cql_query_test.cc
View File

@@ -1119,32 +1119,6 @@ SEASTAR_TEST_CASE(test_range_deletion_scenarios_with_compact_storage) {
});
}
SEASTAR_TEST_CASE(test_invalid_range_deletion) {
cql_test_config cfg;
cfg.disabled_features.insert(sstring(gms::features::UNBOUNDED_RANGE_TOMBSTONES));
return do_with_cql_env_thread([] (cql_test_env& e) {
cquery_nofail(e, "create table cf (p int, c1 int, c2 int, c3 int, primary key (p, c1, c2, c3));");
const auto q = [&] (const char* stmt) { return e.execute_cql(stmt).get(); };
using ire = exceptions::invalid_request_exception;
const auto expected = exception_predicate::message_contains("specify both bounds");
cquery_nofail(e, "delete from cf where p = 1");
cquery_nofail(e, "delete from cf where p = 1 and c1 = 2");
BOOST_REQUIRE_EXCEPTION(q("delete from cf where p = 1 and c1 < 2"), ire, expected);
BOOST_REQUIRE_EXCEPTION(q("delete from cf where p = 1 and c1 >= 0"), ire, expected);
cquery_nofail(e, "delete from cf where p = 1 and c1 < 2 and c1 >= 0");
// TODO: enable when supported:
// cquery_nofail(e, "delete from cf where p = 1 and c1 = 2 and c1 < 2");
BOOST_REQUIRE_EXCEPTION(q("delete from cf where p = 1 and c1 = 2 and c2 >= 0"), ire, expected);
cquery_nofail(e, "delete from cf where p = 1 and c1 = 2 and c2 >= 0 and c2 < 2");
BOOST_REQUIRE_EXCEPTION(q("delete from cf where p = 1 and (c1)>(0)"), ire, expected);
BOOST_REQUIRE_EXCEPTION(q("delete from cf where p = 1 and (c1,c2)>(0,0)"), ire, expected);
BOOST_REQUIRE_EXCEPTION(q("delete from cf where p = 1 and (c1,c2,c3)<=(5,5,5)"), ire, expected);
cquery_nofail(e, "delete from cf where p = 1 and (c1,c2)>(0,0) and (c1,c2)<=(5,5)");
cquery_nofail(e, "delete from cf where p = 1 and (c1,c2)>(0,0) and (c1)<=(5)");
cquery_nofail(e, "delete from cf where p = 1 and (c1,c2)=(1,1)");
}, cfg);
}
SEASTAR_TEST_CASE(test_map_insert_update) {
return do_with_cql_env([] (cql_test_env& e) {
auto make_my_map_type = [] { return map_type_impl::get_instance(int32_type, int32_type, true); };