Merge "Remove sstable_format_slector::sync()" from Pavel E

"
There's an explicit barrier in main that waits for the sstable format
selector to finish selecting it by the time node start to join a cluter.
(Actually -- not quite, when restarting a normal node it joins cluster
in prepare_to_join()).

This explicit barrier is not needed, the sync point already exists in
the way features are enabled, the format-selector just needs to use it.

branch: https://github.com/xemul/scylla/tree/br-format-selector-sync
tests: https://jenkins.scylladb.com/job/releng/job/Scylla-CI/351/
refs: #2795
"

* 'br-format-selector-sync' of https://github.com/xemul/scylla:
  format-selector: Remove .sync() point
  format-selector: Coroutinize maybe_select_format()
  format-selector: Coroutinize simple methods

db/sstables-format-selector.cc

@@ -7,6 +7,7 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
+#include <seastar/core/coroutine.hh>
 #include "sstables-format-selector.hh"
 #include "log.hh"
 #include "replica/database.hh"
@@ -23,8 +24,7 @@ static const sstring SSTABLE_FORMAT_PARAM_NAME = "sstable_format";
 void feature_enabled_listener::on_enabled() {
     if (!_started) {
         _started = true;
-        // FIXME -- discarded future
-        (void)_selector.maybe_select_format(_format);
+        _selector.maybe_select_format(_format).get();
     }
 }
 
@@ -37,55 +37,41 @@ sstables_format_selector::sstables_format_selector(gms::gossiper& g, sharded<gms
 { }
 
 future<> sstables_format_selector::maybe_select_format(sstables::sstable_version_types new_format) {
-    return with_gate(_sel, [this, new_format] {
-        return do_maybe_select_format(new_format);
-    });
-}
+    auto hg = _sel.hold();
+    auto units = co_await get_units(_sem, 1);
 
-future<> sstables_format_selector::do_maybe_select_format(sstables::sstable_version_types new_format) {
-    return with_semaphore(_sem, 1, [this, new_format] {
-        if (new_format <= _selected_format) {
-            return make_ready_future<bool>(false);
-        }
-        return db::system_keyspace::set_scylla_local_param(SSTABLE_FORMAT_PARAM_NAME, to_string(new_format)).then([this, new_format] {
-            return select_format(new_format);
-        }).then([] { return true; });
-    }).then([this] (bool update_features) {
-        if (!update_features) {
-            return make_ready_future<>();
-        }
-        return _gossiper.add_local_application_state(gms::application_state::SUPPORTED_FEATURES,
-                 gms::versioned_value::supported_features(_features.local().supported_feature_set()));
-    });
+    if (new_format > _selected_format) {
+        co_await db::system_keyspace::set_scylla_local_param(SSTABLE_FORMAT_PARAM_NAME, to_string(new_format));
+        co_await select_format(new_format);
+        // FIXME discarded future
+        (void)_gossiper.add_local_application_state(gms::application_state::SUPPORTED_FEATURES,
+                 gms::versioned_value::supported_features(_features.local().supported_feature_set())).finally([h = std::move(hg)] {});
+    }
 }
 
 future<> sstables_format_selector::start() {
     assert(this_shard_id() == 0);
-    return read_sstables_format().then([this] {
-        _features.local().me_sstable.when_enabled(_me_feature_listener);
-        _features.local().md_sstable.when_enabled(_md_feature_listener);
-        return make_ready_future<>();
-    });
+    co_await read_sstables_format();
+    _features.local().me_sstable.when_enabled(_me_feature_listener);
+    _features.local().md_sstable.when_enabled(_md_feature_listener);
 }
 
 future<> sstables_format_selector::stop() {
-    return _sel.close();
+    co_await _sel.close();
 }
 
 future<> sstables_format_selector::read_sstables_format() {
-    return db::system_keyspace::get_scylla_local_param(SSTABLE_FORMAT_PARAM_NAME).then([this] (std::optional<sstring> format_opt) {
-        if (format_opt) {
-            sstables::sstable_version_types format = sstables::from_string(*format_opt);
-            return select_format(format);
-        }
-        return make_ready_future<>();
-    });
+    std::optional<sstring> format_opt = co_await db::system_keyspace::get_scylla_local_param(SSTABLE_FORMAT_PARAM_NAME);
+    if (format_opt) {
+        sstables::sstable_version_types format = sstables::from_string(*format_opt);
+        co_await select_format(format);
+    }
 }
 
 future<> sstables_format_selector::select_format(sstables::sstable_version_types format) {
     logger.info("Selected {} sstables format", to_string(format));
     _selected_format = format;
-    return _db.invoke_on_all([this] (replica::database& db) {
+    co_await _db.invoke_on_all([this] (replica::database& db) {
         db.set_format(_selected_format);
     });
 }

db/sstables-format-selector.hh

@@ -57,8 +57,6 @@ class sstables_format_selector {
     future<> select_format(sstables::sstable_version_types new_format);
     future<> read_sstables_format();
 
-    future<> do_maybe_select_format(sstables::sstable_version_types new_format);
-
 public:
     sstables_format_selector(gms::gossiper& g, sharded<gms::feature_service>& f, sharded<replica::database>& db);
 
@@ -66,10 +64,6 @@ public:
     future<> stop();
 
     future<> maybe_select_format(sstables::sstable_version_types new_format);
-
-    void sync() {
-        get_units(_sem, 1).get0();
-    }
 };
 
 } // namespace sstables

main.cc

@@ -1289,7 +1289,6 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl
                 mm.local().passive_announce(std::move(schema_version));
             });
             gossiper.local().wait_for_gossip_to_settle().get();
-            sst_format_selector.sync();
 
             with_scheduling_group(maintenance_scheduling_group, [&] {
                 return ss.local().join_cluster();