From 33a0366ed87d39acc4abec8d8a08c75e3b6ab83f Mon Sep 17 00:00:00 2001
From: Avi Kivity <avi@scylladb.com>
Date: Mon, 3 Dec 2018 00:02:36 +0200
Subject: [PATCH] storage_service: fix gap when feature::when_enabled() doesn't
 work

storage_service::register_features() reassigns to feature variables in
storage_service. This means that any call to feature::when_enabled() will be
orphaned when the feature is assigned.

Now that feature lifetimes are not tied to gossip, we can move the feature
initialization to the constructor and eliminate the gap. When gossip is started
it will evaluate application_states and enable features that the cluster agrees on.
---
 service/storage_service.cc | 36 +++++++++++++++---------------------
 service/storage_service.hh |  1 -
 2 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/service/storage_service.cc b/service/storage_service.cc
index f289a7cd19..d65f0cdffb 100644
--- a/service/storage_service.cc
+++ b/service/storage_service.cc
@@ -129,6 +129,21 @@ storage_service::storage_service(distributed<database>& db, sharded<auth::servic
         : _feature_service(feature_service)
         , _db(db)
         , _auth_service(auth_service)
+        , _range_tombstones_feature(_feature_service, RANGE_TOMBSTONES_FEATURE)
+        , _large_partitions_feature(_feature_service, LARGE_PARTITIONS_FEATURE)
+        , _materialized_views_feature(_feature_service, MATERIALIZED_VIEWS_FEATURE)
+        , _counters_feature(_feature_service, COUNTERS_FEATURE)
+        , _indexes_feature(_feature_service, INDEXES_FEATURE)
+        , _digest_multipartition_read_feature(_feature_service, DIGEST_MULTIPARTITION_READ_FEATURE)
+        , _correct_counter_order_feature(_feature_service, CORRECT_COUNTER_ORDER_FEATURE)
+        , _schema_tables_v3(_feature_service, SCHEMA_TABLES_V3)
+        , _correct_non_compound_range_tombstones(_feature_service, CORRECT_NON_COMPOUND_RANGE_TOMBSTONES)
+        , _write_failure_reply_feature(_feature_service, WRITE_FAILURE_REPLY_FEATURE)
+        , _xxhash_feature(_feature_service, XXHASH_FEATURE)
+        , _roles_feature(_feature_service, ROLES_FEATURE)
+        , _la_sstable_feature(_feature_service, LA_SSTABLE_FEATURE)
+        , _stream_with_rpc_stream_feature(_feature_service, STREAM_WITH_RPC_STREAM)
+        , _mc_sstable_feature(_feature_service, MC_SSTABLE_FEATURE)
         , _replicate_action([this] { return do_replicate_to_all_cores(); })
         , _update_pending_ranges_action([this] { return do_update_pending_ranges(); })
         , _sys_dist_ks(sys_dist_ks) {
@@ -426,9 +441,6 @@ void storage_service::prepare_to_join(std::vector<inet_address> loaded_endpoints
     auto& proxy = service::get_storage_proxy();
     // gossip Schema.emptyVersion forcing immediate check for schema updates (see MigrationManager#maybeScheduleSchemaPull)
     update_schema_version_and_announce(proxy).get();// Ensure we know our own actual Schema UUID in preparation for updates
-    get_storage_service().invoke_on_all([] (auto& ss) {
-        ss.register_features();
-    }).get();
 #if 0
     if (!MessagingService.instance().isListening())
         MessagingService.instance().listen(FBUtilities.getLocalAddress());
@@ -439,24 +451,6 @@ void storage_service::prepare_to_join(std::vector<inet_address> loaded_endpoints
 #endif
 }
 
-void storage_service::register_features() {
-    _range_tombstones_feature = gms::feature(_feature_service, RANGE_TOMBSTONES_FEATURE);
-    _large_partitions_feature = gms::feature(_feature_service, LARGE_PARTITIONS_FEATURE);
-    _counters_feature = gms::feature(_feature_service, COUNTERS_FEATURE);
-    _digest_multipartition_read_feature = gms::feature(_feature_service, DIGEST_MULTIPARTITION_READ_FEATURE);
-    _correct_counter_order_feature = gms::feature(_feature_service, CORRECT_COUNTER_ORDER_FEATURE);
-    _schema_tables_v3 = gms::feature(_feature_service, SCHEMA_TABLES_V3);
-    _correct_non_compound_range_tombstones = gms::feature(_feature_service, CORRECT_NON_COMPOUND_RANGE_TOMBSTONES);
-    _write_failure_reply_feature = gms::feature(_feature_service, WRITE_FAILURE_REPLY_FEATURE);
-    _xxhash_feature = gms::feature(_feature_service, XXHASH_FEATURE);
-    _roles_feature = gms::feature(_feature_service, ROLES_FEATURE);
-    _la_sstable_feature = gms::feature(_feature_service, LA_SSTABLE_FEATURE);
-    _stream_with_rpc_stream_feature = gms::feature(_feature_service, STREAM_WITH_RPC_STREAM);
-    _mc_sstable_feature = gms::feature(_feature_service, MC_SSTABLE_FEATURE);
-    _materialized_views_feature = gms::feature(_feature_service, MATERIALIZED_VIEWS_FEATURE);
-    _indexes_feature = gms::feature(_feature_service, INDEXES_FEATURE);
-}
-
 // Runs inside seastar::async context
 void storage_service::join_token_ring(int delay) {
     // This function only gets called on shard 0, but we want to set _joined
diff --git a/service/storage_service.hh b/service/storage_service.hh
index 742836c857..82d269aa53 100644
--- a/service/storage_service.hh
+++ b/service/storage_service.hh
@@ -454,7 +454,6 @@ public:
 private:
     bool should_bootstrap();
     void prepare_to_join(std::vector<inet_address> loaded_endpoints, bind_messaging_port do_bind = bind_messaging_port::yes);
-    void register_features();
     void join_token_ring(int delay);
 public:
     future<> join_ring();