[QUEUED scylladb next] Merge 'build: update C++ standard to C++23' from Avi Kivity

0 views
Skip to first unread message

Commit Bot

<bot@cloudius-systems.com>
unread,
Jun 28, 2024, 11:03:18 AMJun 28
to scylladb-dev@googlegroups.com, Pavel Emelyanov
From: Pavel Emelyanov <xe...@scylladb.com>
Committer: Pavel Emelyanov <xe...@scylladb.com>
Branch: next

Merge 'build: update C++ standard to C++23' from Avi Kivity

Switch the C++ standard from C++20 to C++23. This is straightforward, but there are a few
fallouts (mostly due to std::unique_ptr that became constexpr) that need to be fixed first.

Internal enhancement - no backport required

Closes scylladb/scylladb#19528

* github.com:scylladb/scylladb:
build: switch to C++23
config: avoid binding an lvalue reference to an rvalue reference
readers: define query::partition_slice before using it in default argument
test: define table_for_tests earlier
compaction: define compaction_group::table_state earlier
compaction: compaction_group: define destructor out-of-line
compaction_manager: define compaction_manager::strategy_control earlier

---
diff --git a/CMakeLists.txt b/CMakeLists.txt
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -44,7 +44,7 @@ endif()

include(limit_jobs)
# Configure Seastar compile options to align with Scylla
-set(CMAKE_CXX_STANDARD "20" CACHE INTERNAL "")
+set(CMAKE_CXX_STANDARD "23" CACHE INTERNAL "")
set(CMAKE_CXX_EXTENSIONS ON CACHE INTERNAL "")
set(CMAKE_CXX_SCAN_FOR_MODULES OFF CACHE INTERNAL "")
set(CMAKE_CXX_VISIBILITY_PRESET hidden)
diff --git a/compaction/compaction_manager.cc b/compaction/compaction_manager.cc
--- a/compaction/compaction_manager.cc
+++ b/compaction/compaction_manager.cc
@@ -871,6 +871,28 @@ sstables::compaction_stopped_exception compaction_task_executor::make_compaction
return sstables::compaction_stopped_exception(s->ks_name(), s->cf_name(), _compaction_data.stop_requested);
}

+class compaction_manager::strategy_control : public compaction::strategy_control {
+ compaction_manager& _cm;
+public:
+ explicit strategy_control(compaction_manager& cm) noexcept : _cm(cm) {}
+
+ bool has_ongoing_compaction(table_state& table_s) const noexcept override {
+ return std::any_of(_cm._tasks.begin(), _cm._tasks.end(), [&s = table_s.schema()] (const shared_ptr<compaction_task_executor>& task) {
+ return task->compaction_running()
+ && task->compacting_table()->schema()->ks_name() == s->ks_name()
+ && task->compacting_table()->schema()->cf_name() == s->cf_name();
+ });
+ }
+
+ std::vector<sstables::shared_sstable> candidates(table_state& t) const override {
+ return _cm.get_candidates(t, *t.main_sstable_set().all());
+ }
+
+ std::vector<sstables::frozen_sstable_run> candidates_as_runs(table_state& t) const override {
+ return _cm.get_candidates(t, t.main_sstable_set().all_sstable_runs());
+ }
+};
+
compaction_manager::compaction_manager(config cfg, abort_source& as, tasks::task_manager& tm)
: _task_manager_module(make_shared<task_manager_module>(tm))
, _cfg(std::move(cfg))
@@ -2160,28 +2182,6 @@ void compaction_manager::propagate_replacement(table_state& t,
}
}

-class compaction_manager::strategy_control : public compaction::strategy_control {
- compaction_manager& _cm;
-public:
- explicit strategy_control(compaction_manager& cm) noexcept : _cm(cm) {}
-
- bool has_ongoing_compaction(table_state& table_s) const noexcept override {
- return std::any_of(_cm._tasks.begin(), _cm._tasks.end(), [&s = table_s.schema()] (const shared_ptr<compaction_task_executor>& task) {
- return task->compaction_running()
- && task->compacting_table()->schema()->ks_name() == s->ks_name()
- && task->compacting_table()->schema()->cf_name() == s->cf_name();
- });
- }
-
- std::vector<sstables::shared_sstable> candidates(table_state& t) const override {
- return _cm.get_candidates(t, *t.main_sstable_set().all());
- }
-
- std::vector<sstables::frozen_sstable_run> candidates_as_runs(table_state& t) const override {
- return _cm.get_candidates(t, t.main_sstable_set().all_sstable_runs());
- }
-};
-
strategy_control& compaction_manager::get_strategy_control() const noexcept {
return *_strategy_control;
}
diff --git a/configure.py b/configure.py
--- a/configure.py
+++ b/configure.py
@@ -1684,10 +1684,10 @@ def configure_seastar(build_dir, mode, mode_config):
'-DCMAKE_C_COMPILER={}'.format(args.cc),
'-DCMAKE_CXX_COMPILER={}'.format(args.cxx),
'-DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON',
- '-DCMAKE_CXX_STANDARD=20',
+ '-DCMAKE_CXX_STANDARD=23',
'-DSeastar_CXX_FLAGS=SHELL:{}'.format(mode_config['lib_cflags']),
'-DSeastar_LD_FLAGS={}'.format(semicolon_separated(mode_config['lib_ldflags'], seastar_cxx_ld_flags)),
- '-DSeastar_CXX_DIALECT=gnu++20',
+ '-DSeastar_CXX_DIALECT=gnu++23',
'-DSeastar_API_LEVEL=7',
'-DSeastar_DEPRECATED_OSTREAM_FORMATTERS=OFF',
'-DSeastar_UNUSED_RESULT_ERROR=ON',
@@ -1750,7 +1750,7 @@ def configure_abseil(build_dir, mode, mode_config):
'-DCMAKE_CXX_COMPILER={}'.format(args.cxx),
'-DCMAKE_CXX_FLAGS_{}={}'.format(cmake_mode.upper(), cxx_flags),
'-DCMAKE_EXPORT_COMPILE_COMMANDS=ON',
- '-DCMAKE_CXX_STANDARD=20',
+ '-DCMAKE_CXX_STANDARD=23',
'-DABSL_PROPAGATE_CXX_STD=ON',
]

@@ -1885,7 +1885,7 @@ def write_build_file(f,
configure_args = {configure_args}
builddir = {outdir}
cxx = {cxx}
- cxxflags = -std=gnu++20 {user_cflags} {warnings} {defines}
+ cxxflags = -std=gnu++23 {user_cflags} {warnings} {defines}
ldflags = {linker_flags} {user_ldflags}
ldflags_build = {linker_flags}
libs = {libs}
diff --git a/main.cc b/main.cc
--- a/main.cc
+++ b/main.cc
@@ -648,7 +648,8 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl
init("list-tools", bpo::bool_switch(), "list included tools and exit");

bpo::options_description deprecated_options("Deprecated options - ignored");
- cfg->add_deprecated_options(deprecated_options.add_options());
+ auto deprecated_options_easy_init = deprecated_options.add_options();
+ cfg->add_deprecated_options(deprecated_options_easy_init);
app.get_options_description().add(deprecated_options);

// TODO : default, always read?
diff --git a/readers/reversing_v2.hh b/readers/reversing_v2.hh
--- a/readers/reversing_v2.hh
+++ b/readers/reversing_v2.hh
@@ -8,12 +8,12 @@

#pragma once
#include <memory>
+#include "query-request.hh"

class mutation_reader;

namespace query {
struct max_result_size;
- class partition_slice;
}


diff --git a/replica/compaction_group.hh b/replica/compaction_group.hh
--- a/replica/compaction_group.hh
+++ b/replica/compaction_group.hh
@@ -85,6 +85,7 @@ public:
boost::intrusive::constant_time_size<false>>;

compaction_group(table& t, size_t gid, dht::token_range token_range);
+ ~compaction_group();

void update_id_and_range(size_t id, dht::token_range token_range) {
_group_id = id;
diff --git a/replica/table.cc b/replica/table.cc
--- a/replica/table.cc
+++ b/replica/table.cc
@@ -1992,6 +1992,96 @@ table::make_memtable_list(compaction_group& cg) {
return make_lw_shared<memtable_list>(std::move(seal), std::move(get_schema), _config.dirty_memory_manager, _memtable_shared_data, _stats, _config.memory_compaction_scheduling_group);
}

+class compaction_group::table_state : public compaction::table_state {
+ table& _t;
+ compaction_group& _cg;
+public:
+ explicit table_state(table& t, compaction_group& cg) : _t(t), _cg(cg) {}
+
+ const schema_ptr& schema() const noexcept override {
+ return _t.schema();
+ }
+ unsigned min_compaction_threshold() const noexcept override {
+ // During receiving stream operations, the less we compact the faster streaming is. For
+ // bootstrap and replace thereThere are no readers so it is fine to be less aggressive with
+ // compactions as long as we don't ignore them completely (this could create a problem for
+ // when streaming ends)
+ if (_t._is_bootstrap_or_replace) {
+ auto target = std::min(_t.schema()->max_compaction_threshold(), 16);
+ return std::max(_t.schema()->min_compaction_threshold(), target);
+ } else {
+ return _t.schema()->min_compaction_threshold();
+ }
+ }
+ bool compaction_enforce_min_threshold() const noexcept override {
+ return _t.get_config().compaction_enforce_min_threshold || _t._is_bootstrap_or_replace;
+ }
+ const sstables::sstable_set& main_sstable_set() const override {
+ return *_cg.main_sstables();
+ }
+ const sstables::sstable_set& maintenance_sstable_set() const override {
+ return *_cg.maintenance_sstables();
+ }
+ std::unordered_set<sstables::shared_sstable> fully_expired_sstables(const std::vector<sstables::shared_sstable>& sstables, gc_clock::time_point query_time) const override {
+ return sstables::get_fully_expired_sstables(*this, sstables, query_time);
+ }
+ const std::vector<sstables::shared_sstable>& compacted_undeleted_sstables() const noexcept override {
+ return _cg.compacted_undeleted_sstables();
+ }
+ sstables::compaction_strategy& get_compaction_strategy() const noexcept override {
+ return _t.get_compaction_strategy();
+ }
+ compaction::compaction_strategy_state& get_compaction_strategy_state() noexcept override {
+ return _cg._compaction_strategy_state;
+ }
+ reader_permit make_compaction_reader_permit() const override {
+ return _t.compaction_concurrency_semaphore().make_tracking_only_permit(schema(), "compaction", db::no_timeout, {});
+ }
+ sstables::sstables_manager& get_sstables_manager() noexcept override {
+ return _t.get_sstables_manager();
+ }
+ sstables::shared_sstable make_sstable() const override {
+ return _t.make_sstable();
+ }
+ sstables::sstable_writer_config configure_writer(sstring origin) const override {
+ auto cfg = _t.get_sstables_manager().configure_writer(std::move(origin));
+ return cfg;
+ }
+ api::timestamp_type min_memtable_timestamp() const override {
+ return _cg.min_memtable_timestamp();
+ }
+ bool memtable_has_key(const dht::decorated_key& key) const override {
+ return _cg.memtable_has_key(key);
+ }
+ future<> on_compaction_completion(sstables::compaction_completion_desc desc, sstables::offstrategy offstrategy) override {
+ if (offstrategy) {
+ co_await _cg.update_sstable_lists_on_off_strategy_completion(std::move(desc));
+ _cg.trigger_compaction();
+ co_return;
+ }
+ co_await _cg.update_main_sstable_list_on_compaction_completion(std::move(desc));
+ }
+ bool is_auto_compaction_disabled_by_user() const noexcept override {
+ return _t.is_auto_compaction_disabled_by_user();
+ }
+ bool tombstone_gc_enabled() const noexcept override {
+ return _t._tombstone_gc_enabled;
+ }
+ const tombstone_gc_state& get_tombstone_gc_state() const noexcept override {
+ return _t.get_compaction_manager().get_tombstone_gc_state();
+ }
+ compaction_backlog_tracker& get_backlog_tracker() override {
+ return _t._compaction_manager.get_backlog_tracker(*this);
+ }
+ const std::string get_group_id() const noexcept override {
+ return fmt::format("{}", _cg.group_id());
+ }
+
+ seastar::condition_variable& get_staging_done_condition() noexcept override {
+ return _cg.get_staging_done_condition();
+ }
+};
+
compaction_group::compaction_group(table& t, size_t group_id, dht::token_range token_range)
: _t(t)
, _table_state(std::make_unique<table_state>(t, *this))
@@ -2005,6 +2095,8 @@ compaction_group::compaction_group(table& t, size_t group_id, dht::token_range t
_t._compaction_manager.add(as_table_state());
}

+compaction_group::~compaction_group() = default;
+
future<> compaction_group::stop() noexcept {
if (_async_gate.is_closed()) {
co_return;
@@ -3312,96 +3404,6 @@ std::vector<mutation_source> table::select_memtables_as_mutation_sources(dht::to
return mss;
}

-class compaction_group::table_state : public compaction::table_state {
- table& _t;
- compaction_group& _cg;
-public:
- explicit table_state(table& t, compaction_group& cg) : _t(t), _cg(cg) {}
-
- const schema_ptr& schema() const noexcept override {
- return _t.schema();
- }
- unsigned min_compaction_threshold() const noexcept override {
- // During receiving stream operations, the less we compact the faster streaming is. For
- // bootstrap and replace thereThere are no readers so it is fine to be less aggressive with
- // compactions as long as we don't ignore them completely (this could create a problem for
- // when streaming ends)
- if (_t._is_bootstrap_or_replace) {
- auto target = std::min(_t.schema()->max_compaction_threshold(), 16);
- return std::max(_t.schema()->min_compaction_threshold(), target);
- } else {
- return _t.schema()->min_compaction_threshold();
- }
- }
- bool compaction_enforce_min_threshold() const noexcept override {
- return _t.get_config().compaction_enforce_min_threshold || _t._is_bootstrap_or_replace;
- }
- const sstables::sstable_set& main_sstable_set() const override {
- return *_cg.main_sstables();
- }
- const sstables::sstable_set& maintenance_sstable_set() const override {
- return *_cg.maintenance_sstables();
- }
- std::unordered_set<sstables::shared_sstable> fully_expired_sstables(const std::vector<sstables::shared_sstable>& sstables, gc_clock::time_point query_time) const override {
- return sstables::get_fully_expired_sstables(*this, sstables, query_time);
- }
- const std::vector<sstables::shared_sstable>& compacted_undeleted_sstables() const noexcept override {
- return _cg.compacted_undeleted_sstables();
- }
- sstables::compaction_strategy& get_compaction_strategy() const noexcept override {
- return _t.get_compaction_strategy();
- }
- compaction::compaction_strategy_state& get_compaction_strategy_state() noexcept override {
- return _cg._compaction_strategy_state;
- }
- reader_permit make_compaction_reader_permit() const override {
- return _t.compaction_concurrency_semaphore().make_tracking_only_permit(schema(), "compaction", db::no_timeout, {});
- }
- sstables::sstables_manager& get_sstables_manager() noexcept override {
- return _t.get_sstables_manager();
- }
- sstables::shared_sstable make_sstable() const override {
- return _t.make_sstable();
- }
- sstables::sstable_writer_config configure_writer(sstring origin) const override {
- auto cfg = _t.get_sstables_manager().configure_writer(std::move(origin));
- return cfg;
- }
- api::timestamp_type min_memtable_timestamp() const override {
- return _cg.min_memtable_timestamp();
- }
- bool memtable_has_key(const dht::decorated_key& key) const override {
- return _cg.memtable_has_key(key);
- }
- future<> on_compaction_completion(sstables::compaction_completion_desc desc, sstables::offstrategy offstrategy) override {
- if (offstrategy) {
- co_await _cg.update_sstable_lists_on_off_strategy_completion(std::move(desc));
- _cg.trigger_compaction();
- co_return;
- }
- co_await _cg.update_main_sstable_list_on_compaction_completion(std::move(desc));
- }
- bool is_auto_compaction_disabled_by_user() const noexcept override {
- return _t.is_auto_compaction_disabled_by_user();
- }
- bool tombstone_gc_enabled() const noexcept override {
- return _t._tombstone_gc_enabled;
- }
- const tombstone_gc_state& get_tombstone_gc_state() const noexcept override {
- return _t.get_compaction_manager().get_tombstone_gc_state();
- }
- compaction_backlog_tracker& get_backlog_tracker() override {
- return _t._compaction_manager.get_backlog_tracker(*this);
- }
- const std::string get_group_id() const noexcept override {
- return fmt::format("{}", _cg.group_id());
- }
-
- seastar::condition_variable& get_staging_done_condition() noexcept override {
- return _cg.get_staging_done_condition();
- }
-};
-
compaction_backlog_tracker& compaction_group::get_backlog_tracker() {
return as_table_state().get_backlog_tracker();
}
diff --git a/test/lib/test_services.cc b/test/lib/test_services.cc
--- a/test/lib/test_services.cc
+++ b/test/lib/test_services.cc
@@ -26,17 +26,6 @@
static const sstring some_keyspace("ks");
static const sstring some_column_family("cf");

-table_for_tests::data::data()
-{ }
-
-table_for_tests::data::~data() {}
-
-schema_ptr table_for_tests::make_default_schema() {
- return schema_builder(some_keyspace, some_column_family)
- .with_column(utf8_type->decompose("p1"), utf8_type, column_kind::partition_key)
- .build();
-}
-
class table_for_tests::table_state : public compaction::table_state {
table_for_tests::data& _data;
sstables::sstables_manager& _sstables_manager;
@@ -127,6 +116,17 @@ class table_for_tests::table_state : public compaction::table_state {
}
};

+table_for_tests::data::data()
+{ }
+
+table_for_tests::data::~data() {}
+
+schema_ptr table_for_tests::make_default_schema() {
+ return schema_builder(some_keyspace, some_column_family)
+ .with_column(utf8_type->decompose("p1"), utf8_type, column_kind::partition_key)
+ .build();
+}
+
table_for_tests::table_for_tests(sstables::sstables_manager& sstables_manager, compaction_manager& cm, schema_ptr s, replica::table::config cfg, data_dictionary::storage_options storage)
: _data(make_lw_shared<data>())
{
diff --git a/utils/config_file.cc b/utils/config_file.cc
--- a/utils/config_file.cc
+++ b/utils/config_file.cc
@@ -270,7 +270,7 @@ utils::config_file::add_options(bpo::options_description_easy_init& init) {


bpo::options_description_easy_init&
-utils::config_file::add_deprecated_options(bpo::options_description_easy_init&& init) {
+utils::config_file::add_deprecated_options(bpo::options_description_easy_init& init) {
for (config_src& src : _cfgs) {
if (src.status() == value_status::Deprecated) {
src.add_command_line_option(init);
diff --git a/utils/config_file.hh b/utils/config_file.hh
--- a/utils/config_file.hh
+++ b/utils/config_file.hh
@@ -281,7 +281,7 @@ public:
boost::program_options::options_description_easy_init&
add_options(boost::program_options::options_description_easy_init&);
boost::program_options::options_description_easy_init&
- add_deprecated_options(boost::program_options::options_description_easy_init&&);
+ add_deprecated_options(boost::program_options::options_description_easy_init&);

/**
* Default behaviour for yaml parser is to throw on

Commit Bot

<bot@cloudius-systems.com>
unread,
Jun 28, 2024, 4:10:23 PMJun 28
to scylladb-dev@googlegroups.com, Pavel Emelyanov
From: Pavel Emelyanov <xe...@scylladb.com>
Committer: Pavel Emelyanov <xe...@scylladb.com>
Branch: master
Reply all
Reply to author
Forward
0 new messages