bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

alternator: exclude CDC log table from ListTables

Browse Source 
The Alternator command ListTables is supposed to list actual tables
created with CreateTable, and should list things like materialized views
(created for GSI or LSI) or CDC log tables.

We already properly excluded materialized views from the list - and
had the tests to prove it - but forgot both the exclusion and the testing
for CDC log tables - so creating a table xyz with streams enable would
cause ListTables to also list "xyz_scylla_cdc_log".

This patch fixes both oversights: It adds the code to exclude CDC logs
from the output of ListTables, add adds a test which reproduces the bug
before this fix, and verifies the fix works.

Fixes #19911.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
(cherry picked from commit d293a5787f)

Closes scylladb/scylladb#19938
This commit is contained in:
Nadav Har'El
2024-07-28 21:18:12 +03:00
committed by Botond Dénes
parent c75dbc1f9c
commit 753fc87efa
2 changed files with 26 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
alternator/executor.cc
View File

@@ -9,6 +9,7 @@
#include <fmt/ranges.h>
#include <seastar/core/sleep.hh>
#include "alternator/executor.hh"
#include "cdc/log.hh"
#include "db/config.hh"
#include "log.hh"
#include "schema/schema_builder.hh"
@@ -4439,8 +4440,10 @@ future<executor::request_return_type> executor::list_tables(client_state& client
auto tables = _proxy.data_dictionary().get_tables(); // hold on to temporary, table_names isn't a container, it's a view
auto table_names = tables
| boost::adaptors::filtered([] (data_dictionary::table t) {
return t.schema()->ks_name().find(KEYSPACE_NAME_PREFIX) == 0 && !t.schema()->is_view();
| boost::adaptors::filtered([this] (data_dictionary::table t) {
return t.schema()->ks_name().find(KEYSPACE_NAME_PREFIX) == 0 &&
!t.schema()->is_view() &&
!cdc::is_log_for_some_table(_proxy.local_db(), t.schema()->ks_name(), t.schema()->cf_name());
})
| boost::adaptors::transformed([] (data_dictionary::table t) {
return t.schema()->cf_name();

22
test/alternator/test_streams.py
View File

@@ -14,7 +14,7 @@ import pytest
from boto3.dynamodb.types import TypeDeserializer
from botocore.exceptions import ClientError
from test.alternator.util import unique_table_name, create_test_table, new_test_table, random_string, freeze
from test.alternator.util import unique_table_name, create_test_table, new_test_table, random_string, freeze, list_tables
# 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
@@ -1581,6 +1581,26 @@ def test_stream_arn_unchanging(dynamodb, dynamodbstreams):
assert len(streams['Streams']) == 1
assert streams['Streams'][0]['StreamArn'] == arn
# Enabling a stream shouldn't cause any extra table to appear in ListTables.
# In issue #19911, enabling streams on a table called xyz caused the name
# "xyz_scylla_cdc_log" to appear in ListTables. The following test creates
# a table with a long unique name, and ensures that only one table containing
# this name as a substring is listed.
# In test_gsi.py and test_lsi.py we have similar tests for GSI and LSI.
# Reproduces #19911
def test_stream_list_tables(dynamodb):
with new_test_table(dynamodb,
Tags=TAGS,
StreamSpecification={'StreamEnabled': True, 'StreamViewType': 'KEYS_ONLY'},
KeySchema=[ { 'AttributeName': 'p', 'KeyType': 'HASH' } ],
AttributeDefinitions=[ { 'AttributeName': 'p', 'AttributeType': 'S' }, ]
) as table:
# Check that the long and unique table name (created by
# unique_table_name()) isn't a substring of any table name,
# except of course the table itself:
for listed_name in list_tables(dynamodb):
assert table.name == listed_name or table.name not in listed_name
# TODO: tests on multiple partitions
# TODO: write a test that disabling the stream and re-enabling it works, but
# requires the user to wait for the first stream to become DISABLED before