From 36f14f89dfca152ca2596e526753a96ee6a100d7 Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Sun, 21 Jan 2024 16:34:48 +0200 Subject: [PATCH] test/alternator: run some tests without tablets If an Alternator table uses tablets (we'll turn this on in a following patch), some tests are known to fail because of features not yet supported with tablets, namely: Refs #16317 - Support Alternator Streams with tablets (CDC) Refs #16567 - Support Alternator TTL with tablets This patch changes all tests failing on tablets due to one of these two known issues to explicitly ask to disable tablets when creating their test table. This means that at least we continue to test these two features (Streams and TTL) even if they don't yet work with tablets. We'll need to remember to remove this override when tablet support for CDC and Alternator TTL arrives. I left a comment in the right places in the code with the relevant issue numbers, to remind us what to change when we fix those issues. This patch also adds xfail_tablets and skip_tablets fixtures that can be used to xfail or skip tests when running with tablets - but we don't use them yet - and may never use them, but since I already wrote this code it won't hurt having it, just in case. When running without tablets, or against an older Scylla or on DynamoDB, the tests with these marks are run normally. Signed-off-by: Nadav Har'El --- test/alternator/conftest.py | 36 +++++++++++++++++++++++++++++++++ test/alternator/test_metrics.py | 5 +++++ test/alternator/test_streams.py | 11 ++++++++++ test/alternator/test_ttl.py | 20 ++++++++++++++++++ 4 files changed, 72 insertions(+) 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/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' },