diff --git a/alternator/executor.cc b/alternator/executor.cc index d37fcddcf350..7a35f57360c8 100644 --- a/alternator/executor.cc +++ b/alternator/executor.cc @@ -60,6 +60,27 @@ logging::logger elogger("alternator-executor"); namespace alternator { +enum class table_status { + active = 0, + creating, + updating, + deleting +}; + +static sstring_view table_status_to_sstring(table_status tbl_status) { + switch(tbl_status) { + case table_status::active: + return "ACTIVE"; + case table_status::creating: + return "CREATING"; + case table_status::updating: + return "UPDATING"; + case table_status::deleting: + return "DELETING"; + } + return "UKNOWN"; +} + static future> create_keyspace(std::string_view keyspace_name, service::storage_proxy& sp, service::migration_manager& mm, gms::gossiper& gossiper, api::timestamp_type); static map_type attrs_type() { @@ -413,22 +434,8 @@ static rjson::value generate_arn_for_index(const schema& schema, std::string_vie schema.ks_name(), schema.cf_name(), index_name)); } -bool is_alternator_keyspace(const sstring& ks_name) { - return ks_name.find(executor::KEYSPACE_NAME_PREFIX) == 0; -} - -sstring executor::table_name(const schema& s) { - return s.cf_name(); -} - -future executor::describe_table(client_state& client_state, tracing::trace_state_ptr trace_state, service_permit permit, rjson::value request) { - _stats.api_operations.describe_table++; - elogger.trace("Describing table {}", request); - - schema_ptr schema = get_table(_proxy, request); - - tracing::add_table_name(trace_state, schema->ks_name(), schema->cf_name()); - +static rjson::value fill_table_description(schema_ptr schema, table_status tbl_status, service::storage_proxy const& proxy) +{ rjson::value table_description = rjson::empty_object(); rjson::add(table_description, "TableName", rjson::from_string(schema->cf_name())); // FIXME: take the tables creation time, not the current time! @@ -439,9 +446,8 @@ future executor::describe_table(client_state& cli // We don't currently do this in Alternator - instead CreateTable waits // until the table is really available. So/ DescribeTable returns either // ACTIVE or doesn't exist at all (and DescribeTable returns an error). - // The other states (CREATING, UPDATING, DELETING) are not currently - // returned. - rjson::add(table_description, "TableStatus", "ACTIVE"); + // The states CREATING and UPDATING are not currently returned. + rjson::add(table_description, "TableStatus", rjson::from_string(table_status_to_sstring(tbl_status))); rjson::add(table_description, "TableArn", generate_arn_for_table(*schema)); rjson::add(table_description, "TableId", rjson::from_string(schema->id().to_sstring())); // FIXME: Instead of hardcoding, we should take into account which mode was chosen @@ -458,9 +464,9 @@ future executor::describe_table(client_state& cli std::unordered_map key_attribute_types; // Add base table's KeySchema and collect types for AttributeDefinitions: - describe_key_schema(table_description, *schema, key_attribute_types); + executor::describe_key_schema(table_description, *schema, key_attribute_types); - data_dictionary::table t = _proxy.data_dictionary().find_column_family(schema); + data_dictionary::table t = proxy.data_dictionary().find_column_family(schema); if (!t.views().empty()) { rjson::value gsi_array = rjson::empty_array(); rjson::value lsi_array = rjson::empty_array(); @@ -476,7 +482,7 @@ future executor::describe_table(client_state& cli rjson::add(view_entry, "IndexName", rjson::from_string(index_name)); rjson::add(view_entry, "IndexArn", generate_arn_for_index(*schema, index_name)); // Add indexes's KeySchema and collect types for AttributeDefinitions: - describe_key_schema(view_entry, *vptr, key_attribute_types); + executor::describe_key_schema(view_entry, *vptr, key_attribute_types); // Add projection type rjson::value projection = rjson::empty_object(); rjson::add(projection, "ProjectionType", "ALL"); @@ -504,10 +510,29 @@ future executor::describe_table(client_state& cli } rjson::add(table_description, "AttributeDefinitions", std::move(attribute_definitions)); - supplement_table_stream_info(table_description, *schema, _proxy); - + executor::supplement_table_stream_info(table_description, *schema, proxy); + // FIXME: still missing some response fields (issue #5026) + return table_description; +} + +bool is_alternator_keyspace(const sstring& ks_name) { + return ks_name.find(executor::KEYSPACE_NAME_PREFIX) == 0; +} +sstring executor::table_name(const schema& s) { + return s.cf_name(); +} + +future executor::describe_table(client_state& client_state, tracing::trace_state_ptr trace_state, service_permit permit, rjson::value request) { + _stats.api_operations.describe_table++; + elogger.trace("Describing table {}", request); + + schema_ptr schema = get_table(_proxy, request); + + tracing::add_table_name(trace_state, schema->ks_name(), schema->cf_name()); + + rjson::value table_description = fill_table_description(schema, table_status::active, _proxy); rjson::value response = rjson::empty_object(); rjson::add(response, "Table", std::move(table_description)); elogger.trace("returning {}", response); @@ -523,6 +548,9 @@ future executor::delete_table(client_state& clien tracing::add_table_name(trace_state, keyspace_name, table_name); auto& p = _proxy.container(); + schema_ptr schema = get_table(_proxy, request); + rjson::value table_description = fill_table_description(schema, table_status::deleting, _proxy); + co_await _mm.container().invoke_on(0, [&] (service::migration_manager& mm) -> future<> { // FIXME: the following needs to be in a loop. If mm.announce() below // fails, we need to retry the whole thing. @@ -540,10 +568,6 @@ future executor::delete_table(client_state& clien co_await mm.announce(std::move(m), std::move(group0_guard)); }); - // FIXME: need more attributes? - rjson::value table_description = rjson::empty_object(); - rjson::add(table_description, "TableName", rjson::from_string(table_name)); - rjson::add(table_description, "TableStatus", "DELETING"); rjson::value response = rjson::empty_object(); rjson::add(response, "TableDescription", std::move(table_description)); elogger.trace("returning {}", response); diff --git a/alternator/executor.hh b/alternator/executor.hh index d3d32621fe59..590b4693043e 100644 --- a/alternator/executor.hh +++ b/alternator/executor.hh @@ -225,9 +225,10 @@ private: friend class rmw_operation; static void describe_key_schema(rjson::value& parent, const schema&, std::unordered_map * = nullptr); - static void describe_key_schema(rjson::value& parent, const schema& schema, std::unordered_map&); public: + static void describe_key_schema(rjson::value& parent, const schema& schema, std::unordered_map&); + static std::optional describe_single_item(schema_ptr, const query::partition_slice&, const cql3::selection::selection&, @@ -248,7 +249,7 @@ public: static void add_stream_options(const rjson::value& stream_spec, schema_builder&, service::storage_proxy& sp); static void supplement_table_info(rjson::value& descr, const schema& schema, service::storage_proxy& sp); - static void supplement_table_stream_info(rjson::value& descr, const schema& schema, service::storage_proxy& sp); + static void supplement_table_stream_info(rjson::value& descr, const schema& schema, const service::storage_proxy& sp); }; // is_big() checks approximately if the given JSON value is "bigger" than diff --git a/alternator/streams.cc b/alternator/streams.cc index 6ea781a65690..58051e042f49 100644 --- a/alternator/streams.cc +++ b/alternator/streams.cc @@ -1096,7 +1096,7 @@ void executor::add_stream_options(const rjson::value& stream_specification, sche } } -void executor::supplement_table_stream_info(rjson::value& descr, const schema& schema, service::storage_proxy& sp) { +void executor::supplement_table_stream_info(rjson::value& descr, const schema& schema, const service::storage_proxy& sp) { auto& opts = schema.cdc_options(); if (opts.enabled()) { auto db = sp.data_dictionary(); diff --git a/test/alternator/test_table.py b/test/alternator/test_table.py index b83f0d235149..54c5ffc946ad 100644 --- a/test/alternator/test_table.py +++ b/test/alternator/test_table.py @@ -11,7 +11,8 @@ import time import threading from botocore.exceptions import ClientError -from util import list_tables, unique_table_name, create_test_table, random_string, new_test_table +from util import list_tables, multiset, unique_table_name, create_test_table, random_string, new_test_table +from re import fullmatch # Utility function for create a table with a given name and some valid # schema.. This function initiates the table's creation, but doesn't @@ -552,3 +553,60 @@ def creates(): time.sleep(1) continue raise + +# Test that DeleteTable returns correct TableDescription (issue #11472) +def test_delete_table_description(dynamodb): + table_name = unique_table_name() + + table = create_table(dynamodb, table_name) + table.meta.client.get_waiter('table_exists').wait(TableName=table_name) + got = table.delete()['TableDescription'] + + assert got['TableName'] == table_name + assert got['TableStatus'] == 'DELETING' + + # as far as we still return incorrect CreationDateTime we check only field existence (issue #5013) + # when fix #5013 add CreationDateTime value checking here. + # Seems currently DynamoDB doesn't return CreationDateTime on DeleteTable so we disabled this assertion temporarily. + # Check when DynamoDB will support it later and re-enable the field's verification. + + # assert 'CreationDateTime' in got + + assert 'TableId' in got + assert fullmatch('[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}', got['TableId']) + + assert 'TableArn' in got and got['TableArn'].startswith('arn:') + + expected_schema = { + 'KeySchema': [ { 'AttributeName': 'p', 'KeyType': 'HASH' }, + { 'AttributeName': 'c', 'KeyType': 'RANGE' } + ], + 'AttributeDefinitions': [ + { 'AttributeName': 'p', 'AttributeType': 'S' }, + { 'AttributeName': 'c', 'AttributeType': 'S' }, + ] + } + assert got['KeySchema'] == expected_schema['KeySchema'] + # The list of attribute definitions may be arbitrarily reordered + assert multiset(got['AttributeDefinitions']) == multiset(expected_schema['AttributeDefinitions']) + + assert got['BillingModeSummary']['BillingMode'] == 'PAY_PER_REQUEST' + assert 'LastUpdateToPayPerRequestDateTime' in got['BillingModeSummary'] + assert got['BillingModeSummary']['LastUpdateToPayPerRequestDateTime'] == got['CreationDateTime'] + + assert got['ProvisionedThroughput']['NumberOfDecreasesToday'] == 0 + assert got['ProvisionedThroughput']['WriteCapacityUnits'] == 0 + assert got['ProvisionedThroughput']['ReadCapacityUnits'] == 0 + +# Test that DeleteTable returns correct TableDescription (issue #11472) and has no not implemented fields (see #5026 issue) +# after any field implementation move its testing code to the 'test_delete_table_description' test +@pytest.mark.xfail(reason="#5026: TableDescription still doesn't implement these fields") +def test_delete_table_description_missing_fields(dynamodb): + table_name = unique_table_name() + + table = create_table(dynamodb, table_name) + table.meta.client.get_waiter('table_exists').wait(TableName=table_name) + got = table.delete()['TableDescription'] + + assert 'TableSizeBytes' in got + assert 'ItemCount' in got