Skip to content

Commit d3c1be9

Browse files
denesbdenesb
committed
Merge 'alternator: enable tablets by default if experimental feature is enabled' from Nadav Har'El
This series does a similar change to Alternator as was done recently to CQL: 1. If the "tablets" experimental feature in enabled, new Alternator tables will use tablets automatically, without requiring an option on each new table. A default choice of initial_tablets is used. These choices can still be overridden per-table if the user wants to. 3. In particular, all test/alternator tests will also automatically run with tablets enabled 4. However, some tests will fail on tablets because they use features that haven't yet been implemented with tablets - namely Alternator Streams (Refs #16317) and Alternator TTL (Refs #16567). These tests will - until those features are implemented with tablets - continue to be run without tablets. 5. An option is added to the test/alternator/run to allow developers to manually run tests without tablets enabled, if they wish to (this option will be useful in the short term, and can be removed later). Fixes #16355 Closes #16900 * github.com:scylladb/scylladb: test/alternator: add "--vnodes" option to run script alternator: use tablets by default, if available test/alternator: run some tests without tablets
2 parents cb5453d + 4d6b286 commit d3c1be9

6 files changed

Lines changed: 108 additions & 9 deletions

File tree

‎alternator/executor.cc‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <seastar/core/sleep.hh>
1010
#include "alternator/executor.hh"
11+
#include "db/config.hh"
1112
#include "log.hh"
1213
#include "schema/schema_builder.hh"
1314
#include "exceptions/exceptions.hh"
@@ -4531,20 +4532,34 @@ static lw_shared_ptr<keyspace_metadata> create_keyspace_metadata(std::string_vie
45314532
keyspace_name, rf, endpoint_count);
45324533
}
45334534
auto opts = get_network_topology_options(sp, gossiper, rf);
4534-
std::optional<unsigned> initial_tablets;
45354535

4536-
// Tablets are not yet enabled by default on Alternator tables, because of
4537-
// missing support for CDC (see issue #16313). Until then, allow
4538-
// requesting tablets at table-creation time by supplying following tag,
4539-
// with an integer value. This is useful for testing Tablet support in
4540-
// Alternator even before it is ready for prime time.
4536+
// If the "tablets" experimental feature is available, we enable tablets
4537+
// by default on all Alternator tables. However, some Alternator features
4538+
// are not yet available with tablets (Streams #16313 and TTL #16567) so
4539+
// we allow disabling tablets at table-creation by supplying the following
4540+
// tags with any non-numeric value (e.g., empty string or the word "none").
4541+
// Supplying it with an integer value allows overriding the default choice
4542+
// of initial_tablets (setting the value to 0 asks for the default choice).
45414543
// If we make this tag a permanent feature, it will get a "system:" prefix -
45424544
// until then we give it the "experimental:" prefix to not commit to it.
45434545
static constexpr auto INITIAL_TABLETS_TAG_KEY = "experimental:initial_tablets";
4544-
if (tags_map.contains(INITIAL_TABLETS_TAG_KEY)) {
4545-
initial_tablets = std::stol(tags_map.at(INITIAL_TABLETS_TAG_KEY));
4546+
std::optional<unsigned> initial_tablets;
4547+
if (sp.get_db().local().get_config().check_experimental(db::experimental_features_t::feature::TABLETS)) {
4548+
auto it = tags_map.find(INITIAL_TABLETS_TAG_KEY);
4549+
if (it == tags_map.end()) {
4550+
// No tag - ask to choose a reasonable default number of tablets
4551+
initial_tablets = 0;
4552+
} else {
4553+
// Tag set. If it's a valid number, use it. If not - e.g., it's
4554+
// empty or a word like "none", disable tablets by setting
4555+
// initial_tablets to a disengaged optional.
4556+
try {
4557+
initial_tablets = std::stol(tags_map.at(INITIAL_TABLETS_TAG_KEY));
4558+
} catch(...) {
4559+
initial_tablets = std::nullopt;
4560+
}
4561+
}
45464562
}
4547-
45484563
return keyspace_metadata::new_keyspace(keyspace_name, "org.apache.cassandra.locator.NetworkTopologyStrategy", std::move(opts), initial_tablets);
45494564
}
45504565

‎test/alternator/conftest.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,3 +310,39 @@ def optional_rest_api(dynamodb):
310310
except:
311311
return None
312312
return url
313+
314+
# Fixture to check once whether newly created Alternator tables use the
315+
# tablet feature. It is used by the xfail_tablets and skip_tablets fixtures
316+
# below to xfail or skip a test which is known to be failing with tablets.
317+
# This is a temporary measure - eventually everything in Scylla should work
318+
# correctly with tablets, and these fixtures can be removed.
319+
@pytest.fixture(scope="session")
320+
def has_tablets(dynamodb, test_table):
321+
# We rely on some knowledge of Alternator internals:
322+
# 1. For table with name X, Scylla creates a keyspace called alternator_X
323+
# 2. We can read a CQL system table using the ".scylla.alternator." prefix.
324+
info = dynamodb.Table('.scylla.alternator.system_schema.scylla_keyspaces')
325+
try:
326+
response = info.query(
327+
KeyConditions={'keyspace_name': {
328+
'AttributeValueList': ['alternator_'+test_table.name],
329+
'ComparisonOperator': 'EQ'}})
330+
except dynamodb.meta.client.exceptions.ResourceNotFoundException:
331+
# The internal Scylla table doesn't even exist, either this isn't
332+
# Scylla or it's older Scylla and doesn't use tablets.
333+
return False
334+
if not 'Items' in response or not response['Items']:
335+
return False
336+
if 'initial_tablets' in response['Items'][0] and response['Items'][0]['initial_tablets']:
337+
return True
338+
return False
339+
340+
@pytest.fixture(scope="function")
341+
def xfail_tablets(request, has_tablets):
342+
if has_tablets:
343+
request.node.add_marker(pytest.mark.xfail(reason='Test expected to fail when Alternator tables use tablets'))
344+
345+
@pytest.fixture(scope="function")
346+
def skip_tablets(has_tablets):
347+
if has_tablets:
348+
pytest.skip("Test may crash when Alternator tables use tablets")

‎test/alternator/run‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
2424
print('Scylla is: ' + run.find_scylla() + '.')
2525

2626
extra_scylla_options = []
27+
remove_scylla_options = []
28+
29+
# If the "--vnodes" option is given, drop the "tablets" experimental
30+
# feature (turned on in run.py) so that all tests will be run with the
31+
# old vnode-based replication instead of tablets. This option only has
32+
# temporary usefulness, and should eventually be removed.
33+
if '--vnodes' in sys.argv:
34+
sys.argv.remove('--vnodes')
35+
remove_scylla_options.append('--experimental-features=tablets')
2736

2837
if "-h" in sys.argv or "--help" in sys.argv:
2938
run.run_pytest(sys.path[0], sys.argv)
@@ -60,6 +69,9 @@ def run_alternator_cmd(pid, dir):
6069
cmd += ['--alternator-port', '8000']
6170
cmd += extra_scylla_options
6271

72+
for i in remove_scylla_options:
73+
cmd.remove(i)
74+
6375
return (cmd, env)
6476

6577
pid = run.run_with_temporary_dir(run_alternator_cmd)

‎test/alternator/test_metrics.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,10 +262,15 @@ def alternator_ttl_period_in_seconds(dynamodb, request):
262262
# up to the setting of alternator_ttl_period_in_seconds. test/alternator/run
263263
# sets this to 1 second, which becomes the maximum delay of this test, but
264264
# if it is set higher we skip this test unless --runveryslow is enabled.
265+
# This test fails with tablets due to #16567, so to temporarily ensure that
266+
# Alternator TTL is still being tested, we use the following TAGS to
267+
# coerce Alternator to create the test table without tablets.
268+
TAGS = [{'Key': 'experimental:initial_tablets', 'Value': 'none'}]
265269
def test_ttl_stats(dynamodb, metrics, alternator_ttl_period_in_seconds):
266270
print(alternator_ttl_period_in_seconds)
267271
with check_increases_metric(metrics, ['scylla_expiration_scan_passes', 'scylla_expiration_scan_table', 'scylla_expiration_items_deleted']):
268272
with new_test_table(dynamodb,
273+
Tags = TAGS,
269274
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ],
270275
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table:
271276
# Insert one already-expired item, and then enable TTL:

‎test/alternator/test_streams.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@
1515
from urllib.error import URLError
1616
from boto3.dynamodb.types import TypeDeserializer
1717

18+
# All tests in this file are expected to fail with tablets due to #16317.
19+
# To ensure that Alternator Streams is still being tested, instead of
20+
# xfailing these tests, we temporarily coerce the tests below to avoid
21+
# using default tablets setting, even if it's available. We do this by
22+
# using the following tags when creating each table below:
23+
TAGS = [{'Key': 'experimental:initial_tablets', 'Value': 'none'}]
24+
1825
stream_types = [ 'OLD_IMAGE', 'NEW_IMAGE', 'KEYS_ONLY', 'NEW_AND_OLD_IMAGES']
1926

2027
def disable_stream(dynamodbstreams, table):
@@ -51,6 +58,7 @@ def create_stream_test_table(dynamodb, StreamViewType=None):
5158
if StreamViewType != None:
5259
spec = {'StreamEnabled': True, 'StreamViewType': StreamViewType}
5360
table = create_test_table(dynamodb, StreamSpecification=spec,
61+
Tags=TAGS,
5462
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' },
5563
{ 'AttributeName': 'c', 'KeyType': 'RANGE' }
5664
],
@@ -475,6 +483,7 @@ def test_get_records_nonexistent_iterator(dynamodbstreams):
475483

476484
def create_table_ss(dynamodb, dynamodbstreams, type):
477485
table = create_test_table(dynamodb,
486+
Tags=TAGS,
478487
KeySchema=[{ 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' }],
479488
AttributeDefinitions=[{ 'AttributeName': 'p', 'AttributeType': 'S' }, { 'AttributeName': 'c', 'AttributeType': 'S' }],
480489
StreamSpecification={ 'StreamEnabled': True, 'StreamViewType': type })
@@ -807,6 +816,7 @@ def do_updates(table, p, c):
807816
@pytest.fixture(scope="function")
808817
def test_table_ss_old_image_and_lsi(dynamodb, dynamodbstreams):
809818
table = create_test_table(dynamodb,
819+
Tags=TAGS,
810820
KeySchema=[
811821
{'AttributeName': 'p', 'KeyType': 'HASH'},
812822
{'AttributeName': 'c', 'KeyType': 'RANGE'}],
@@ -1281,6 +1291,7 @@ def test_streams_1_new_and_old_images(test_table_ss_new_and_old_images, dynamodb
12811291
def test_table_stream_with_result(dynamodb, dynamodbstreams):
12821292
tablename = unique_table_name()
12831293
result = dynamodb.meta.client.create_table(TableName=tablename,
1294+
Tags=TAGS,
12841295
BillingMode='PAY_PER_REQUEST',
12851296
StreamSpecification={'StreamEnabled': True, 'StreamViewType': 'KEYS_ONLY'},
12861297
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' },

‎test/alternator/test_ttl.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@
1313
from contextlib import contextmanager
1414
from decimal import Decimal
1515

16+
# All tests in this file are expected to fail with tablets due to #16567.
17+
# To ensure that Alternator TTL is still being tested, instead of
18+
# xfailing these tests, we temporarily coerce the tests below to avoid
19+
# using default tablets setting, even if it's available. We do this by
20+
# using the following tags when creating each table below:
21+
TAGS = [{'Key': 'experimental:initial_tablets', 'Value': 'none'}]
22+
1623
# passes_or_raises() is similar to pytest.raises(), except that while raises()
1724
# expects a certain exception must happen, the new passes_or_raises()
1825
# 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):
8592
# and this information becomes available via DescribeTimeToLive
8693
def test_ttl_enable(dynamodb):
8794
with new_test_table(dynamodb,
95+
Tags=TAGS,
8896
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ],
8997
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table:
9098
client = table.meta.client
@@ -120,6 +128,7 @@ def test_ttl_enable(dynamodb):
120128
# disable case.
121129
def test_ttl_disable_errors(dynamodb):
122130
with new_test_table(dynamodb,
131+
Tags=TAGS,
123132
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ],
124133
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table:
125134
client = table.meta.client
@@ -152,6 +161,7 @@ def test_ttl_disable_errors(dynamodb):
152161
# minutes). But on Scylla it is currently almost instantaneous.
153162
def test_ttl_disable(dynamodb, veryslow_on_aws):
154163
with new_test_table(dynamodb,
164+
Tags=TAGS,
155165
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ],
156166
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table:
157167
client = table.meta.client
@@ -185,6 +195,7 @@ def test_update_ttl_errors(dynamodb):
185195
client.update_time_to_live(TableName=nonexistent_table,
186196
TimeToLiveSpecification={'AttributeName': 'expiration', 'Enabled': True})
187197
with new_test_table(dynamodb,
198+
Tags=TAGS,
188199
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ],
189200
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table:
190201
# AttributeName must be between 1 and 255 characters long.
@@ -232,6 +243,7 @@ def test_ttl_expiration(dynamodb):
232243
delta = math.ceil(duration / 4)
233244
assert delta >= 1
234245
with new_test_table(dynamodb,
246+
Tags=TAGS,
235247
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ],
236248
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table:
237249
# 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):
354366
max_duration = 1200 if is_aws(dynamodb) else 240
355367
sleep = 30 if is_aws(dynamodb) else 0.1
356368
with new_test_table(dynamodb,
369+
Tags=TAGS,
357370
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' },
358371
{ 'AttributeName': 'c', 'KeyType': 'RANGE' } ],
359372
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' },
@@ -392,6 +405,7 @@ def test_ttl_expiration_hash(dynamodb, waits_for_expiration):
392405
max_duration = 1200 if is_aws(dynamodb) else 240
393406
sleep = 30 if is_aws(dynamodb) else 0.1
394407
with new_test_table(dynamodb,
408+
Tags=TAGS,
395409
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ],
396410
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'N' } ]) as table:
397411
ttl_spec = {'AttributeName': 'p', 'Enabled': True}
@@ -424,6 +438,7 @@ def test_ttl_expiration_range(dynamodb, waits_for_expiration):
424438
max_duration = 1200 if is_aws(dynamodb) else 240
425439
sleep = 30 if is_aws(dynamodb) else 0.1
426440
with new_test_table(dynamodb,
441+
Tags=TAGS,
427442
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, { 'AttributeName': 'c', 'KeyType': 'RANGE' } ],
428443
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' }, { 'AttributeName': 'c', 'AttributeType': 'N' } ]) as table:
429444
ttl_spec = {'AttributeName': 'c', 'Enabled': True}
@@ -463,6 +478,7 @@ def check_expired():
463478
def test_ttl_expiration_hash_wrong_type(dynamodb):
464479
duration = 900 if is_aws(dynamodb) else 3
465480
with new_test_table(dynamodb,
481+
Tags=TAGS,
466482
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' }, ],
467483
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' } ]) as table:
468484
ttl_spec = {'AttributeName': 'p', 'Enabled': True}
@@ -496,6 +512,7 @@ def test_ttl_expiration_gsi_lsi(dynamodb, waits_for_expiration):
496512
max_duration = 3600 if is_aws(dynamodb) else 240
497513
sleep = 30 if is_aws(dynamodb) else 0.1
498514
with new_test_table(dynamodb,
515+
Tags=TAGS,
499516
KeySchema=[
500517
{ 'AttributeName': 'p', 'KeyType': 'HASH' },
501518
{ 'AttributeName': 'c', 'KeyType': 'RANGE' },
@@ -589,6 +606,7 @@ def test_ttl_expiration_lsi_key(dynamodb, waits_for_expiration):
589606
max_duration = 3600 if is_aws(dynamodb) else 240
590607
sleep = 30 if is_aws(dynamodb) else 0.1
591608
with new_test_table(dynamodb,
609+
Tags=TAGS,
592610
KeySchema=[
593611
{ 'AttributeName': 'p', 'KeyType': 'HASH' },
594612
{ 'AttributeName': 'c', 'KeyType': 'RANGE' },
@@ -644,6 +662,7 @@ def test_ttl_expiration_streams(dynamodb, dynamodbstreams):
644662
# max_duration, we report a failure.
645663
max_duration = 3600 if is_aws(dynamodb) else 10
646664
with new_test_table(dynamodb,
665+
Tags=TAGS,
647666
KeySchema=[
648667
{ 'AttributeName': 'p', 'KeyType': 'HASH' },
649668
{ 'AttributeName': 'c', 'KeyType': 'RANGE' },
@@ -756,6 +775,7 @@ def test_ttl_expiration_long(dynamodb, waits_for_expiration):
756775
N=400
757776
max_duration = 1200 if is_aws(dynamodb) else 15
758777
with new_test_table(dynamodb,
778+
Tags=TAGS,
759779
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' },
760780
{ 'AttributeName': 'c', 'KeyType': 'RANGE' } ],
761781
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'N' },

0 commit comments

Comments
 (0)