This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 536ccda [master] introduce filter_type for ListTables
536ccda is described below
commit 536ccdaac556bdfa0447915275ed8458f525b389
Author: Alexey Serbin <[email protected]>
AuthorDate: Wed Oct 7 14:33:26 2020 -0700
[master] introduce filter_type for ListTables
This patch introduces a new 'filter_type' field for the
ListTablesRequestPB message. With this patch, it's possible
to explicitly filter tables to be returned from the system catalog.
I added corresponding unit test as well.
This patch doesn't update kudu CLI -- this is to be done
in a separate patch.
Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Reviewed-on: http://gerrit.cloudera.org:8080/16561
Reviewed-by: Andrew Wong <[email protected]>
Tested-by: Kudu Jenkins
---
src/kudu/master/catalog_manager.cc | 21 +++++--
src/kudu/master/master-test.cc | 122 +++++++++++++++++++++++++++++++++++--
src/kudu/master/master.proto | 5 ++
3 files changed, 138 insertions(+), 10 deletions(-)
diff --git a/src/kudu/master/catalog_manager.cc
b/src/kudu/master/catalog_manager.cc
index 9475fa4..20b358c 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -3153,15 +3153,28 @@ Status CatalogManager::ListTables(const
ListTablesRequestPB* req,
tables_info.emplace_back(entry.second);
}
}
+ unordered_set<int> table_types;
+ for (auto idx = 0; idx < req->type_filter_size(); ++idx) {
+ table_types.emplace(req->type_filter(idx));
+ }
+ if (table_types.empty()) {
+ // The default behavior is to list only user tables (that's backwards
+ // compatible).
+ table_types.emplace(TableTypePB::DEFAULT_TABLE);
+ }
unordered_map<string, scoped_refptr<TableInfo>> table_info_by_name;
unordered_map<string, bool> table_owner_map;
for (const auto& table_info : tables_info) {
TableMetadataLock ltm(table_info.get(), LockMode::READ);
const auto& table_data = ltm.data();
- // Don't list system tables or tables that aren't running
- // TODO(awong): consider allow for opting into listing system tables.
- if (!table_data.is_running() ||
- table_data.pb.table_type() != TableTypePB::DEFAULT_TABLE) {
+ // Don't list tables that aren't running
+ if (!table_data.is_running()) {
+ continue;
+ }
+ // The table type might be unset in the data stored in the system catalog.
+ const auto table_type = table_data.pb.has_table_type()
+ ? table_data.pb.table_type() : TableTypePB::DEFAULT_TABLE;
+ if (!ContainsKey(table_types, table_type)) {
continue;
}
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index 7198740..3ecf4ed 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -165,12 +165,14 @@ class MasterTest : public KuduTest {
void DoListTables(const ListTablesRequestPB& req, ListTablesResponsePB*
resp);
void DoListAllTables(ListTablesResponsePB* resp);
Status CreateTable(const string& table_name,
- const Schema& schema);
+ const Schema& schema,
+ const optional<TableTypePB>& table_type = boost::none);
Status CreateTable(const string& table_name,
const Schema& schema,
const vector<KuduPartialRow>& split_rows,
const vector<pair<KuduPartialRow, KuduPartialRow>>&
bounds,
- const optional<string>& owner);
+ const optional<string>& owner,
+ const optional<TableTypePB>& table_type = boost::none);
shared_ptr<Messenger> client_messenger_;
unique_ptr<MiniMaster> mini_master_;
@@ -527,27 +529,32 @@ TEST_F(MasterTest, TestRegisterAndHeartbeat) {
}
Status MasterTest::CreateTable(const string& table_name,
- const Schema& schema) {
+ const Schema& schema,
+ const optional<TableTypePB>& table_type) {
KuduPartialRow split1(&schema);
RETURN_NOT_OK(split1.SetInt32("key", 10));
KuduPartialRow split2(&schema);
RETURN_NOT_OK(split2.SetInt32("key", 20));
- return CreateTable(table_name, schema, { split1, split2 }, {}, boost::none);
+ return CreateTable(
+ table_name, schema, { split1, split2 }, {}, boost::none, table_type);
}
Status MasterTest::CreateTable(const string& table_name,
const Schema& schema,
const vector<KuduPartialRow>& split_rows,
const vector<pair<KuduPartialRow,
KuduPartialRow>>& bounds,
- const optional<string>& owner) {
-
+ const optional<string>& owner,
+ const optional<TableTypePB>& table_type) {
CreateTableRequestPB req;
CreateTableResponsePB resp;
RpcController controller;
req.set_name(table_name);
+ if (table_type) {
+ req.set_table_type(*table_type);
+ }
RETURN_NOT_OK(SchemaToPB(schema, req.mutable_schema()));
RowOperationsPBEncoder encoder(req.mutable_split_rows_range_bounds());
for (const KuduPartialRow& row : split_rows) {
@@ -668,6 +675,109 @@ TEST_F(MasterTest, TestCatalog) {
}
}
+TEST_F(MasterTest, ListTablesWithTableFilter) {
+ static const char* const kUserTableName = "user_table";
+ static const char* const kSystemTableName = "system_table";
+ const Schema kSchema({ColumnSchema("key", INT32), ColumnSchema("v1", INT8)},
1);
+
+ // Make sure this test scenario stays valid
+ ASSERT_EQ(TableTypePB_MAX, TableTypePB::TXN_STATUS_TABLE);
+
+ // Given there is not any tablet server in the cluster, there should be no
+ // tables out there yet, so a request with an empty (default) filter should
+ // return empty list of tables.
+ {
+ ListTablesRequestPB req;
+ ListTablesResponsePB resp;
+ NO_FATALS(DoListTables(req, &resp));
+ ASSERT_EQ(0, resp.tables_size());
+ }
+
+ // The same reasoning as above, but supply filter with all known table types
+ // in the 'table_type' field.
+ {
+ ListTablesRequestPB req;
+ req.add_type_filter(TableTypePB::DEFAULT_TABLE);
+ req.add_type_filter(TableTypePB::TXN_STATUS_TABLE);
+ ListTablesResponsePB resp;
+ NO_FATALS(DoListTables(req, &resp));
+ ASSERT_EQ(0, resp.tables_size());
+ }
+
+ ASSERT_OK(CreateTable(kUserTableName, kSchema));
+
+ // An empty 'filter_type' field is equivalent to setting the 'filter_type'
+ // to [TableTypePB::DEFAULT_TABLE].
+ {
+ ListTablesRequestPB req;
+ ListTablesResponsePB resp;
+ NO_FATALS(DoListTables(req, &resp));
+ ASSERT_EQ(1, resp.tables_size());
+ ASSERT_EQ(kUserTableName, resp.tables(0).name());
+ }
+
+ // Specify the table type filter explicitly.
+ {
+ ListTablesRequestPB req;
+ req.add_type_filter(TableTypePB::DEFAULT_TABLE);
+ ListTablesResponsePB resp;
+ NO_FATALS(DoListTables(req, &resp));
+ ASSERT_EQ(1, resp.tables_size());
+ ASSERT_EQ(kUserTableName, resp.tables(0).name());
+ }
+
+ // No system tables are present at this point.
+ {
+ ListTablesRequestPB req;
+ req.add_type_filter(TableTypePB::TXN_STATUS_TABLE);
+ ListTablesResponsePB resp;
+ NO_FATALS(DoListTables(req, &resp));
+ ASSERT_EQ(0, resp.tables_size());
+ }
+
+ ASSERT_OK(CreateTable(
+ kSystemTableName, kSchema, TableTypePB::TXN_STATUS_TABLE));
+
+ // Only user tables should be listed because the explicitly specified
+ // 'type_filter' is set to [TableTypePB::DEFAULT_TABLE].
+ {
+ ListTablesRequestPB req;
+ req.add_type_filter(TableTypePB::DEFAULT_TABLE);
+ ListTablesResponsePB resp;
+ NO_FATALS(DoListTables(req, &resp));
+ ASSERT_EQ(1, resp.tables_size());
+ ASSERT_EQ(kUserTableName, resp.tables(0).name());
+ }
+
+ // Only the txn status system table should be listed because the explicitly
+ // specified 'type_filter' is set to [TableTypePB::TXN_STATUS_TABLE].
+ {
+ ListTablesRequestPB req;
+ req.add_type_filter(TableTypePB::TXN_STATUS_TABLE);
+ ListTablesResponsePB resp;
+ NO_FATALS(DoListTables(req, &resp));
+ ASSERT_EQ(1, resp.tables_size());
+ ASSERT_EQ(kSystemTableName, resp.tables(0).name());
+ }
+
+ // Both tables should be output when the filter is set to
+ // [TableTypePB::DEFAULT_TABLE, TableTypePB::TXN_STATUS_TABLE].
+ {
+ ListTablesRequestPB req;
+ req.add_type_filter(TableTypePB::DEFAULT_TABLE);
+ req.add_type_filter(TableTypePB::TXN_STATUS_TABLE);
+ ListTablesResponsePB resp;
+ NO_FATALS(DoListTables(req, &resp));
+ ASSERT_EQ(2, resp.tables_size());
+ unordered_set<string> table_names;
+ table_names.emplace(resp.tables(0).name());
+ table_names.emplace(resp.tables(1).name());
+ ASSERT_EQ(2, table_names.size());
+ ASSERT_TRUE(ContainsKey(table_names, kUserTableName));
+ ASSERT_TRUE(ContainsKey(table_names, kSystemTableName));
+ }
+}
+
TEST_F(MasterTest, TestCreateTableCheckRangeInvariants) {
const char *kTableName = "testtb";
const Schema kTableSchema({ ColumnSchema("key", INT32), ColumnSchema("val",
INT32) }, 1);
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index 60fadbc..4898509 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -552,6 +552,11 @@ message DeleteTableResponsePB {
message ListTablesRequestPB {
// When used, only returns tables that satisfy a substring match on
name_filter.
optional string name_filter = 1;
+
+ // The filter for the table types to return. If not set or empty, it is
+ // interpreted as if it were set to [TableTypePB::DEFAULT_TABLE], meaning
+ // to include only user-defined tables.
+ repeated TableTypePB type_filter = 2;
}
message ListTablesResponsePB {
