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 1754f51 [client] fix MetaCacheEntry::Contains()
1754f51 is described below
commit 1754f517b279f349da8c648251ce47a552d54794
Author: Alexey Serbin <[email protected]>
AuthorDate: Thu Nov 11 11:11:37 2021 -0800
[client] fix MetaCacheEntry::Contains()
It seems that while working on a50091e2d, I went too far with
back-porting changes from the revamped code and used the unabridged
MetaCacheEntry::Contains() implementation from the new approach. That
turned to be problematic if still using the legacy way of comparing
partition keys for tables using only hash partitioning.
This patch returns the legacy implementation of the
MetaCacheEntry::Contains() method back and adds a couple of stress
test scenarios for the C++ client's metacache. Before reverting to
the legacy implementation of the MetaCacheEntry::Contains() method,
both Perf and PerfSynthetic test scenarios were failing because
KuduDeleteIgnore operations would timeout since MetaCache was endlessly
fetching information on particular PartitionKeys again and again when
MetaCacheEntry::Contains() returned 'false' instead of returning 'true'.
This is a follow-up to a50091e2d4509feac2f29128107102ec52fcb7b0.
Change-Id: I7f2deeefbf2e3cedc7654b5d977382501db14b4e
Reviewed-on: http://gerrit.cloudera.org:8080/18024
Tested-by: Kudu Jenkins
Reviewed-by: Attila Bukor <[email protected]>
---
src/kudu/client/client.h | 2 +
src/kudu/client/meta_cache.cc | 14 +--
src/kudu/client/meta_cache.h | 1 +
src/kudu/integration-tests/client-stress-test.cc | 152 ++++++++++++++++++++++-
4 files changed, 154 insertions(+), 15 deletions(-)
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index ff3910b..3905ec8 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -56,6 +56,7 @@ namespace kudu {
class AlterTableTest;
class AuthzTokenTest;
class ClientStressTest_TestUniqueClientIds_Test;
+class MetaCacheLookupStressTest_PerfSynthetic_Test;
class DisableWriteWhenExceedingQuotaTest;
class KuduPartialRow;
class MonoDelta;
@@ -997,6 +998,7 @@ class KUDU_EXPORT KuduClient : public
sp::enable_shared_from_this<KuduClient> {
friend class tools::TableLister;
FRIEND_TEST(kudu::ClientStressTest, TestUniqueClientIds);
+ FRIEND_TEST(kudu::MetaCacheLookupStressTest, PerfSynthetic);
FRIEND_TEST(ClientTest, ClearCacheAndConcurrentWorkload);
FRIEND_TEST(ClientTest, ConnectionNegotiationTimeout);
FRIEND_TEST(ClientTest, TestBasicIdBasedLookup);
diff --git a/src/kudu/client/meta_cache.cc b/src/kudu/client/meta_cache.cc
index e8f2aa4..e475bf0 100644
--- a/src/kudu/client/meta_cache.cc
+++ b/src/kudu/client/meta_cache.cc
@@ -384,17 +384,9 @@ string RemoteTablet::ReplicasAsStringUnlocked() const {
bool MetaCacheEntry::Contains(const PartitionKey& partition_key) const {
DCHECK(Initialized());
- const auto& entry_hash_key = !lower_bound_partition_key().empty()
- ? lower_bound_partition_key().hash_key()
- : upper_bound_partition_key().hash_key();
- // The hash part of the key has 'the exact' semantics: they should match
- // for the partition key in question and the meta-cache entry.
- if (partition_key.hash_key() != entry_hash_key) {
- return false;
- }
- return lower_bound_partition_key().range_key() <= partition_key.range_key()
&&
- (upper_bound_partition_key().empty() ||
- partition_key.range_key() <
upper_bound_partition_key().range_key());
+ return lower_bound_partition_key() <= partition_key &&
+ (upper_bound_partition_key().empty() ||
+ upper_bound_partition_key() > partition_key);
}
bool MetaCacheEntry::stale() const {
diff --git a/src/kudu/client/meta_cache.h b/src/kudu/client/meta_cache.h
index 5eace9a..cd66863 100644
--- a/src/kudu/client/meta_cache.h
+++ b/src/kudu/client/meta_cache.h
@@ -480,6 +480,7 @@ class MetaCache : public RefCountedThreadSafe<MetaCache> {
FRIEND_TEST(client::ClientTest, TestMasterLookupPermits);
FRIEND_TEST(client::ClientTest, TestMetaCacheExpiry);
+ FRIEND_TEST(MetaCacheLookupStressTest, PerfSynthetic);
// Called on the slow LookupTablet path when the master responds. Populates
// the tablet caches and returns a reference to the first one.
diff --git a/src/kudu/integration-tests/client-stress-test.cc
b/src/kudu/integration-tests/client-stress-test.cc
index 8bd8ade..a9731f3 100644
--- a/src/kudu/integration-tests/client-stress-test.cc
+++ b/src/kudu/integration-tests/client-stress-test.cc
@@ -32,10 +32,16 @@
#include "kudu/client/client-internal.h"
#include "kudu/client/client-test-util.h"
#include "kudu/client/client.h"
+#include "kudu/client/meta_cache.h"
#include "kudu/client/scan_predicate.h"
+#include "kudu/client/schema.h"
#include "kudu/client/shared_ptr.h" // IWYU pragma: keep
#include "kudu/client/value.h"
+#include "kudu/client/write_op.h"
+#include "kudu/common/partial_row.h"
+#include "kudu/common/partition.h"
#include "kudu/gutil/port.h"
+#include "kudu/gutil/ref_counted.h"
#include "kudu/gutil/strings/join.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/integration-tests/cluster_itest_util.h"
@@ -47,6 +53,7 @@
#include "kudu/util/pstack_watcher.h"
#include "kudu/util/random.h"
#include "kudu/util/status.h"
+#include "kudu/util/stopwatch.h"
#include "kudu/util/test_macros.h"
#include "kudu/util/test_util.h"
@@ -55,8 +62,14 @@ METRIC_DECLARE_counter(leader_memory_pressure_rejections);
METRIC_DECLARE_counter(follower_memory_pressure_rejections);
using kudu::client::KuduClient;
+using kudu::client::KuduColumnSchema;
+using kudu::client::KuduDeleteIgnore;
using kudu::client::KuduScanner;
+using kudu::client::KuduSchema;
+using kudu::client::KuduSchemaBuilder;
+using kudu::client::KuduSession;
using kudu::client::KuduTable;
+using kudu::client::KuduTableCreator;
using kudu::cluster::ExternalMiniCluster;
using kudu::cluster::ExternalMiniClusterOptions;
using std::set;
@@ -70,7 +83,7 @@ namespace kudu {
class ClientStressTest : public KuduTest {
public:
- virtual void SetUp() OVERRIDE {
+ void SetUp() override {
KuduTest::SetUp();
ExternalMiniClusterOptions opts = default_opts();
@@ -82,9 +95,11 @@ class ClientStressTest : public KuduTest {
ASSERT_OK(cluster_->Start());
}
- virtual void TearDown() OVERRIDE {
+ void TearDown() override {
alarm(0);
- cluster_->Shutdown();
+ if (cluster_) {
+ cluster_->Shutdown();
+ }
KuduTest::TearDown();
}
@@ -176,7 +191,7 @@ TEST_F(ClientStressTest, TestStartScans) {
// Override the base test to run in multi-master mode.
class ClientStressTest_MultiMaster : public ClientStressTest {
protected:
- virtual bool multi_master() const OVERRIDE {
+ bool multi_master() const override {
return true;
}
};
@@ -319,4 +334,133 @@ TEST_F(ClientStressTest, TestUniqueClientIds) {
}
}
+#if !defined(THREAD_SANITIZER) && !defined(ADDRESS_SANITIZER)
+// Test for stress scenarios exercising meta-cache lookup path. The scenarios
+// not to be run under sanitizer builds since they are CPU intensive.
+class MetaCacheLookupStressTest : public ClientStressTest {
+ public:
+ void SetUp() override {
+ using client::sp::shared_ptr;
+
+ // All scenarios of this test are supposed to be CPU-intensive, so check
+ // for KUDU_ALLOW_SLOW_TESTS during the SetUp() phase.
+ SKIP_IF_SLOW_NOT_ALLOWED();
+ ClientStressTest::SetUp();
+
+ KuduSchemaBuilder b;
+ b.AddColumn(kKeyColumn)->Type(KuduColumnSchema::INT64)->NotNull();
+ b.AddColumn(kStrColumn)->Type(KuduColumnSchema::STRING)->NotNull();
+ b.SetPrimaryKey({ kKeyColumn, kStrColumn });
+ KuduSchema schema;
+ ASSERT_OK(b.Build(&schema));
+
+ ASSERT_OK(cluster_->CreateClient(nullptr, &client_));
+
+ unique_ptr<KuduTableCreator> tc(client_->NewTableCreator());
+
+ // The table contains many tablets, so give it extra time to create
+ // corresponding tablets.
+ ASSERT_OK(tc->table_name(kTableName)
+ .schema(&schema)
+ .num_replicas(1)
+ .add_hash_partitions({ kKeyColumn, kStrColumn }, 64)
+ .timeout(MonoDelta::FromSeconds(300))
+ .Create());
+ ASSERT_OK(client_->OpenTable(kTableName, &table_));
+
+ // Prime the client's meta-cache.
+ shared_ptr<KuduSession> session(client_->NewSession());
+ ASSERT_OK(session->SetFlushMode(KuduSession::AUTO_FLUSH_BACKGROUND));
+ for (int32_t idx = 0; idx < kNumRows; ++idx) {
+ unique_ptr<KuduDeleteIgnore> op(table_->NewDeleteIgnore());
+ ASSERT_OK(op->mutable_row()->SetInt64(0, idx));
+ ASSERT_OK(op->mutable_row()->SetString(1,
+ string(1, static_cast<char>(idx % 128))));
+ ASSERT_OK(session->Apply(op.release()));
+ }
+ ASSERT_OK(session->Flush());
+ }
+
+ protected:
+ static constexpr auto kNumRows = 1000000;
+ static constexpr const char* kTableName = "meta_cache_lookup";
+ static constexpr const char* kKeyColumn = "key";
+ static constexpr const char* kStrColumn = "str_val";
+
+ ExternalMiniClusterOptions default_opts() const override {
+ ExternalMiniClusterOptions opts;
+ opts.num_tablet_servers = 10;
+ return opts;
+ }
+
+ vector<unique_ptr<KuduDeleteIgnore>> GenerateOperations(int64_t num_ops) {
+ vector<unique_ptr<KuduDeleteIgnore>> ops;
+ ops.reserve(num_ops);
+ for (int64_t idx = 0; idx < num_ops; ++idx) {
+ unique_ptr<KuduDeleteIgnore> op(table_->NewDeleteIgnore());
+ CHECK_OK(op->mutable_row()->SetInt64(0, idx));
+ CHECK_OK(op->mutable_row()->SetString(
+ 1, std::to_string(idx) + string(1, static_cast<char>(idx % 128))));
+ ops.emplace_back(std::move(op));
+ }
+ return ops;
+ }
+
+ client::sp::shared_ptr<KuduClient> client_;
+ client::sp::shared_ptr<KuduTable> table_;
+};
+
+// This test scenario creates a table with many partitions and generates a lot
+// of rows, stressing meta-cache fast lookup path in the client.
+TEST_F(MetaCacheLookupStressTest, PerfSynthetic) {
+ vector<unique_ptr<KuduDeleteIgnore>> ops(GenerateOperations(kNumRows));
+
+ // Start the stopwatch and run the benchmark. All the lookups in the
+ // meta-cache should be fast since the meta-cache has been populated
+ // by the activity above.
+ Stopwatch sw;
+ sw.start();
+ {
+ auto& meta_cache = client_->data_->meta_cache_;
+ for (const auto& op : ops) {
+ client::internal::MetaCacheEntry entry;
+ ASSERT_TRUE(meta_cache->LookupEntryByKeyFastPath(
+ table_.get(), table_->partition_schema().EncodeKey(op->row()),
&entry));
+ }
+ }
+ sw.stop();
+
+ const auto wall_spent_ms = sw.elapsed().wall_millis();
+ LOG(INFO) << Substitute("Total time spent: $0 ms", wall_spent_ms);
+ LOG(INFO) << Substitute("Time per row: $0 ms", wall_spent_ms / kNumRows);
+}
+
+TEST_F(MetaCacheLookupStressTest, Perf) {
+ vector<unique_ptr<KuduDeleteIgnore>> ops(GenerateOperations(kNumRows));
+ // Create a session using manual flushing mode and set the buffer to be
+ // large enough to accommodate all the generated operations at once.
+ client::sp::shared_ptr<KuduSession> session(client_->NewSession());
+ ASSERT_OK(session->SetFlushMode(KuduSession::MANUAL_FLUSH));
+ ASSERT_OK(session->SetMutationBufferSpace(64 * 1024 * 1024));
+
+ // Start the stopwatch and run the benchmark. All the lookups in the
+ // meta-cache should be fast since the meta-cache has been populated
+ // by the activity above. The lookups starts once Apply() is called,
+ // and to make sure every entry has been lookup and, KuduSession::Flush()
+ // is called: in addition to lookups in the meta-cache, it sends all
+ // the accumulated operations to the server.
+ Stopwatch sw;
+ sw.start();
+ for (auto& op : ops) {
+ ASSERT_OK(session->Apply(op.release()));
+ }
+ ASSERT_OK(session->Flush());
+ sw.stop();
+
+ const auto wall_spent_ms = sw.elapsed().wall_millis();
+ LOG(INFO) << Substitute("Total time spent: $0 ms", wall_spent_ms);
+ LOG(INFO) << Substitute("Time per row: $0 ms", wall_spent_ms / kNumRows);
+}
+#endif // #if !defined(THREAD_SANITIZER) && !defined(ADDRESS_SANITIZER)
+
} // namespace kudu
