diff --git a/alternator/executor.cc b/alternator/executor.cc index 90bb570913..0217773ef6 100644 --- a/alternator/executor.cc +++ b/alternator/executor.cc @@ -8,6 +8,7 @@ #include #include "alternator/executor.hh" +#include "db/config.hh" #include "log.hh" #include "schema/schema_builder.hh" #include "exceptions/exceptions.hh" @@ -4531,20 +4532,34 @@ static lw_shared_ptr create_keyspace_metadata(std::string_vie keyspace_name, rf, endpoint_count); } auto opts = get_network_topology_options(sp, gossiper, rf); - std::optional initial_tablets; - // Tablets are not yet enabled by default on Alternator tables, because of - // missing support for CDC (see issue #16313). Until then, allow - // requesting tablets at table-creation time by supplying following tag, - // with an integer value. This is useful for testing Tablet support in - // Alternator even before it is ready for prime time. + // 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). // 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"; - if (tags_map.contains(INITIAL_TABLETS_TAG_KEY)) { - initial_tablets = std::stol(tags_map.at(INITIAL_TABLETS_TAG_KEY)); + std::optional 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 { + // 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. + try { + initial_tablets = std::stol(tags_map.at(INITIAL_TABLETS_TAG_KEY)); + } catch(...) { + initial_tablets = std::nullopt; + } + } } - return keyspace_metadata::new_keyspace(keyspace_name, "org.apache.cassandra.locator.NetworkTopologyStrategy", std::move(opts), initial_tablets); } diff --git a/test/alternator/conftest.py b/test/alternator/conftest.py index 25e6019ca2..19c663e2c9 100644 --- a/test/alternator/conftest.py +++ b/test/alternator/conftest.py @@ -310,3 +310,39 @@ def optional_rest_api(dynamodb): except: return None return url + +# Fixture to check once whether newly created Alternator tables use the +# tablet feature. It is used by the xfail_tablets and skip_tablets fixtures +# below to xfail or skip a test which is known to be failing with tablets. +# This is a temporary measure - eventually everything in Scylla should work +# correctly with tablets, and these fixtures can be removed. +@pytest.fixture(scope="session") +def has_tablets(dynamodb, test_table): + # We rely on some knowledge of Alternator internals: + # 1. For table with name X, Scylla creates a keyspace called alternator_X + # 2. We can read a CQL system table using the ".scylla.alternator." prefix. + info = dynamodb.Table('.scylla.alternator.system_schema.scylla_keyspaces') + try: + response = info.query( + KeyConditions={'keyspace_name': { + 'AttributeValueList': ['alternator_'+test_table.name], + 'ComparisonOperator': 'EQ'}}) + except dynamodb.meta.client.exceptions.ResourceNotFoundException: + # The internal Scylla table doesn't even exist, either this isn't + # Scylla or it's older Scylla and doesn't use tablets. + return False + if not 'Items' in response or not response['Items']: + return False + if 'initial_tablets' in response['Items'][0] and response['Items'][0]['initial_tablets']: + return True + return False + +@pytest.fixture(scope="function") +def xfail_tablets(request, has_tablets): + if has_tablets: + request.node.add_marker(pytest.mark.xfail(reason='Test expected to fail when Alternator tables use tablets')) + +@pytest.fixture(scope="function") +def skip_tablets(has_tablets): + if has_tablets: + pytest.skip("Test may crash when Alternator tables use tablets") diff --git a/test/alternator/run b/test/alternator/run index e0d6f1ad17..707494db3b 100755 --- a/test/alternator/run +++ b/test/alternator/run @@ -24,6 +24,15 @@ urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) print('Scylla is: ' + run.find_scylla() + '.') extra_scylla_options = [] +remove_scylla_options = [] + +# If the "--vnodes" option is given, drop the "tablets" experimental +# feature (turned on in run.py) so that all tests will be run with the +# old vnode-based replication instead of tablets. This option only has +# temporary usefulness, and should eventually be removed. +if '--vnodes' in sys.argv: + sys.argv.remove('--vnodes') + remove_scylla_options.append('--experimental-features=tablets') if "-h" in sys.argv or "--help" in sys.argv: run.run_pytest(sys.path[0], sys.argv) @@ -60,6 +69,9 @@ def run_alternator_cmd(pid, dir): cmd += ['--alternator-port', '8000'] cmd += extra_scylla_options + for i in remove_scylla_options: + cmd.remove(i) + return (cmd, env) pid = run.run_with_temporary_dir(run_alternator_cmd) diff --git a/test/alternator/test_metrics.py b/test/alternator/test_metrics.py index 3e9fb5727e..5845fb3ec8 100644 --- a/test/alternator/test_metrics.py +++ b/test/alternator/test_metrics.py @@ -262,10 +262,15 @@ def alternator_ttl_period_in_seconds(dynamodb, request): # up to the setting of alternator_ttl_period_in_seconds. test/alternator/run # sets this to 1 second, which becomes the maximum delay of this test, but # if it is set higher we skip this test unless --runveryslow is enabled. +# This test fails with tablets due to #16567, so to temporarily ensure that +# Alternator TTL is still being tested, we use the following TAGS to +# coerce Alternator to create the test table without tablets. +TAGS = [{'Key': 'experimental:initial_tablets', 'Value': 'none'}] def test_ttl_stats(dynamodb, metrics, alternator_ttl_period_in_seconds): print(alternator_ttl_period_in_seconds) with check_increases_metric(metrics, ['scylla_expiration_scan_passes', 'scylla_expiration_scan_table', 'scylla_expiration_items_deleted']): with new_test_table(dynamodb, + Tags = TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table: # Insert one already-expired item, and then enable TTL: diff --git a/test/alternator/test_streams.py b/test/alternator/test_streams.py index a8ae03b26b..6948a94b0a 100644 --- a/test/alternator/test_streams.py +++ b/test/alternator/test_streams.py @@ -15,6 +15,13 @@ from contextlib import contextmanager, ExitStack from urllib.error import URLError from boto3.dynamodb.types import TypeDeserializer +# All tests in this file are expected to fail with tablets due to #16317. +# To ensure that Alternator Streams is still being tested, instead of +# xfailing these tests, we temporarily coerce the tests below to avoid +# using default tablets setting, even if it's available. We do this by +# using the following tags when creating each table below: +TAGS = [{'Key': 'experimental:initial_tablets', 'Value': 'none'}] + stream_types = [ 'OLD_IMAGE', 'NEW_IMAGE', 'KEYS_ONLY', 'NEW_AND_OLD_IMAGES'] def disable_stream(dynamodbstreams, table): @@ -51,6 +58,7 @@ def create_stream_test_table(dynamodb, StreamViewType=None): if StreamViewType != None: spec = {'StreamEnabled': True, 'StreamViewType': StreamViewType} table = create_test_table(dynamodb, StreamSpecification=spec, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' } ], @@ -475,6 +483,7 @@ def test_get_records_nonexistent_iterator(dynamodbstreams): def create_table_ss(dynamodb, dynamodbstreams, type): table = create_test_table(dynamodb, + Tags=TAGS, KeySchema=[{ 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' }], AttributeDefinitions=[{ 'AttributeName': 'p', 'AttributeType': 'S' }, { 'AttributeName': 'c', 'AttributeType': 'S' }], StreamSpecification={ 'StreamEnabled': True, 'StreamViewType': type }) @@ -807,6 +816,7 @@ def test_streams_updateitem_old_image_empty_item(test_table_ss_old_image, dynamo @pytest.fixture(scope="function") def test_table_ss_old_image_and_lsi(dynamodb, dynamodbstreams): table = create_test_table(dynamodb, + Tags=TAGS, KeySchema=[ {'AttributeName': 'p', 'KeyType': 'HASH'}, {'AttributeName': 'c', 'KeyType': 'RANGE'}], @@ -1281,6 +1291,7 @@ def test_streams_1_new_and_old_images(test_table_ss_new_and_old_images, dynamodb def test_table_stream_with_result(dynamodb, dynamodbstreams): tablename = unique_table_name() result = dynamodb.meta.client.create_table(TableName=tablename, + Tags=TAGS, BillingMode='PAY_PER_REQUEST', StreamSpecification={'StreamEnabled': True, 'StreamViewType': 'KEYS_ONLY'}, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, diff --git a/test/alternator/test_ttl.py b/test/alternator/test_ttl.py index 93b637c9a3..ad60167a75 100644 --- a/test/alternator/test_ttl.py +++ b/test/alternator/test_ttl.py @@ -13,6 +13,13 @@ from util import new_test_table, random_string, full_query, unique_table_name, i from contextlib import contextmanager from decimal import Decimal +# All tests in this file are expected to fail with tablets due to #16567. +# To ensure that Alternator TTL is still being tested, instead of +# xfailing these tests, we temporarily coerce the tests below to avoid +# using default tablets setting, even if it's available. We do this by +# using the following tags when creating each table below: +TAGS = [{'Key': 'experimental:initial_tablets', 'Value': 'none'}] + # passes_or_raises() is similar to pytest.raises(), except that while raises() # expects a certain exception must happen, the new passes_or_raises() # expects the code to either pass (not raise), or if it throws, it must @@ -85,6 +92,7 @@ def test_describe_ttl_without_ttl(test_table): # and this information becomes available via DescribeTimeToLive def test_ttl_enable(dynamodb): with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table: client = table.meta.client @@ -120,6 +128,7 @@ def test_ttl_enable(dynamodb): # disable case. def test_ttl_disable_errors(dynamodb): with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table: client = table.meta.client @@ -152,6 +161,7 @@ def test_ttl_disable_errors(dynamodb): # minutes). But on Scylla it is currently almost instantaneous. def test_ttl_disable(dynamodb, veryslow_on_aws): with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table: client = table.meta.client @@ -185,6 +195,7 @@ def test_update_ttl_errors(dynamodb): client.update_time_to_live(TableName=nonexistent_table, TimeToLiveSpecification={'AttributeName': 'expiration', 'Enabled': True}) with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table: # AttributeName must be between 1 and 255 characters long. @@ -232,6 +243,7 @@ def test_ttl_expiration(dynamodb): delta = math.ceil(duration / 4) assert delta >= 1 with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table: # Insert one expiring item *before* enabling the TTL, to verify that @@ -354,6 +366,7 @@ def test_ttl_expiration_with_rangekey(dynamodb, waits_for_expiration): max_duration = 1200 if is_aws(dynamodb) else 240 sleep = 30 if is_aws(dynamodb) else 0.1 with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' } ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' }, @@ -392,6 +405,7 @@ def test_ttl_expiration_hash(dynamodb, waits_for_expiration): max_duration = 1200 if is_aws(dynamodb) else 240 sleep = 30 if is_aws(dynamodb) else 0.1 with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'N' } ]) as table: ttl_spec = {'AttributeName': 'p', 'Enabled': True} @@ -424,6 +438,7 @@ def test_ttl_expiration_range(dynamodb, waits_for_expiration): max_duration = 1200 if is_aws(dynamodb) else 240 sleep = 30 if is_aws(dynamodb) else 0.1 with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' } ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' }, { 'AttributeName': 'c', 'AttributeType': 'N' } ]) as table: ttl_spec = {'AttributeName': 'c', 'Enabled': True} @@ -463,6 +478,7 @@ def test_ttl_expiration_range(dynamodb, waits_for_expiration): def test_ttl_expiration_hash_wrong_type(dynamodb): duration = 900 if is_aws(dynamodb) else 3 with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table: ttl_spec = {'AttributeName': 'p', 'Enabled': True} @@ -496,6 +512,7 @@ def test_ttl_expiration_gsi_lsi(dynamodb, waits_for_expiration): max_duration = 3600 if is_aws(dynamodb) else 240 sleep = 30 if is_aws(dynamodb) else 0.1 with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' }, @@ -589,6 +606,7 @@ def test_ttl_expiration_lsi_key(dynamodb, waits_for_expiration): max_duration = 3600 if is_aws(dynamodb) else 240 sleep = 30 if is_aws(dynamodb) else 0.1 with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' }, @@ -644,6 +662,7 @@ def test_ttl_expiration_streams(dynamodb, dynamodbstreams): # max_duration, we report a failure. max_duration = 3600 if is_aws(dynamodb) else 10 with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' }, @@ -756,6 +775,7 @@ def test_ttl_expiration_long(dynamodb, waits_for_expiration): N=400 max_duration = 1200 if is_aws(dynamodb) else 15 with new_test_table(dynamodb, + Tags=TAGS, KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' } ], AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'N' },