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 scylladb/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
This commit is contained in:
Botond Dénes
2024-01-29 09:22:13 +02:00
6 changed files with 108 additions and 9 deletions

View File

@@ -8,6 +8,7 @@
#include <seastar/core/sleep.hh>
#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<keyspace_metadata> create_keyspace_metadata(std::string_vie
keyspace_name, rf, endpoint_count);
}
auto opts = get_network_topology_options(sp, gossiper, rf);
std::optional<unsigned> 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<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 {
// 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);
}

View File

@@ -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")

View File

@@ -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)

View File

@@ -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:

View File

@@ -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' },

View File

@@ -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' },