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 scylladb#16317) and Alternator TTL (Refs scylladb#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 scylladb#16355 Closes scylladb#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)