You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2022/08/27 14:49:40 UTC

[kudu] branch master updated (e649d16d4 -> 9021f2758)

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


    from e649d16d4 [util] address TODO to remove dummy encryption key
     new 8b35fc59b [test] fix ListTableCliParamTest
     new 66c5f52ab [test] instantiate TsRecoveryITestDeathTest
     new 9021f2758 [test] instantiate ScanPrivilegeVirtualColumnsTest

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/kudu/consensus/log-test-base.h                 |  1 +
 src/kudu/consensus/log-test.cc                     |  3 +-
 src/kudu/integration-tests/ts_recovery-itest.cc    |  6 +++-
 src/kudu/tools/kudu-admin-test.cc                  | 39 +++++++++++-----------
 .../tserver/tablet_server_authorization-test.cc    |  9 +++++
 5 files changed, 37 insertions(+), 21 deletions(-)


[kudu] 01/03: [test] fix ListTableCliParamTest

Posted by al...@apache.org.
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

commit 8b35fc59b666dbb18a021b776741e28df7bdb4ff
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Fri Aug 26 13:01:16 2022 -0700

    [test] fix ListTableCliParamTest
    
    This patch adds the missing instantiation of ListTableCliParamTest
    and fixes the test, so now it's run and pass.
    
    This is a follow-up to 76d540c137c991127865d5d2455d113677a9afe9.
    
    Change-Id: I66c8e7fe66cf648341d74a184c63f6f622933f77
    Reviewed-on: http://gerrit.cloudera.org:8080/18919
    Tested-by: Alexey Serbin <al...@apache.org>
    Reviewed-by: Yingchun Lai <ac...@gmail.com>
---
 src/kudu/tools/kudu-admin-test.cc | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/src/kudu/tools/kudu-admin-test.cc b/src/kudu/tools/kudu-admin-test.cc
index 088a36e75..803fb62bc 100644
--- a/src/kudu/tools/kudu-admin-test.cc
+++ b/src/kudu/tools/kudu-admin-test.cc
@@ -2185,9 +2185,10 @@ class ListTableCliParamTest : public AdminCliTest,
 };
 
 // Basic test that the kudu tool works in the list tablets case.
-TEST_P(ListTableCliParamTest, TestListTabletWithPartition) {
-  auto show_hp = GetParam() ? PartitionSchema::HashPartitionInfo::SHOW :
-      PartitionSchema::HashPartitionInfo::HIDE;
+TEST_P(ListTableCliParamTest, ListTabletWithPartitionInfo) {
+  const auto show_hp = GetParam() ? PartitionSchema::HashPartitionInfo::SHOW
+                                  : PartitionSchema::HashPartitionInfo::HIDE;
+  const auto kTimeout = MonoDelta::FromSeconds(30);
   FLAGS_num_tablet_servers = 1;
   FLAGS_num_replicas = 1;
 
@@ -2196,12 +2197,11 @@ TEST_P(ListTableCliParamTest, TestListTabletWithPartition) {
   vector<TServerDetails*> tservers;
   vector<string> base_tablet_ids;
   AppendValuesFromMap(tablet_servers_, &tservers);
-  ListRunningTabletIds(tservers.front(),
-                       MonoDelta::FromSeconds(30), &base_tablet_ids);
+  ListRunningTabletIds(tservers.front(), kTimeout, &base_tablet_ids);
 
   // Test a table with all types in its schema, multiple hash partitioning
   // levels, multiple range partitions, and non-covered ranges.
-  const string kTableId = "TestTableListPartition";
+  constexpr const char* const kTableName = "TestTableListPartition";
   KuduSchema schema;
 
   // Build the schema.
@@ -2226,7 +2226,7 @@ TEST_P(ListTableCliParamTest, TestListTabletWithPartition) {
     unique_ptr<KuduPartialRow> upper_bound1(schema.NewRow());
     ASSERT_OK(upper_bound1->SetInt32("key_range", 3));
     unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
-    ASSERT_OK(table_creator->table_name(kTableId)
+    ASSERT_OK(table_creator->table_name(kTableName)
         .schema(&schema)
         .add_hash_partitions({"key_hash0"}, 2)
         .add_hash_partitions({"key_hash1", "key_hash2"}, 3)
@@ -2238,8 +2238,7 @@ TEST_P(ListTableCliParamTest, TestListTabletWithPartition) {
   }
 
   vector<string> new_tablet_ids;
-  ListRunningTabletIds(tservers.front(),
-                       MonoDelta::FromSeconds(30), &new_tablet_ids);
+  ListRunningTabletIds(tservers.front(), kTimeout, &new_tablet_ids);
   vector<string> delta_tablet_ids;
   for (auto& tablet_id : base_tablet_ids) {
     if (std::find(new_tablet_ids.begin(), new_tablet_ids.end(), tablet_id) ==
@@ -2255,36 +2254,38 @@ TEST_P(ListTableCliParamTest, TestListTabletWithPartition) {
     "table",
     "list",
     "--list_tablets",
-    GetParam() ? "--show_tablet_partition_info" : "",
+    "--show_tablet_partition_info",
+    Substitute("--show_hash_partition_info=$0", GetParam()),
     "--tables",
-    kTableId,
+    kTableName,
     cluster_->master()->bound_rpc_addr().ToString(),
   }, &stdout, &stderr);
   ASSERT_TRUE(s.ok()) << ToolRunInfo(s, stdout, stderr);
 
   client::sp::shared_ptr<KuduTable> table;
-  ASSERT_OK(client_->OpenTable(kTableId, &table));
+  ASSERT_OK(client_->OpenTable(kTableName, &table));
   const auto& partition_schema = table->partition_schema();
   const auto& schema_internal = KuduSchema::ToSchema(table->schema());
 
   // make sure table name correct
-  ASSERT_STR_CONTAINS(stdout, kTableId);
+  ASSERT_STR_CONTAINS(stdout, kTableName);
 
   master::ListTablesResponsePB tables_info;
-  ASSERT_OK(ListTablesWithInfo(cluster_->master_proxy(), kTableId,
-      MonoDelta::FromSeconds(30), &tables_info));
+  ASSERT_OK(ListTablesWithInfo(
+      cluster_->master_proxy(), kTableName, kTimeout, &tables_info));
   for (const auto& table : tables_info.tables()) {
     for (const auto& pt : table.tablet_with_partition()) {
       Partition partition;
       Partition::FromPB(pt.partition(), &partition);
-      string partition_str = partition_schema.PartitionDebugString(partition,
-                                                                   schema_internal,
-                                                                   show_hp);
-      string tablet_with_partition = pt.tablet_id() + " : " + partition_str;
+      const auto partition_str = partition_schema.PartitionDebugString(
+          partition, schema_internal, show_hp);
+      const auto tablet_with_partition = pt.tablet_id() + " : " + partition_str;
       ASSERT_STR_CONTAINS(stdout, tablet_with_partition);
     }
   }
 }
+INSTANTIATE_TEST_SUITE_P(IsHashPartitionInfoShown, ListTableCliParamTest,
+                         ::testing::Bool());
 
 TEST_F(AdminCliTest, TestLocateRow) {
   FLAGS_num_tablet_servers = 1;


[kudu] 02/03: [test] instantiate TsRecoveryITestDeathTest

Posted by al...@apache.org.
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

commit 66c5f52ab46807f925c6826283f84ded4e193f18
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Fri Aug 26 13:31:13 2022 -0700

    [test] instantiate TsRecoveryITestDeathTest
    
    As it turned out, the parameterized TsRecoveryITestDeathTest was not
    instantiated.  Digging into the history of changes, I found that [1]
    introduced TEST_F --> TEST_P change without adding corresponding
    instantiations for the parameterized test suite.
    
    I fixed the test by adapting it for the parameterization it went through
    and added the missing instantiation macro.  In addition, [2] introduced
    an extra DCHECK() into TabletReplica::FinishConsensusOnlyRound().  Once
    instantiated, the test started hitting the DCHECK().  To address that
    issue, I updated the test-only function AppendNoOpsToLogSync() in
    log-test-base.h to fill in the 'noop_request' field along with setting
    the 'op_type' field to NO_OP.  With that, since I added some extra data
    into a NO_OP log record, I also updated the reference size of the WAL
    segment in Raft consensus log test.
    
    [1] https://github.com/apache/kudu/commit/371a00b7aeba244aa63d92bf479cbb356b4dfbca
    [2] https://github.com/apache/kudu/commit/bc817a44867c586bf4e0539aa564b282c666a49d
    
    Change-Id: Iaf48782ad17ac40023ee94770820d8d403c5cf96
    Reviewed-on: http://gerrit.cloudera.org:8080/18920
    Tested-by: Kudu Jenkins
    Reviewed-by: Yingchun Lai <ac...@gmail.com>
---
 src/kudu/consensus/log-test-base.h              | 1 +
 src/kudu/consensus/log-test.cc                  | 3 ++-
 src/kudu/integration-tests/ts_recovery-itest.cc | 6 +++++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/kudu/consensus/log-test-base.h b/src/kudu/consensus/log-test-base.h
index 669210292..6aa1f280c 100644
--- a/src/kudu/consensus/log-test-base.h
+++ b/src/kudu/consensus/log-test-base.h
@@ -80,6 +80,7 @@ inline Status AppendNoOpsToLogSync(clock::Clock* clock,
 
     repl->mutable_id()->CopyFrom(*op_id);
     repl->set_op_type(consensus::NO_OP);
+    repl->mutable_noop_request(); // add a no-op request field
     repl->set_timestamp(clock->Now().ToUint64());
 
     // Increment op_id.
diff --git a/src/kudu/consensus/log-test.cc b/src/kudu/consensus/log-test.cc
index c1005b104..4ea6e0bc8 100644
--- a/src/kudu/consensus/log-test.cc
+++ b/src/kudu/consensus/log-test.cc
@@ -467,6 +467,7 @@ TEST_F(LogTest, TestWriteAndReadToAndFromInProgressSegment) {
   ReplicateMsg* repl = log_entry->mutable_replicate();
   repl->mutable_id()->CopyFrom(op_id);
   repl->set_op_type(NO_OP);
+  repl->mutable_noop_request();
   repl->set_timestamp(0L);
 
   // Entries are prefixed with a header.
@@ -1074,7 +1075,7 @@ TEST_F(LogTest, TestGetGCableDataSize) {
 
   const int kNumTotalSegments = 5;
   const int kNumOpsPerSegment = 5;
-  const int64_t kSegmentSizeBytes = 337 + env_->GetEncryptionHeaderSize();
+  const int64_t kSegmentSizeBytes = 352 + env_->GetEncryptionHeaderSize();
   OpId op_id = MakeOpId(1, 10);
   // Create 5 segments, starting from log index 10, with 5 ops per segment.
   // [10-14], [15-19], [20-24], [25-29], [30-34]
diff --git a/src/kudu/integration-tests/ts_recovery-itest.cc b/src/kudu/integration-tests/ts_recovery-itest.cc
index 35b93d8b5..ea499004f 100644
--- a/src/kudu/integration-tests/ts_recovery-itest.cc
+++ b/src/kudu/integration-tests/ts_recovery-itest.cc
@@ -23,6 +23,7 @@
 #include <ostream>
 #include <string>
 #include <thread>
+#include <type_traits>
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
@@ -552,12 +553,14 @@ TEST_P(TsRecoveryITest, TestChangeMaxCellSize) {
 // test scenario isn't present for TSAN builds.
 class TsRecoveryITestDeathTest : public TsRecoveryITest {
 };
+INSTANTIATE_TEST_SUITE_P(BlockManagerType, TsRecoveryITestDeathTest,
+                         ::testing::ValuesIn(BlockManager::block_manager_types()));
 
 // Test that tablet bootstrap can automatically repair itself if it finds an
 // overflowed OpId index written to the log caused by KUDU-1933.
 // Also serves as a regression itest for KUDU-1933 by writing ops with a high
 // term and index.
-TEST_P(TsRecoveryITestDeathTest, TestRecoverFromOpIdOverflow) {
+TEST_P(TsRecoveryITestDeathTest, RecoverFromOpIdOverflow) {
   // Create the initial tablet files on disk, then shut down the cluster so we
   // can meddle with the WAL.
   NO_FATALS(StartClusterOneTs());
@@ -584,6 +587,7 @@ TEST_P(TsRecoveryITestDeathTest, TestRecoverFromOpIdOverflow) {
     FsManagerOpts opts;
     opts.wal_root = ets->wal_dir();
     opts.data_roots = ets->data_dirs();
+    opts.block_manager_type = GetParam();
     unique_ptr<FsManager> fs_manager(new FsManager(env_, opts));
     ASSERT_OK(fs_manager->Open());
     scoped_refptr<ConsensusMetadataManager> cmeta_manager(


[kudu] 03/03: [test] instantiate ScanPrivilegeVirtualColumnsTest

Posted by al...@apache.org.
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

commit 9021f275824faa2bdfe699786957c40c219697c1
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Fri Aug 26 18:07:33 2022 -0700

    [test] instantiate ScanPrivilegeVirtualColumnsTest
    
    Before this patch, the ScanPrivilegeVirtualColumnsTest suite was not
    instantiated, so no tests from the parameterized suite were run.  This
    patch adds the missing instantiation, so now the tests are run.
    
    This is a follow-up to 66f4bb136e1bc42e8c031548b56ee1927002ac09.
    
    Change-Id: Id60acd02f22a9ed617c590d8cf0387a366ddc0b1
    Reviewed-on: http://gerrit.cloudera.org:8080/18921
    Tested-by: Kudu Jenkins
    Reviewed-by: Yingchun Lai <ac...@gmail.com>
---
 src/kudu/tserver/tablet_server_authorization-test.cc | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/kudu/tserver/tablet_server_authorization-test.cc b/src/kudu/tserver/tablet_server_authorization-test.cc
index ea7fd6a67..69ec62b1a 100644
--- a/src/kudu/tserver/tablet_server_authorization-test.cc
+++ b/src/kudu/tserver/tablet_server_authorization-test.cc
@@ -974,6 +974,15 @@ TEST_P(ScanPrivilegeVirtualColumnsTest, TestNoProjection) {
     NO_FATALS(CheckPrivileges(req_func, scan, privileges, ExpectedAuthz::DENIED));
   }
 }
+INSTANTIATE_TEST_SUITE_P(RequestorFuncs, ScanPrivilegeVirtualColumnsTest,
+                         ::testing::Combine(
+        ::testing::ValuesIn(vector<ScanFunc>({
+            &ScanRequestor<DeprecatedField::DONT_USE, SpecialColumn::VIRTUAL>,
+            &ScanRequestor<DeprecatedField::USE, SpecialColumn::VIRTUAL>,
+            &ChecksumRequestor<DeprecatedField::DONT_USE, SpecialColumn::VIRTUAL>,
+            &ChecksumRequestor<DeprecatedField::USE, SpecialColumn::VIRTUAL>,
+        })),
+        ::testing::Bool()));
 
 class ScanPrivilegeWithBadNamesTest: public ScanPrivilegeAuthzTest {};