[scylladb-dev] [PATCH v2] db: add experimental option for raft

[Pavel Solodovnikov]

Introduce `raft` experimental option.
Adjust the tests accordingly to accomodate the new option.

Hide the code related to `raft_group_registry` behind
the switch. The service object is still constructed
but no initialization is performed (`init()` is not
called) if the flag is not set.

Later, other raft-related things, such as raft schema
changes, will also use this flag.

Also, don't introduce a corresponding gossiper feature
just yet, because again, it should be done after the
raft schema changes API contract is stabilized.

This will be done in a separate series, probably related to
implementing the feature itself.

Tests: unit(dev)

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
---
Changelog:

v2:
* Rename `raft-schema-changes` flag to just `raft`
* Don't initialize raft RPC if the flag is not set

db/config.cc | 10 ++++++++--
db/config.hh | 2 +-
main.cc | 12 ++++++++++--
test/boost/config_test.cc | 19 +++++++++++++++++++
4 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/db/config.cc b/db/config.cc
index 00897e100a..df8197862b 100644
--- a/db/config.cc
+++ b/db/config.cc
@@ -985,11 +985,17 @@ std::unordered_map<sstring, db::experimental_features_t::feature> db::experiment
// to UNUSED switch for a while, then remove altogether.
// Change Data Capture is no longer experimental. Map it
// to UNUSED_CDC switch for a while, then remove altogether.
- return {{"lwt", UNUSED}, {"udf", UDF}, {"cdc", UNUSED_CDC}, {"alternator-streams", ALTERNATOR_STREAMS}};
+ return {
+ {"lwt", UNUSED},
+ {"udf", UDF},
+ {"cdc", UNUSED_CDC},
+ {"alternator-streams", ALTERNATOR_STREAMS},
+ {"raft", RAFT}
+ };
}

std::vector<enum_option<db::experimental_features_t>> db::experimental_features_t::all() {
- return {UDF, ALTERNATOR_STREAMS};
+ return {UDF, ALTERNATOR_STREAMS, RAFT};
}

std::unordered_map<sstring, db::tri_mode_restriction_t::mode> db::tri_mode_restriction_t::map() {
diff --git a/db/config.hh b/db/config.hh
index c87b813f9b..940f79548c 100644
--- a/db/config.hh
+++ b/db/config.hh
@@ -82,7 +82,7 @@ namespace db {

/// Enumeration of all valid values for the `experimental` config entry.
struct experimental_features_t {
- enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS };
+ enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS, RAFT };
static std::unordered_map<sstring, feature> map(); // See enum_option.
static std::vector<enum_option<experimental_features_t>> all();
};
diff --git a/main.cc b/main.cc
index 871aab16fd..c1d0f5f6e2 100644
--- a/main.cc
+++ b/main.cc
@@ -1105,11 +1105,19 @@ int main(int ac, char** av) {
auto stop_proxy_handlers = defer_verbose_shutdown("storage proxy RPC verbs", [&proxy] {
proxy.invoke_on_all(&service::storage_proxy::uninit_messaging_service).get();
});
- supervisor::notify("starting Raft RPC");
- raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+
+ const bool raft_enabled = cfg->check_experimental(db::experimental_features_t::RAFT);
+ if (raft_enabled) {
+ supervisor::notify("starting Raft RPC");
+ raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+ }
auto stop_raft_rpc = defer_verbose_shutdown("Raft RPC", [&raft_gr] {
raft_gr.invoke_on_all(&service::raft_group_registry::uninit).get();
});
+ if (!raft_enabled) {
+ stop_raft_rpc->cancel();
+ }
+
supervisor::notify("starting streaming service");
streaming::stream_session::init_streaming_service(db, sys_dist_ks, view_update_generator, messaging, mm).get();
auto stop_streaming_service = defer_verbose_shutdown("streaming service", [] {
diff --git a/test/boost/config_test.cc b/test/boost/config_test.cc
index cb3bc3ea9f..7189122a9e 100644
--- a/test/boost/config_test.cc
+++ b/test/boost/config_test.cc
@@ -936,6 +936,7 @@ SEASTAR_TEST_CASE(test_parse_experimental_features_cdc) {
BOOST_CHECK(!cfg.check_experimental(ef::UNUSED));
BOOST_CHECK(!cfg.check_experimental(ef::UDF));
BOOST_CHECK(!cfg.check_experimental(ef::ALTERNATOR_STREAMS));
+ BOOST_CHECK(!cfg.check_experimental(ef::RAFT));
return make_ready_future();
}

@@ -948,6 +949,7 @@ SEASTAR_TEST_CASE(test_parse_experimental_features_unused) {
BOOST_CHECK(cfg.check_experimental(ef::UNUSED));
BOOST_CHECK(!cfg.check_experimental(ef::UDF));
BOOST_CHECK(!cfg.check_experimental(ef::ALTERNATOR_STREAMS));
+ BOOST_CHECK(!cfg.check_experimental(ef::RAFT));
return make_ready_future();
}

@@ -960,6 +962,7 @@ SEASTAR_TEST_CASE(test_parse_experimental_features_udf) {
BOOST_CHECK(!cfg.check_experimental(ef::UNUSED));
BOOST_CHECK(cfg.check_experimental(ef::UDF));
BOOST_CHECK(!cfg.check_experimental(ef::ALTERNATOR_STREAMS));
+ BOOST_CHECK(!cfg.check_experimental(ef::RAFT));
return make_ready_future();
}

@@ -972,6 +975,20 @@ SEASTAR_TEST_CASE(test_parse_experimental_features_alternator_streams) {
BOOST_CHECK(!cfg.check_experimental(ef::UNUSED));
BOOST_CHECK(!cfg.check_experimental(ef::UDF));
BOOST_CHECK(cfg.check_experimental(ef::ALTERNATOR_STREAMS));
+ BOOST_CHECK(!cfg.check_experimental(ef::RAFT));
+ return make_ready_future();
+}
+
+SEASTAR_TEST_CASE(test_parse_experimental_features_raft) {
+ auto cfg_ptr = std::make_unique<config>();
+ config& cfg = *cfg_ptr;
+ cfg.read_from_yaml("experimental_features:\n - raft\n", throw_on_error);
+ BOOST_CHECK_EQUAL(cfg.experimental_features(), features{ef::RAFT});
+ BOOST_CHECK(!cfg.check_experimental(ef::UNUSED_CDC));
+ BOOST_CHECK(!cfg.check_experimental(ef::UNUSED));
+ BOOST_CHECK(!cfg.check_experimental(ef::UDF));
+ BOOST_CHECK(!cfg.check_experimental(ef::ALTERNATOR_STREAMS));
+ BOOST_CHECK(cfg.check_experimental(ef::RAFT));
return make_ready_future();
}

@@ -1011,6 +1028,7 @@ SEASTAR_TEST_CASE(test_parse_experimental_true) {
BOOST_CHECK(!cfg.check_experimental(ef::UNUSED));
BOOST_CHECK(cfg.check_experimental(ef::UDF));
BOOST_CHECK(cfg.check_experimental(ef::ALTERNATOR_STREAMS));
+ BOOST_CHECK(cfg.check_experimental(ef::RAFT));
return make_ready_future();
}

@@ -1022,5 +1040,6 @@ SEASTAR_TEST_CASE(test_parse_experimental_false) {
BOOST_CHECK(!cfg.check_experimental(ef::UNUSED));
BOOST_CHECK(!cfg.check_experimental(ef::UDF));
BOOST_CHECK(!cfg.check_experimental(ef::ALTERNATOR_STREAMS));
+ BOOST_CHECK(!cfg.check_experimental(ef::RAFT));
return make_ready_future();
}
--
2.31.1

[Konstantin Osipov]

* Pavel Solodovnikov <pa.solo...@scylladb.com> [21/07/28 23:07]:

> Introduce `raft` experimental option.
> Adjust the tests accordingly to accomodate the new option.

Thanks!

Reviewed-by: Konstantin Osipov <kos...@scylladb.com>

To reviewers: I think option --experimental=raft is good, because
this option switches on the core raft infrastructure. If we don't have
it, we we don't have any other raft feature, such as raft-schema
or raft-topology.

We can also try to pack as much functionality as possible into
this experimental option (e.g. raft-schema).

> --
> You received this message because you are subscribed to the Google Groups "ScyllaDB development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to scylladb-dev...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/scylladb-dev/20210728200230.1330396-1-pa.solodovnikov%40scylladb.com.

--
Konstantin Osipov, Moscow, Russia
https://scylladb.com

[Kamil Braun]

BTW. Note that people are actually running --experimental on production clusters and in practice we still need to support upgrades even when in theory we don't (I learned the hard way...)

AFAIK --experimental turns every experimental feature on.

Maybe we should make an exception here? I.e. only turn Raft stuff on if we explicitly ask for --experimental=raft (and not just --experimental).

To view this discussion on the web visit https://groups.google.com/d/msgid/scylladb-dev/20210730065414.GA363728%40starling.

[Pavel Solodovnikov]

On Fri, Jul 30, 2021 at 11:59 AM Kamil Braun <kbr...@scylladb.com> wrote:
>
> BTW. Note that people are actually running --experimental on production clusters and in practice we still need to support upgrades even when in theory we don't (I learned the hard way...)
> AFAIK --experimental turns every experimental feature on.
> Maybe we should make an exception here? I.e. only turn Raft stuff on if we explicitly ask for --experimental=raft (and not just --experimental).

Sounds reasonable to me. Kostja, what do you think?

[Konstantin Osipov]

* Pavel Solodovnikov <pa.solo...@scylladb.com> [21/07/30 19:23]:

> On Fri, Jul 30, 2021 at 11:59 AM Kamil Braun <kbr...@scylladb.com> wrote:
> >
> > BTW. Note that people are actually running --experimental on production clusters and in practice we still need to support upgrades even when in theory we don't (I learned the hard way...)
> > AFAIK --experimental turns every experimental feature on.
> > Maybe we should make an exception here? I.e. only turn Raft stuff on if we explicitly ask for --experimental=raft (and not just --experimental).
> Sounds reasonable to me. Kostja, what do you think?

I think we do not officially support people running
--experimental.

[Kamil Braun]

I thought that too but then you have angry users (potential enterprise customers!) asking for workarounds on Slack :)

[Konstantin Osipov]

* Kamil Braun <kbr...@scylladb.com> [21/07/30 20:39]:

> > > > BTW. Note that people are actually running --experimental on
> > production clusters and in practice we still need to support upgrades even
> > when in theory we don't (I learned the hard way...)
> > > > AFAIK --experimental turns every experimental feature on.
> > > > Maybe we should make an exception here? I.e. only turn Raft stuff on
> > if we explicitly ask for --experimental=raft (and not just --experimental).
> > > Sounds reasonable to me. Kostja, what do you think?
> >
> > I think we do not officially support people running
> > --experimental.
> >
> I thought that too but then you have angry users (potential enterprise
> customers!) asking for workarounds on Slack :)

I think we as developers should not self-inflict this. If
Tzach or Shlomi insist, we should support it. We can not possibly atone
every user sin.

[Pavel Solodovnikov]

Shlomi, what do you think about this?

[Kamil Braun]

I wish you took this view regarding cluster seeds configuration :)

The difference is if a user misconfigures seeds during fresh cluster bootstrap, they won't be able to boot the cluster and it's fixable in 5 seconds.

But if they enable experimental by mistake and run it in production for a couple of months, they'll have much bigger problems.

[Konstantin Osipov]

* Kamil Braun <kbr...@scylladb.com> [21/08/02 13:31]:

> > --experimental).
> > > > > Sounds reasonable to me. Kostja, what do you think?
> > > >
> > > > I think we do not officially support people running
> > > > --experimental.
> > > >
> > > I thought that too but then you have angry users (potential enterprise
> > > customers!) asking for workarounds on Slack :)
> >
> > I think we as developers should not self-inflict this. If
> > Tzach or Shlomi insist, we should support it. We can not possibly atone
> > every user sin.
> >
> I wish you took this view regarding cluster seeds configuration :)

1) This is exactly why product management matters. Parallel boot
is a declared product goal, so we should try to get closer to
making it happen. Supporting --experimental users in
production is a non-goal, so we should focus on cutting the
time spent on it.

2) I did in the end, I accepted that it's enough to abort parallel
boot rather than try to serialize it in scope of our current
work.

[Pavel Solodovnikov]

On Mon, Aug 2, 2021 at 5:35 PM Konstantin Osipov <kos...@scylladb.com> wrote:
>
> * Kamil Braun <kbr...@scylladb.com> [21/08/02 13:31]:
> > > --experimental).
> > > > > > Sounds reasonable to me. Kostja, what do you think?
> > > > >
> > > > > I think we do not officially support people running
> > > > > --experimental.
> > > > >
> > > > I thought that too but then you have angry users (potential enterprise
> > > > customers!) asking for workarounds on Slack :)
> > >
> > > I think we as developers should not self-inflict this. If
> > > Tzach or Shlomi insist, we should support it. We can not possibly atone
> > > every user sin.
> > >
> > I wish you took this view regarding cluster seeds configuration :)
>
> 1) This is exactly why product management matters. Parallel boot
> is a declared product goal, so we should try to get closer to
> making it happen. Supporting --experimental users in
> production is a non-goal, so we should focus on cutting the
> time spent on it.
>
> 2) I did in the end, I accepted that it's enough to abort parallel
> boot rather than try to serialize it in scope of our current
> work.

So, Kostja, do you find Kamil's arguments sufficient?

Personally, I tend to agree with him.
Despite that you are right, it's not a big deal to alter the behavior
a little and save us the trouble later when dealing with existing
users, who already shoot themselves in the foot.

[Pavel Solodovnikov]

Introduce `raft` experimental option.
Adjust the tests accordingly to accomodate the new option.

It's not enabled by default when providing
`--experimental=true` config option and should be
requested explicitly via `--experimental-options=raft`
config option.

Hide the code related to `raft_group_registry` behind
the switch. The service object is still constructed
but no initialization is performed (`init()` is not
called) if the flag is not set.

Later, other raft-related things, such as raft schema
changes, will also use this flag.

Also, don't introduce a corresponding gossiper feature
just yet, because again, it should be done after the
raft schema changes API contract is stabilized.

This will be done in a separate series, probably related to
implementing the feature itself.

Tests: unit(dev)

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
---
Changelog:

v3:
* Enable the option only when explicitly asked to do so.
Don't enable with umbrella `experimental=true` option.

v2:
* Rename `raft-schema-changes` flag to just `raft`
* Don't initialize raft RPC if the flag is not set

db/config.cc | 17 +++++++++++++----
db/config.hh | 4 +++-

main.cc | 12 ++++++++++--
test/boost/config_test.cc | 19 +++++++++++++++++++

4 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/db/config.cc b/db/config.cc
index 00897e100a..962b109f16 100644
--- a/db/config.cc
+++ b/db/config.cc
@@ -929,8 +929,11 @@ db::fs::path db::config::get_conf_sub(db::fs::path sub) {
}

bool db::config::check_experimental(experimental_features_t::feature f) const {
- if (experimental() && f != experimental_features_t::UNUSED && f != experimental_features_t::UNUSED_CDC) {
- return true;
+ if (experimental()
+ && f != experimental_features_t::UNUSED
+ && f != experimental_features_t::UNUSED_CDC
+ && f != experimental_features_t::RAFT) {
+ return true;
}
const auto& optval = experimental_features();
return find(begin(optval), end(optval), enum_option<experimental_features_t>{f}) != end(optval);
@@ -985,11 +988,17 @@ std::unordered_map<sstring, db::experimental_features_t::feature> db::experiment

// to UNUSED switch for a while, then remove altogether.
// Change Data Capture is no longer experimental. Map it
// to UNUSED_CDC switch for a while, then remove altogether.
- return {{"lwt", UNUSED}, {"udf", UDF}, {"cdc", UNUSED_CDC}, {"alternator-streams", ALTERNATOR_STREAMS}};
+ return {
+ {"lwt", UNUSED},
+ {"udf", UDF},
+ {"cdc", UNUSED_CDC},
+ {"alternator-streams", ALTERNATOR_STREAMS},
+ {"raft", RAFT}
+ };
}

std::vector<enum_option<db::experimental_features_t>> db::experimental_features_t::all() {
- return {UDF, ALTERNATOR_STREAMS};
+ return {UDF, ALTERNATOR_STREAMS, RAFT};
}

std::unordered_map<sstring, db::tri_mode_restriction_t::mode> db::tri_mode_restriction_t::map() {
diff --git a/db/config.hh b/db/config.hh

index c87b813f9b..c2c2d3dbd1 100644
--- a/db/config.hh
+++ b/db/config.hh
@@ -82,7 +82,9 @@ namespace db {

/// Enumeration of all valid values for the `experimental` config entry.
struct experimental_features_t {
- enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS };

+ // NOTE: RAFT feature is not enabled via `experimental` umbrella flag.
+ // This option should be enabled explicitly.

+ enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS, RAFT };
static std::unordered_map<sstring, feature> map(); // See enum_option.
static std::vector<enum_option<experimental_features_t>> all();
};
diff --git a/main.cc b/main.cc

index 5dc7135d43..82c258e376 100644
--- a/main.cc
+++ b/main.cc
@@ -1115,11 +1115,19 @@ int main(int ac, char** av) {

auto stop_proxy_handlers = defer_verbose_shutdown("storage proxy RPC verbs", [&proxy] {
proxy.invoke_on_all(&service::storage_proxy::uninit_messaging_service).get();
});
- supervisor::notify("starting Raft RPC");
- raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+
+ const bool raft_enabled = cfg->check_experimental(db::experimental_features_t::RAFT);
+ if (raft_enabled) {
+ supervisor::notify("starting Raft RPC");
+ raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+ }
auto stop_raft_rpc = defer_verbose_shutdown("Raft RPC", [&raft_gr] {
raft_gr.invoke_on_all(&service::raft_group_registry::uninit).get();
});
+ if (!raft_enabled) {
+ stop_raft_rpc->cancel();
+ }
+
supervisor::notify("starting streaming service");
streaming::stream_session::init_streaming_service(db, sys_dist_ks, view_update_generator, messaging, mm).get();
auto stop_streaming_service = defer_verbose_shutdown("streaming service", [] {
diff --git a/test/boost/config_test.cc b/test/boost/config_test.cc

index cb3bc3ea9f..7f90c8a78b 100644

[Avi Kivity]

On 23/08/2021 15.19, Pavel Solodovnikov wrote:
> Introduce `raft` experimental option.
> Adjust the tests accordingly to accomodate the new option.
>
> It's not enabled by default when providing
> `--experimental=true` config option and should be
> requested explicitly via `--experimental-options=raft`
> config option.
>
> Hide the code related to `raft_group_registry` behind
> the switch. The service object is still constructed
> but no initialization is performed (`init()` is not
> called) if the flag is not set.

What about the raft tables in the system keyspace? They should not be
created unless the feature is enabled.

> Later, other raft-related things, such as raft schema
> changes, will also use this flag.

I expect that later raft-based features will have new flags. For
example, after raft schema changes are made generally available, user
raft tables can be an experimental features.

[Pavel Solodovnikov]

пн, 23 авг. 2021 г., 15:27 Avi Kivity <a...@scylladb.com>:

On 23/08/2021 15.19, Pavel Solodovnikov wrote:
> Introduce `raft` experimental option.
> Adjust the tests accordingly to accomodate the new option.
>
> It's not enabled by default when providing
> `--experimental=true` config option and should be
> requested explicitly via `--experimental-options=raft`
> config option.
>
> Hide the code related to `raft_group_registry` behind
> the switch. The service object is still constructed
> but no initialization is performed (`init()` is not
> called) if the flag is not set.


What about the raft tables in the system keyspace? They should not be
created unless the feature is enabled.

Sure, I planned to post it as a separate fix, though.

> Later, other raft-related things, such as raft schema
> changes, will also use this flag.


I expect that later raft-based features will have new flags. For
example, after raft schema changes are made generally available, user
raft tables can be an experimental features.

Yes. This option (raft) is intended to act as a main switch for all raft related features.

[Avi Kivity]

On 23/08/2021 15.45, Pavel Solodovnikov wrote:

пн, 23 авг. 2021 г., 15:27 Avi Kivity <a...@scylladb.com>:

On 23/08/2021 15.19, Pavel Solodovnikov wrote:
> Introduce `raft` experimental option.
> Adjust the tests accordingly to accomodate the new option.
>
> It's not enabled by default when providing
> `--experimental=true` config option and should be
> requested explicitly via `--experimental-options=raft`
> config option.
>
> Hide the code related to `raft_group_registry` behind
> the switch. The service object is still constructed
> but no initialization is performed (`init()` is not
> called) if the flag is not set.


What about the raft tables in the system keyspace? They should not be
created unless the feature is enabled.

Sure, I planned to post it as a separate fix, though.

Ok. Since we'll need to backport this (and the separate fix) to 4.5, I'll need an issue number.

[Commit Bot]

From: Pavel Solodovnikov <pa.solo...@scylladb.com>
Committer: Avi Kivity <a...@scylladb.com>
Branch: next

db: add experimental option for raft

Introduce `raft` experimental option.
Adjust the tests accordingly to accomodate the new option.

It's not enabled by default when providing
`--experimental=true` config option and should be
requested explicitly via `--experimental-options=raft`
config option.

Hide the code related to `raft_group_registry` behind
the switch. The service object is still constructed
but no initialization is performed (`init()` is not
called) if the flag is not set.

Later, other raft-related things, such as raft schema
changes, will also use this flag.

Also, don't introduce a corresponding gossiper feature
just yet, because again, it should be done after the
raft schema changes API contract is stabilized.

This will be done in a separate series, probably related to
implementing the feature itself.

Tests: unit(dev)

Ref #9239.

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
Message-Id: <20210823121956.1676...@scylladb.com>

---
diff --git a/db/config.cc b/db/config.cc
--- a/db/config.cc
+++ b/db/config.cc
@@ -931,8 +931,11 @@ db::fs::path db::config::get_conf_sub(db::fs::path sub) {

}

bool db::config::check_experimental(experimental_features_t::feature f) const {
- if (experimental() && f != experimental_features_t::UNUSED && f != experimental_features_t::UNUSED_CDC) {
- return true;
+ if (experimental()
+ && f != experimental_features_t::UNUSED
+ && f != experimental_features_t::UNUSED_CDC
+ && f != experimental_features_t::RAFT) {
+ return true;
}
const auto& optval = experimental_features();
return find(begin(optval), end(optval), enum_option<experimental_features_t>{f}) != end(optval);

@@ -987,11 +990,17 @@ std::unordered_map<sstring, db::experimental_features_t::feature> db::experiment

// to UNUSED switch for a while, then remove altogether.
// Change Data Capture is no longer experimental. Map it
// to UNUSED_CDC switch for a while, then remove altogether.
- return {{"lwt", UNUSED}, {"udf", UDF}, {"cdc", UNUSED_CDC}, {"alternator-streams", ALTERNATOR_STREAMS}};
+ return {
+ {"lwt", UNUSED},
+ {"udf", UDF},
+ {"cdc", UNUSED_CDC},
+ {"alternator-streams", ALTERNATOR_STREAMS},
+ {"raft", RAFT}
+ };
}

std::vector<enum_option<db::experimental_features_t>> db::experimental_features_t::all() {
- return {UDF, ALTERNATOR_STREAMS};
+ return {UDF, ALTERNATOR_STREAMS, RAFT};
}

std::unordered_map<sstring, db::tri_mode_restriction_t::mode> db::tri_mode_restriction_t::map() {
diff --git a/db/config.hh b/db/config.hh

--- a/db/config.hh
+++ b/db/config.hh
@@ -82,7 +82,9 @@ namespace db {

/// Enumeration of all valid values for the `experimental` config entry.
struct experimental_features_t {
- enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS };
+ // NOTE: RAFT feature is not enabled via `experimental` umbrella flag.
+ // This option should be enabled explicitly.
+ enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS, RAFT };
static std::unordered_map<sstring, feature> map(); // See enum_option.
static std::vector<enum_option<experimental_features_t>> all();
};
diff --git a/main.cc b/main.cc

--- a/main.cc
+++ b/main.cc
@@ -1119,11 +1119,19 @@ int main(int ac, char** av) {

auto stop_proxy_handlers = defer_verbose_shutdown("storage proxy RPC verbs", [&proxy] {
proxy.invoke_on_all(&service::storage_proxy::uninit_messaging_service).get();
});
- supervisor::notify("starting Raft RPC");
- raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+
+ const bool raft_enabled = cfg->check_experimental(db::experimental_features_t::RAFT);
+ if (raft_enabled) {
+ supervisor::notify("starting Raft RPC");
+ raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+ }
auto stop_raft_rpc = defer_verbose_shutdown("Raft RPC", [&raft_gr] {
raft_gr.invoke_on_all(&service::raft_group_registry::uninit).get();
});
+ if (!raft_enabled) {
+ stop_raft_rpc->cancel();
+ }
+
supervisor::notify("starting streaming service");
streaming::stream_session::init_streaming_service(db, sys_dist_ks, view_update_generator, messaging, mm).get();
auto stop_streaming_service = defer_verbose_shutdown("streaming service", [] {
diff --git a/test/boost/config_test.cc b/test/boost/config_test.cc

[Commit Bot]

From: Pavel Solodovnikov <pa.solo...@scylladb.com>
Committer: Avi Kivity <a...@scylladb.com>
Branch: next

db: add experimental option for raft

Introduce `raft` experimental option.
Adjust the tests accordingly to accomodate the new option.

It's not enabled by default when providing
`--experimental=true` config option and should be
requested explicitly via `--experimental-options=raft`
config option.

Hide the code related to `raft_group_registry` behind
the switch. The service object is still constructed
but no initialization is performed (`init()` is not
called) if the flag is not set.

Later, other raft-related things, such as raft schema
changes, will also use this flag.

Also, don't introduce a corresponding gossiper feature
just yet, because again, it should be done after the
raft schema changes API contract is stabilized.

This will be done in a separate series, probably related to
implementing the feature itself.

Tests: unit(dev)

Ref #9239.

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
Message-Id: <20210823121956.1676...@scylladb.com>

---
diff --git a/db/config.cc b/db/config.cc

--- a/db/config.cc
+++ b/db/config.cc
@@ -931,8 +931,11 @@ db::fs::path db::config::get_conf_sub(db::fs::path sub) {

}

bool db::config::check_experimental(experimental_features_t::feature f) const {
- if (experimental() && f != experimental_features_t::UNUSED && f != experimental_features_t::UNUSED_CDC) {
- return true;
+ if (experimental()
+ && f != experimental_features_t::UNUSED
+ && f != experimental_features_t::UNUSED_CDC
+ && f != experimental_features_t::RAFT) {
+ return true;
}
const auto& optval = experimental_features();
return find(begin(optval), end(optval), enum_option<experimental_features_t>{f}) != end(optval);

@@ -987,11 +990,17 @@ std::unordered_map<sstring, db::experimental_features_t::feature> db::experiment

// to UNUSED switch for a while, then remove altogether.
// Change Data Capture is no longer experimental. Map it
// to UNUSED_CDC switch for a while, then remove altogether.
- return {{"lwt", UNUSED}, {"udf", UDF}, {"cdc", UNUSED_CDC}, {"alternator-streams", ALTERNATOR_STREAMS}};
+ return {
+ {"lwt", UNUSED},
+ {"udf", UDF},
+ {"cdc", UNUSED_CDC},
+ {"alternator-streams", ALTERNATOR_STREAMS},
+ {"raft", RAFT}
+ };
}

std::vector<enum_option<db::experimental_features_t>> db::experimental_features_t::all() {
- return {UDF, ALTERNATOR_STREAMS};
+ return {UDF, ALTERNATOR_STREAMS, RAFT};
}

std::unordered_map<sstring, db::tri_mode_restriction_t::mode> db::tri_mode_restriction_t::map() {
diff --git a/db/config.hh b/db/config.hh

--- a/db/config.hh
+++ b/db/config.hh
@@ -82,7 +82,9 @@ namespace db {

/// Enumeration of all valid values for the `experimental` config entry.
struct experimental_features_t {
- enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS };
+ // NOTE: RAFT feature is not enabled via `experimental` umbrella flag.
+ // This option should be enabled explicitly.
+ enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS, RAFT };
static std::unordered_map<sstring, feature> map(); // See enum_option.
static std::vector<enum_option<experimental_features_t>> all();
};
diff --git a/main.cc b/main.cc

--- a/main.cc
+++ b/main.cc
@@ -1119,11 +1119,19 @@ int main(int ac, char** av) {

auto stop_proxy_handlers = defer_verbose_shutdown("storage proxy RPC verbs", [&proxy] {
proxy.invoke_on_all(&service::storage_proxy::uninit_messaging_service).get();
});
- supervisor::notify("starting Raft RPC");
- raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+
+ const bool raft_enabled = cfg->check_experimental(db::experimental_features_t::RAFT);
+ if (raft_enabled) {
+ supervisor::notify("starting Raft RPC");
+ raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+ }
auto stop_raft_rpc = defer_verbose_shutdown("Raft RPC", [&raft_gr] {
raft_gr.invoke_on_all(&service::raft_group_registry::uninit).get();
});
+ if (!raft_enabled) {
+ stop_raft_rpc->cancel();
+ }
+
supervisor::notify("starting streaming service");
streaming::stream_session::init_streaming_service(db, sys_dist_ks, view_update_generator, messaging, mm).get();
auto stop_streaming_service = defer_verbose_shutdown("streaming service", [] {
diff --git a/test/boost/config_test.cc b/test/boost/config_test.cc

[Commit Bot]

From: Pavel Solodovnikov <pa.solo...@scylladb.com>
Committer: Avi Kivity <a...@scylladb.com>

Branch: master

db: add experimental option for raft

Introduce `raft` experimental option.
Adjust the tests accordingly to accomodate the new option.

It's not enabled by default when providing
`--experimental=true` config option and should be
requested explicitly via `--experimental-options=raft`
config option.

Hide the code related to `raft_group_registry` behind
the switch. The service object is still constructed
but no initialization is performed (`init()` is not
called) if the flag is not set.

Later, other raft-related things, such as raft schema
changes, will also use this flag.

Also, don't introduce a corresponding gossiper feature
just yet, because again, it should be done after the
raft schema changes API contract is stabilized.

This will be done in a separate series, probably related to
implementing the feature itself.

Tests: unit(dev)

Ref #9239.

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
Message-Id: <20210823121956.1676...@scylladb.com>

---
diff --git a/db/config.cc b/db/config.cc

--- a/db/config.cc
+++ b/db/config.cc
@@ -931,8 +931,11 @@ db::fs::path db::config::get_conf_sub(db::fs::path sub) {

}

bool db::config::check_experimental(experimental_features_t::feature f) const {
- if (experimental() && f != experimental_features_t::UNUSED && f != experimental_features_t::UNUSED_CDC) {
- return true;
+ if (experimental()
+ && f != experimental_features_t::UNUSED
+ && f != experimental_features_t::UNUSED_CDC
+ && f != experimental_features_t::RAFT) {
+ return true;
}
const auto& optval = experimental_features();
return find(begin(optval), end(optval), enum_option<experimental_features_t>{f}) != end(optval);

@@ -987,11 +990,17 @@ std::unordered_map<sstring, db::experimental_features_t::feature> db::experiment

// to UNUSED switch for a while, then remove altogether.
// Change Data Capture is no longer experimental. Map it
// to UNUSED_CDC switch for a while, then remove altogether.
- return {{"lwt", UNUSED}, {"udf", UDF}, {"cdc", UNUSED_CDC}, {"alternator-streams", ALTERNATOR_STREAMS}};
+ return {
+ {"lwt", UNUSED},
+ {"udf", UDF},
+ {"cdc", UNUSED_CDC},
+ {"alternator-streams", ALTERNATOR_STREAMS},
+ {"raft", RAFT}
+ };
}

std::vector<enum_option<db::experimental_features_t>> db::experimental_features_t::all() {
- return {UDF, ALTERNATOR_STREAMS};
+ return {UDF, ALTERNATOR_STREAMS, RAFT};
}

std::unordered_map<sstring, db::tri_mode_restriction_t::mode> db::tri_mode_restriction_t::map() {
diff --git a/db/config.hh b/db/config.hh

--- a/db/config.hh
+++ b/db/config.hh
@@ -82,7 +82,9 @@ namespace db {

/// Enumeration of all valid values for the `experimental` config entry.
struct experimental_features_t {
- enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS };
+ // NOTE: RAFT feature is not enabled via `experimental` umbrella flag.
+ // This option should be enabled explicitly.
+ enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS, RAFT };
static std::unordered_map<sstring, feature> map(); // See enum_option.
static std::vector<enum_option<experimental_features_t>> all();
};
diff --git a/main.cc b/main.cc

--- a/main.cc
+++ b/main.cc
@@ -1119,11 +1119,19 @@ int main(int ac, char** av) {

auto stop_proxy_handlers = defer_verbose_shutdown("storage proxy RPC verbs", [&proxy] {
proxy.invoke_on_all(&service::storage_proxy::uninit_messaging_service).get();
});
- supervisor::notify("starting Raft RPC");
- raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+
+ const bool raft_enabled = cfg->check_experimental(db::experimental_features_t::RAFT);
+ if (raft_enabled) {
+ supervisor::notify("starting Raft RPC");
+ raft_gr.invoke_on_all(&service::raft_group_registry::init).get();
+ }
auto stop_raft_rpc = defer_verbose_shutdown("Raft RPC", [&raft_gr] {
raft_gr.invoke_on_all(&service::raft_group_registry::uninit).get();
});
+ if (!raft_enabled) {
+ stop_raft_rpc->cancel();
+ }
+
supervisor::notify("starting streaming service");
streaming::stream_session::init_streaming_service(db, sys_dist_ks, view_update_generator, messaging, mm).get();
auto stop_streaming_service = defer_verbose_shutdown("streaming service", [] {
diff --git a/test/boost/config_test.cc b/test/boost/config_test.cc

[Commit Bot]

From: Pavel Solodovnikov <pa.solo...@scylladb.com>
Committer: Avi Kivity <a...@scylladb.com>

Branch: next-4.5

db: add experimental option for raft

Introduce `raft` experimental option.
Adjust the tests accordingly to accomodate the new option.

It's not enabled by default when providing
`--experimental=true` config option and should be
requested explicitly via `--experimental-options=raft`
config option.

Hide the code related to `raft_group_registry` behind
the switch. The service object is still constructed
but no initialization is performed (`init()` is not
called) if the flag is not set.

Later, other raft-related things, such as raft schema
changes, will also use this flag.

Also, don't introduce a corresponding gossiper feature
just yet, because again, it should be done after the
raft schema changes API contract is stabilized.

This will be done in a separate series, probably related to
implementing the feature itself.

Tests: unit(dev)

Ref #9239.

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
Message-Id: <20210823121956.1676...@scylladb.com>

(cherry picked from commit 22794efc220ef85e01f343b255b603a72e0de1f9)

---
diff --git a/db/config.cc b/db/config.cc

--- a/db/config.cc
+++ b/db/config.cc
@@ -908,8 +908,11 @@ db::fs::path db::config::get_conf_sub(db::fs::path sub) {

}

bool db::config::check_experimental(experimental_features_t::feature f) const {
- if (experimental() && f != experimental_features_t::UNUSED && f != experimental_features_t::UNUSED_CDC) {
- return true;
+ if (experimental()
+ && f != experimental_features_t::UNUSED
+ && f != experimental_features_t::UNUSED_CDC
+ && f != experimental_features_t::RAFT) {
+ return true;
}
const auto& optval = experimental_features();
return find(begin(optval), end(optval), enum_option<experimental_features_t>{f}) != end(optval);

@@ -964,11 +967,17 @@ std::unordered_map<sstring, db::experimental_features_t::feature> db::experiment

// to UNUSED switch for a while, then remove altogether.
// Change Data Capture is no longer experimental. Map it
// to UNUSED_CDC switch for a while, then remove altogether.
- return {{"lwt", UNUSED}, {"udf", UDF}, {"cdc", UNUSED_CDC}, {"alternator-streams", ALTERNATOR_STREAMS}};
+ return {
+ {"lwt", UNUSED},
+ {"udf", UDF},
+ {"cdc", UNUSED_CDC},
+ {"alternator-streams", ALTERNATOR_STREAMS},
+ {"raft", RAFT}
+ };
}

std::vector<enum_option<db::experimental_features_t>> db::experimental_features_t::all() {
- return {UDF, ALTERNATOR_STREAMS};
+ return {UDF, ALTERNATOR_STREAMS, RAFT};
}

template struct utils::config_file::named_value<seastar::log_level>;
diff --git a/db/config.hh b/db/config.hh

--- a/db/config.hh
+++ b/db/config.hh
@@ -82,7 +82,9 @@ namespace db {

/// Enumeration of all valid values for the `experimental` config entry.
struct experimental_features_t {
- enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS };
+ // NOTE: RAFT feature is not enabled via `experimental` umbrella flag.
+ // This option should be enabled explicitly.
+ enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS, RAFT };
static std::unordered_map<sstring, feature> map(); // See enum_option.
static std::vector<enum_option<experimental_features_t>> all();
};
diff --git a/main.cc b/main.cc

--- a/main.cc
+++ b/main.cc
@@ -1046,12 +1046,20 @@ int main(int ac, char** av) {

auto stop_proxy_handlers = defer_verbose_shutdown("storage proxy RPC verbs", [&proxy] {
proxy.invoke_on_all(&service::storage_proxy::uninit_messaging_service).get();
});

- supervisor::notify("initializing Raft services");
- raft_srvs.start(std::ref(messaging), std::ref(gossiper), std::ref(qp)).get();
- raft_srvs.invoke_on_all(&raft_services::init).get();

+
+ const bool raft_enabled = cfg->check_experimental(db::experimental_features_t::RAFT);
+ if (raft_enabled) {

+ supervisor::notify("initializing Raft services");
+ raft_srvs.start(std::ref(messaging), std::ref(gossiper), std::ref(qp)).get();
+ raft_srvs.invoke_on_all(&raft_services::init).get();
+ }
auto stop_raft_sc_handlers = defer_verbose_shutdown("Raft services", [&raft_srvs] {
raft_srvs.invoke_on_all(&raft_services::uninit).get();
});
+ if (!raft_enabled) {
+ stop_raft_sc_handlers->cancel();

+ }
+
supervisor::notify("starting streaming service");

streaming::stream_session::init_streaming_service(db, sys_dist_ks, view_update_generator, messaging).get();

auto stop_streaming_service = defer_verbose_shutdown("streaming service", [] {
diff --git a/test/boost/config_test.cc b/test/boost/config_test.cc

[Commit Bot]

From: Pavel Solodovnikov <pa.solo...@scylladb.com>
Committer: Avi Kivity <a...@scylladb.com>

Branch: branch-4.5

db: add experimental option for raft

Introduce `raft` experimental option.
Adjust the tests accordingly to accomodate the new option.

It's not enabled by default when providing
`--experimental=true` config option and should be
requested explicitly via `--experimental-options=raft`
config option.

Hide the code related to `raft_group_registry` behind
the switch. The service object is still constructed
but no initialization is performed (`init()` is not
called) if the flag is not set.

Later, other raft-related things, such as raft schema
changes, will also use this flag.

Also, don't introduce a corresponding gossiper feature
just yet, because again, it should be done after the
raft schema changes API contract is stabilized.

This will be done in a separate series, probably related to
implementing the feature itself.

Tests: unit(dev)

Ref #9239.

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
Message-Id: <20210823121956.1676...@scylladb.com>
(cherry picked from commit 22794efc220ef85e01f343b255b603a72e0de1f9)

---
diff --git a/db/config.cc b/db/config.cc

--- a/db/config.cc
+++ b/db/config.cc
@@ -908,8 +908,11 @@ db::fs::path db::config::get_conf_sub(db::fs::path sub) {

}

bool db::config::check_experimental(experimental_features_t::feature f) const {
- if (experimental() && f != experimental_features_t::UNUSED && f != experimental_features_t::UNUSED_CDC) {
- return true;
+ if (experimental()
+ && f != experimental_features_t::UNUSED
+ && f != experimental_features_t::UNUSED_CDC
+ && f != experimental_features_t::RAFT) {
+ return true;
}
const auto& optval = experimental_features();
return find(begin(optval), end(optval), enum_option<experimental_features_t>{f}) != end(optval);

@@ -964,11 +967,17 @@ std::unordered_map<sstring, db::experimental_features_t::feature> db::experiment

// to UNUSED switch for a while, then remove altogether.
// Change Data Capture is no longer experimental. Map it
// to UNUSED_CDC switch for a while, then remove altogether.
- return {{"lwt", UNUSED}, {"udf", UDF}, {"cdc", UNUSED_CDC}, {"alternator-streams", ALTERNATOR_STREAMS}};
+ return {
+ {"lwt", UNUSED},
+ {"udf", UDF},
+ {"cdc", UNUSED_CDC},
+ {"alternator-streams", ALTERNATOR_STREAMS},
+ {"raft", RAFT}
+ };
}

std::vector<enum_option<db::experimental_features_t>> db::experimental_features_t::all() {
- return {UDF, ALTERNATOR_STREAMS};
+ return {UDF, ALTERNATOR_STREAMS, RAFT};
}

template struct utils::config_file::named_value<seastar::log_level>;
diff --git a/db/config.hh b/db/config.hh

--- a/db/config.hh
+++ b/db/config.hh
@@ -82,7 +82,9 @@ namespace db {

/// Enumeration of all valid values for the `experimental` config entry.
struct experimental_features_t {
- enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS };
+ // NOTE: RAFT feature is not enabled via `experimental` umbrella flag.
+ // This option should be enabled explicitly.
+ enum feature { UNUSED, UDF, UNUSED_CDC, ALTERNATOR_STREAMS, RAFT };
static std::unordered_map<sstring, feature> map(); // See enum_option.
static std::vector<enum_option<experimental_features_t>> all();
};
diff --git a/main.cc b/main.cc

--- a/main.cc
+++ b/main.cc
@@ -1046,12 +1046,20 @@ int main(int ac, char** av) {

auto stop_proxy_handlers = defer_verbose_shutdown("storage proxy RPC verbs", [&proxy] {
proxy.invoke_on_all(&service::storage_proxy::uninit_messaging_service).get();
});

- supervisor::notify("initializing Raft services");
- raft_srvs.start(std::ref(messaging), std::ref(gossiper), std::ref(qp)).get();

- raft_srvs.invoke_on_all(&raft_services::init).get();

+
+ const bool raft_enabled = cfg->check_experimental(db::experimental_features_t::RAFT);
+ if (raft_enabled) {

+ supervisor::notify("initializing Raft services");
+ raft_srvs.start(std::ref(messaging), std::ref(gossiper), std::ref(qp)).get();
+ raft_srvs.invoke_on_all(&raft_services::init).get();
+ }
auto stop_raft_sc_handlers = defer_verbose_shutdown("Raft services", [&raft_srvs] {

raft_srvs.invoke_on_all(&raft_services::uninit).get();
});
+ if (!raft_enabled) {
+ stop_raft_sc_handlers->cancel();

+ }
+
supervisor::notify("starting streaming service");

streaming::stream_session::init_streaming_service(db, sys_dist_ks, view_update_generator, messaging).get();

auto stop_streaming_service = defer_verbose_shutdown("streaming service", [] {
diff --git a/test/boost/config_test.cc b/test/boost/config_test.cc