alternator: do not use tablets on new Alternator tables

nyh · denesb · commit c24bc3b57a0f · 2024-04-04T12:11:29.000+03:00
A few months ago, in merge d3c1be9, we decided that if Scylla has the experimental "tablets" feature enabled, new Alternator tables should use this feature by default - exactly like this is the default for new CQL tables. Sadly, it was now decided to reverse this decision: We do not yet trust enough LWT on tablets, and since Alternator often (if not always) relies on LWT, we want Alternator tables to continue to use vnodes - not tablets. The fix is trivial - just changing the default. No test needed to change because anyway, all Alternator tests work correctly on Scylla with the tablets experimental feature disabled. I added a new test to enshrine the fact that Alternator does not use tablets. An unfortunate result of this patch will be that Alternator tables created on versions with this patch (e.g., Scylla 6.0) will not use tablets and will continue to not use tablets even if Scylla is upgraded (currently, the use of tablets is decided at table creation time, and there is no way to "upgrade" a vnode-based table to be tablet based). This patch should be reverted as soon as LWT support matures on tablets. Signed-off-by: Nadav Har'El <nyh@scylladb.com> Closes #18157
diff --git a/alternator/executor.cc b/alternator/executor.cc
@@ -4559,23 +4559,25 @@ static lw_shared_ptr<keyspace_metadata> create_keyspace_metadata(std::string_vie
     }
     auto opts = get_network_topology_options(sp, gossiper, rf);
 
-    // If the "tablets" experimental feature is available, we enable tablets
-    // by default on all Alternator tables. However, some Alternator features
-    // are not yet available with tablets (Streams #16313 and TTL #16567) so
-    // we allow disabling tablets at table-creation by supplying the following
-    // tags with any non-numeric value (e.g., empty string or the word "none").
-    // Supplying it with an integer value allows overriding the default choice
-    // of initial_tablets (setting the value to 0 asks for the default choice).
+    // Even if the "tablets" experimental feature is available, we currently
+    // do not enable tablets by default on Alternator tables because LWT is
+    // not yet fully supported with tablets.
+    // The user can override the choice of whether or not to use tablets at
+    // table-creation time by supplying the following tag with a numeric value
+    // (setting the value to 0 means enabling tablets with automatic selection
+    // of the best number of tablets).
+    // Setting this tag to any non-numeric value (e.g., an empty string or the
+    // word "none") will ask to disable tablets.
     // If we make this tag a permanent feature, it will get a "system:" prefix -
     // until then we give it the "experimental:" prefix to not commit to it.
     static constexpr auto INITIAL_TABLETS_TAG_KEY = "experimental:initial_tablets";
+    // initial_tablets currently defaults to unset, so tablets will not be
+    // used by default on new Alternator tables. Change this initialization
+    // to 0 enable tablets by default, with automatic number of tablets.
     std::optional<unsigned> initial_tablets;
     if (sp.get_db().local().get_config().check_experimental(db::experimental_features_t::feature::TABLETS)) {
         auto it = tags_map.find(INITIAL_TABLETS_TAG_KEY);
-        if (it == tags_map.end()) {
-            // No tag - ask to choose a reasonable default number of tablets
-            initial_tablets = 0;
-        } else {
+        if (it != tags_map.end()) {
             // Tag set. If it's a valid number, use it. If not - e.g., it's
             // empty or a word like "none", disable tablets by setting
             // initial_tablets to a disengaged optional.
diff --git a/test/alternator/test_table.py b/test/alternator/test_table.py
@@ -683,4 +683,12 @@ def test_delete_table_description_with_si(dynamodb):
     assert got_delete['TableName'] == table.name
     for i in ['KeySchema', 'AttributeDefinitions', 'GlobalSecondaryIndexes', 'LocalSecondaryIndexes']:
         assert i in got_describe
-        assert not i in got_delete
+        assert not i in got_delete
+
+# Currently, because of incomplete LWT support, Alternator tables do not use
+# tablets by default - even if the tablets experimental feature is enabled.
+# This test enshrines this fact - that an Alternator table doesn't use tablets.
+# This is a temporary test: When we reverse this decision and tablets go back
+# to being used by default on Alternator tables, this test should be deleted.
+def test_alternator_doesnt_use_tablets(dynamodb, has_tablets):
+    assert not has_tablets
