Skip to content

Commit

Permalink
schema: add scylla specific options to schema description
Browse files Browse the repository at this point in the history 
Add `paxos_grace_seconds`, `tombstone_gc`, `cdc` and `synchronous_updates`
options to schema description.

Fixes: #12389
Fixes: scylladb/scylla-enterprise#2979

Closes #16786
  • Loading branch information
@Jadw1 @denesb
Jadw1 authored and denesb committed Jan 16, 2024
1 parent 7c4ec8c commit 29da20b
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 20 deletions.
27 changes: 25 additions & 2 deletions schema.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
#include "utils/rjson.hh"
#include "tombstone_gc_options.hh"
#include "db/per_partition_rate_limit_extension.hh"
#include "db/tags/utils.hh"
#include "db/tags/extension.hh"

constexpr int32_t schema::NAME_LENGTH;

Expand Down Expand Up @@ -764,6 +766,15 @@ static bool is_index(replica::database& db, const table_id& id, const schema& s)
return db.find_column_family(id).get_index_manager().is_index(s);
}

static bool is_update_synchronously_view(const schema& s) {
auto tag_opt = db::find_tag(s, db::SYNCHRONOUS_VIEW_UPDATES_TAG_KEY);
if (!tag_opt.has_value()) {
return false;
}

return *tag_opt == "true";
}

sstring schema::element_type(replica::database& db) const {
if (is_view()) {
if (is_index(db, view_info()->base_id(), *this)) {
Expand Down Expand Up @@ -907,8 +918,20 @@ std::ostream& schema::describe(replica::database& db, std::ostream& os, bool wit
os << "\n AND memtable_flush_period_in_ms = " << memtable_flush_period();
os << "\n AND min_index_interval = " << min_index_interval();
os << "\n AND read_repair_chance = " << read_repair_chance();
os << "\n AND speculative_retry = '" << speculative_retry().to_sstring() << "';";
os << "\n";
os << "\n AND speculative_retry = '" << speculative_retry().to_sstring() << "'";
os << "\n AND paxos_grace_seconds = " << paxos_grace_seconds().count();

auto tombstone_gc_str = tombstone_gc_options().to_sstring();
std::replace(tombstone_gc_str.begin(), tombstone_gc_str.end(), '"', '\'');
os << "\n AND tombstone_gc = " << tombstone_gc_str;

if (cdc_options().enabled()) {
os << "\n AND cdc = " << cdc_options().to_sstring();
}
if (is_view() && !is_index(db, view_info()->base_id(), *this)) {
os << "\n AND synchronous_updates = " << std::boolalpha << is_update_synchronously_view(*this);
}
os << ";\n";

if (with_internals) {
for (auto& cdef : dropped_columns()) {
Expand Down
53 changes: 44 additions & 9 deletions test/boost/cql_query_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@
#include "db/query_context.hh"
#include "service/qos/qos_common.hh"
#include "utils/UUID_gen.hh"
#include "tombstone_gc_extension.hh"
#include "db/tags/extension.hh"
#include "cdc/cdc_extension.hh"
#include "db/paxos_grace_seconds_extension.hh"
#include "db/per_partition_rate_limit_extension.hh"



using namespace std::literals::chrono_literals;

Expand Down Expand Up @@ -4128,6 +4135,18 @@ std::string normalize_white_space(const std::string& str) {
return std::regex_replace(std::regex_replace(" " + str + " ", std::regex("\\s+"), " "), std::regex(", "), ",");
}

cql_test_config describe_test_config() {
auto ext = std::make_shared<db::extensions>();
ext->add_schema_extension<db::tags_extension>(db::tags_extension::NAME);
ext->add_schema_extension<cdc::cdc_extension>(cdc::cdc_extension::NAME);
ext->add_schema_extension<db::paxos_grace_seconds_extension>(db::paxos_grace_seconds_extension::NAME);
ext->add_schema_extension<tombstone_gc_extension>(tombstone_gc_extension::NAME);
ext->add_schema_extension<db::per_partition_rate_limit_extension>(db::per_partition_rate_limit_extension::NAME);

auto cfg = seastar::make_shared<db::config>(ext);
return cql_test_config(cfg);
}

SEASTAR_TEST_CASE(test_describe_simple_schema) {
return do_with_cql_env_thread([] (cql_test_env& e) {
std::unordered_map<std::string, std::string> cql_create_tables {
Expand All @@ -4149,7 +4168,10 @@ SEASTAR_TEST_CASE(test_describe_simple_schema) {
" AND memtable_flush_period_in_ms = 0\n"
" AND min_index_interval = 128\n"
" AND read_repair_chance = 0\n"
" AND speculative_retry = '99.0PERCENTILE';\n"},
" AND speculative_retry = '99.0PERCENTILE'\n"
" AND paxos_grace_seconds = 43200\n"
" AND tombstone_gc = {'mode':'timeout','propagation_delay_in_seconds':'3600'};\n"
},
{"cf1", "CREATE TABLE ks.cf1 (\n"
" pk blob,\n"
" ck blob,\n"
Expand All @@ -4170,7 +4192,9 @@ SEASTAR_TEST_CASE(test_describe_simple_schema) {
" AND memtable_flush_period_in_ms = 0\n"
" AND min_index_interval = 128\n"
" AND read_repair_chance = 0\n"
" AND speculative_retry = '99.0PERCENTILE';\n"
" AND speculative_retry = '99.0PERCENTILE'\n"
" AND paxos_grace_seconds = 43200\n"
" AND tombstone_gc = {'mode':'timeout','propagation_delay_in_seconds':'3600'};\n"
},
{"CF2", "CREATE TABLE ks.\"CF2\" (\n"
" pk blob,\n"
Expand All @@ -4192,7 +4216,9 @@ SEASTAR_TEST_CASE(test_describe_simple_schema) {
" AND memtable_flush_period_in_ms = 10\n"
" AND min_index_interval = 128\n"
" AND read_repair_chance = 0\n"
" AND speculative_retry = '99.0PERCENTILE';\n"
" AND speculative_retry = '99.0PERCENTILE'\n"
" AND paxos_grace_seconds = 43200\n"
" AND tombstone_gc = {'mode':'timeout','propagation_delay_in_seconds':'3600'};\n"
},
{"Cf3", "CREATE TABLE ks.\"Cf3\" (\n"
" pk blob,\n"
Expand All @@ -4215,7 +4241,9 @@ SEASTAR_TEST_CASE(test_describe_simple_schema) {
" AND memtable_flush_period_in_ms = 10\n"
" AND min_index_interval = 128\n"
" AND read_repair_chance = 0\n"
" AND speculative_retry = '99.0PERCENTILE';\n"
" AND speculative_retry = '99.0PERCENTILE'\n"
" AND paxos_grace_seconds = 43200\n"
" AND tombstone_gc = {'mode':'timeout','propagation_delay_in_seconds':'3600'};\n"
},
{"cf4", "CREATE TABLE ks.cf4 (\n"
" pk blob,\n"
Expand All @@ -4237,7 +4265,9 @@ SEASTAR_TEST_CASE(test_describe_simple_schema) {
" AND memtable_flush_period_in_ms = 10\n"
" AND min_index_interval = 128\n"
" AND read_repair_chance = 0\n"
" AND speculative_retry = '99.0PERCENTILE';\n"
" AND speculative_retry = '99.0PERCENTILE'\n"
" AND paxos_grace_seconds = 43200\n"
" AND tombstone_gc = {'mode':'timeout','propagation_delay_in_seconds':'3600'};\n"
}

};
Expand All @@ -4253,7 +4283,7 @@ SEASTAR_TEST_CASE(test_describe_simple_schema) {
schema->describe(e.local_db(), ss, false);
BOOST_CHECK_EQUAL(normalize_white_space(ss.str()), normalize_white_space(ct.second));
}
});
}, describe_test_config());
}

SEASTAR_TEST_CASE(test_describe_view_schema) {
Expand All @@ -4280,7 +4310,9 @@ SEASTAR_TEST_CASE(test_describe_view_schema) {
" AND memtable_flush_period_in_ms = 0\n"
" AND min_index_interval = 128\n"
" AND read_repair_chance = 0\n"
" AND speculative_retry = '99.0PERCENTILE';\n";
" AND speculative_retry = '99.0PERCENTILE'\n"
" AND paxos_grace_seconds = 43200\n"
" AND tombstone_gc = {'mode':'timeout','propagation_delay_in_seconds':'3600'};\n";

std::unordered_map<std::string, std::string> cql_create_tables {
{"cf_view", "CREATE MATERIALIZED VIEW \"KS\".cf_view AS\n"
Expand All @@ -4302,7 +4334,10 @@ SEASTAR_TEST_CASE(test_describe_view_schema) {
" AND memtable_flush_period_in_ms = 0\n"
" AND min_index_interval = 128\n"
" AND read_repair_chance = 0\n"
" AND speculative_retry = '99.0PERCENTILE';\n"},
" AND speculative_retry = '99.0PERCENTILE'\n"
" AND paxos_grace_seconds = 43200\n"
" AND tombstone_gc = {'mode':'timeout','propagation_delay_in_seconds':'3600'}\n"
" AND synchronous_updates = false;\n"},
{"cf_index_index", "CREATE INDEX cf_index ON \"KS\".\"cF\"(col2);"},
{"cf_index1_index", "CREATE INDEX cf_index1 ON \"KS\".\"cF\"(pk);"},
{"cf_index2_index", "CREATE INDEX cf_index2 ON \"KS\".\"cF\"(pk1);"},
Expand All @@ -4326,7 +4361,7 @@ SEASTAR_TEST_CASE(test_describe_view_schema) {
base_schema->describe(e.local_db(), base_ss, false);
BOOST_CHECK_EQUAL(normalize_white_space(base_ss.str()), normalize_white_space(base_table));
}
});
}, describe_test_config());
}

shared_ptr<cql_transport::messages::result_message> cql_func_require_nofail(
Expand Down
36 changes: 27 additions & 9 deletions test/boost/multishard_mutation_query_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
#include "test/lib/log.hh"
#include "test/lib/test_utils.hh"
#include "test/lib/random_utils.hh"
#include "test/lib/random_schema.hh"
#include "tombstone_gc_extension.hh"
#include "db/tags/extension.hh"
#include "cdc/cdc_extension.hh"
#include "db/paxos_grace_seconds_extension.hh"
#include "db/per_partition_rate_limit_extension.hh"

#include <seastar/testing/thread_test_case.hh>

Expand All @@ -29,6 +35,18 @@

const sstring KEYSPACE_NAME = "multishard_mutation_query_test";

static cql_test_config cql_config_with_extensions() {
auto ext = std::make_shared<db::extensions>();
ext->add_schema_extension<db::tags_extension>(db::tags_extension::NAME);
ext->add_schema_extension<cdc::cdc_extension>(cdc::cdc_extension::NAME);
ext->add_schema_extension<db::paxos_grace_seconds_extension>(db::paxos_grace_seconds_extension::NAME);
ext->add_schema_extension<tombstone_gc_extension>(tombstone_gc_extension::NAME);
ext->add_schema_extension<db::per_partition_rate_limit_extension>(db::per_partition_rate_limit_extension::NAME);

auto cfg = seastar::make_shared<db::config>(ext);
return cql_test_config(cfg);
}

static uint64_t aggregate_querier_cache_stat(distributed<replica::database>& db, uint64_t query::querier_cache::stats::*stat) {
return map_reduce(boost::irange(0u, smp::count), [stat, &db] (unsigned shard) {
return db.invoke_on(shard, [stat] (replica::database& local_db) {
Expand Down Expand Up @@ -94,7 +112,7 @@ SEASTAR_THREAD_TEST_CASE(test_abandoned_read) {
require_eventually_empty_caches(env.db());

return make_ready_future<>();
}).get();
}, cql_config_with_extensions()).get();
}

static std::vector<mutation> read_all_partitions_one_by_one(distributed<replica::database>& db, schema_ptr s, std::vector<dht::decorated_key> pkeys,
Expand Down Expand Up @@ -430,7 +448,7 @@ SEASTAR_THREAD_TEST_CASE(test_read_all) {
check_results_are_equal(results1, results3);

return make_ready_future<>();
}).get();
}, cql_config_with_extensions()).get();
}

// Best run with SMP>=2
Expand Down Expand Up @@ -485,7 +503,7 @@ SEASTAR_THREAD_TEST_CASE(test_read_all_multi_range) {
require_eventually_empty_caches(env.db());

return make_ready_future<>();
}).get();
}, cql_config_with_extensions()).get();
}

// Best run with SMP>=2
Expand Down Expand Up @@ -531,7 +549,7 @@ SEASTAR_THREAD_TEST_CASE(test_read_with_partition_row_limits) {
} } }

return make_ready_future<>();
}).get();
}, cql_config_with_extensions()).get();
}

// Best run with SMP>=2
Expand Down Expand Up @@ -569,7 +587,7 @@ SEASTAR_THREAD_TEST_CASE(test_evict_a_shard_reader_on_each_page) {
require_eventually_empty_caches(env.db());

return make_ready_future<>();
}).get();
}, cql_config_with_extensions()).get();
}

// Best run with SMP>=2
Expand Down Expand Up @@ -624,7 +642,7 @@ SEASTAR_THREAD_TEST_CASE(test_read_reversed) {
require_eventually_empty_caches(env.db());

return make_ready_future<>();
}).get();
}, cql_config_with_extensions()).get();
}

namespace {
Expand Down Expand Up @@ -1268,8 +1286,8 @@ run_fuzzy_test_workload(fuzzy_test_config cfg, distributed<replica::database>& d
} // namespace

SEASTAR_THREAD_TEST_CASE(fuzzy_test) {
auto db_cfg = make_shared<db::config>();
db_cfg->enable_commitlog(false);
auto cql_cfg = cql_config_with_extensions();
cql_cfg.db_config->enable_commitlog(false);

do_with_cql_env_thread([] (cql_test_env& env) -> future<> {
// REPLACE RANDOM SEED HERE.
Expand Down Expand Up @@ -1305,5 +1323,5 @@ SEASTAR_THREAD_TEST_CASE(fuzzy_test) {
}).get();

return make_ready_future<>();
}, db_cfg).get();
}, cql_cfg).get();
}
3 changes: 3 additions & 0 deletions test/cql-pytest/test_describe.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,9 @@ def new_random_table(cql, keyspace, udts=[]):
extras["min_index_interval"] = min_idx_interval
extras["max_index_interval"] = random.randrange(min_idx_interval, 10000)
extras["memtable_flush_period_in_ms"] = random.randrange(0, 10000)
extras["paxos_grace_seconds"] = random.randrange(1000, 100000)

extras["tombstone_gc"] = f"{{'mode': 'timeout', 'propagation_delay_in_seconds': '{random.randrange(100, 100000)}'}}"

extra_options = [f"{k} = {v}" for (k, v) in extras.items()]
extra_str = " AND ".join(extra_options)
Expand Down

4 comments on commit 29da20b

@tzach
Copy link
Contributor

@tzach tzach commented on 29da20b Feb 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhalevy, how do we guarantee the next tablet property (tablets?) will be added?

@bhalevy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhalevy, how do we guarantee the next tablet property (tablets?) will be added?

Good question.

We can have a backup/restore test that uses the schema description for restore rather than restoring the schema sstables to make sure the description is accurate.

@mykaul
Copy link
Contributor

@mykaul mykaul commented on 29da20b Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhalevy, how do we guarantee the next tablet property (tablets?) will be added?

Good question.

We can have a backup/restore test that uses the schema description for restore rather than restoring the schema sstables to make sure the description is accurate.

This is the whole idea around the changes we are about to make - to move exactly to schema describe.

@bhalevy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coincidentally, we've just hit https://jenkins.scylladb.com/view/scylla-5.2/job/scylla-5.2/job/dtest-release/40/testReport/snapshot_test/TestSchemaFileInSnapshot/Run_Dtest_Parallel_Cloud_Machines___FullDtest___full_split000___test_restoring_by_schema_with_mv_use_sstableloader/
that failed on schema describe mismatch:

  -     AND speculative_retry = '99.0PERCENTILE'
  +     AND speculative_retry = '99.0PERCENTILE';
  ?                                             +
    
  - scylla_tags = {'system:synchronous_view_updates': 'false'};
  -

Please sign in to comment.