[kudu] 03/03: [tablet] KUDU-3023 validate RPC vs transaction size limit

[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 c235ba188d282ebba6c83e19a0aab07846d4b91a
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Jan 14 11:39:22 2020 -0800

    [tablet] KUDU-3023 validate RPC vs transaction size limit
    
    Added a group validator to enforce consistency between the maximum RPC
    size and the per-tablet transaction memory limit.  This is to make it
    possible to apply big enough write requests replicated by Raft (i.e.,
    very close to the maximum allowed RPC size) by the local replica itself.
    
    I also added a validator for the --tablet_transaction_memory_limit_mb
    flag and updated a test that was using custom setting for the flag
    to pass the newly added group flag validator.
    
    Change-Id: Ia9384038a078027e997d7774b2be0e753c822fec
    Reviewed-on: http://gerrit.cloudera.org:8080/15030
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
    Tested-by: Kudu Jenkins
---
 src/kudu/integration-tests/raft_consensus-itest.cc | 10 +++++--
 .../tablet/transactions/transaction_tracker.cc     | 32 ++++++++++++++++++++--
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/src/kudu/integration-tests/raft_consensus-itest.cc b/src/kudu/integration-tests/raft_consensus-itest.cc
index 0c2c114..9072b2d 100644
--- a/src/kudu/integration-tests/raft_consensus-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest.cc
@@ -2042,9 +2042,13 @@ TEST_F(RaftConsensusITest, TestMemoryRemainsConstantDespiteTwoDeadFollowers) {
   const int64_t kMinRejections = 100;
   const MonoDelta kMaxWaitTime = MonoDelta::FromSeconds(60);
 
-  // Start the cluster with a low per-tablet transaction memory limit, so that
-  // the test can complete faster.
-  NO_FATALS(BuildAndStart({ "--tablet_transaction_memory_limit_mb=2" }));
+  NO_FATALS(BuildAndStart({
+      // Start the cluster with a low per-tablet transaction memory limit,
+      // so that the test can complete faster.
+      "--tablet_transaction_memory_limit_mb=2",
+      // Make the validator of 'RPC vs transactional memory size' happy.
+      "--rpc_max_message_size=2097152",
+  }));
 
   // Kill both followers.
   TServerDetails* details;
diff --git a/src/kudu/tablet/transactions/transaction_tracker.cc b/src/kudu/tablet/transactions/transaction_tracker.cc
index d732bfd..70e6521 100644
--- a/src/kudu/tablet/transactions/transaction_tracker.cc
+++ b/src/kudu/tablet/transactions/transaction_tracker.cc
@@ -37,6 +37,7 @@
 #include "kudu/tablet/transactions/transaction.h"
 #include "kudu/tablet/transactions/transaction_driver.h"
 #include "kudu/util/flag_tags.h"
+#include "kudu/util/flag_validators.h"
 #include "kudu/util/logging.h"
 #include "kudu/util/mem_tracker.h"
 #include "kudu/util/metrics.h"
@@ -50,6 +51,8 @@ DEFINE_int64(tablet_transaction_memory_limit_mb, 64,
              "disabled.");
 TAG_FLAG(tablet_transaction_memory_limit_mb, advanced);
 
+DECLARE_int64(rpc_max_message_size);
+
 METRIC_DEFINE_gauge_uint64(tablet, all_transactions_inflight,
                            "Transactions In Flight",
                            kudu::MetricUnit::kTransactions,
@@ -84,12 +87,37 @@ METRIC_DEFINE_counter(tablet, transaction_memory_limit_rejections,
 using std::shared_ptr;
 using std::string;
 using std::vector;
+using strings::Substitute;
+
+static bool ValidateTransactionMemoryLimit(const char* flagname, int64_t value) {
+  // -1 is a special value for the  --tablet_transaction_memory_limit_mb flag.
+  if (value < -1) {
+    LOG(ERROR) << Substitute("$0: invalid value for flag $1", value, flagname);
+    return false;
+  }
+  return true;
+}
+DEFINE_validator(tablet_transaction_memory_limit_mb, ValidateTransactionMemoryLimit);
+
+static bool ValidateTransactionMemoryAndRpcSize() {
+  const int64_t transaction_max_size =
+      FLAGS_tablet_transaction_memory_limit_mb * 1024 * 1024;
+  const int64_t rpc_max_size = FLAGS_rpc_max_message_size;
+  if (transaction_max_size >= 0 && transaction_max_size < rpc_max_size) {
+    LOG(ERROR) << Substitute(
+        "--tablet_transaction_memory_limit_mb is set too low compared with "
+        "--rpc_max_message_size; increase --tablet_transaction_memory_limit_mb "
+        "at least up to $0", (rpc_max_size + 1024 * 1024 - 1) / (1024 * 1024));
+    return false;
+  }
+  return true;
+}
+GROUP_FLAG_VALIDATOR(transaction_memory_and_rpc_size,
+                     ValidateTransactionMemoryAndRpcSize);
 
 namespace kudu {
 namespace tablet {
 
-using strings::Substitute;
-
 #define MINIT(x) x(METRIC_##x.Instantiate(entity))
 #define GINIT(x) x(METRIC_##x.Instantiate(entity, 0))
 TransactionTracker::Metrics::Metrics(const scoped_refptr<MetricEntity>& entity)