You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by gr...@apache.org on 2020/11/30 00:19:24 UTC

[kudu] 01/02: txns: use HostPorts instead of strings to build TxnSystemClient

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

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

commit f47caaf3ee4339f5e34b9ad75ba4cbcef686c508
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Wed Nov 25 22:25:31 2020 -0800

    txns: use HostPorts instead of strings to build TxnSystemClient
    
    Strings are the inputs for the ClientBuilder, but in most tests, we have
    HostPorts available, so we currently convert the master addresses to a
    vector<string> and pass it to the TxnSystemClient consturctor. Rather
    than converting at all the call-sites, this patch converts the
    constructor to use HostPorts instead.
    
    Change-Id: I92a09a256e84020756f3073cd7289a2245058e4a
    Reviewed-on: http://gerrit.cloudera.org:8080/16787
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/integration-tests/auth_token_expire-itest.cc       |  6 +-----
 .../integration-tests/client-negotiation-failover-itest.cc  |  7 +------
 src/kudu/integration-tests/master_authz-itest.cc            |  9 ++-------
 src/kudu/integration-tests/master_hms-itest.cc              |  7 +------
 src/kudu/integration-tests/txn_status_table-itest.cc        | 13 ++-----------
 src/kudu/master/txn_manager.cc                              |  7 +------
 src/kudu/transactions/txn_system_client.cc                  |  9 +++++++--
 src/kudu/transactions/txn_system_client.h                   |  4 +++-
 8 files changed, 18 insertions(+), 44 deletions(-)

diff --git a/src/kudu/integration-tests/auth_token_expire-itest.cc b/src/kudu/integration-tests/auth_token_expire-itest.cc
index 4f07bb9..b553cce 100644
--- a/src/kudu/integration-tests/auth_token_expire-itest.cc
+++ b/src/kudu/integration-tests/auth_token_expire-itest.cc
@@ -456,12 +456,8 @@ TEST_F(TokenBasedConnectionITest, ReacquireAuthnToken) {
 // of the transaction status table.
 TEST_F(TokenBasedConnectionITest, TxnSystemClientReacquireAuthnToken) {
   SKIP_IF_SLOW_NOT_ALLOWED();
-  vector<string> master_addrs;
-  for (const auto& hp : cluster_->master_rpc_addrs()) {
-    master_addrs.emplace_back(hp.ToString());
-  }
   unique_ptr<TxnSystemClient> txn_client;
-  ASSERT_OK(TxnSystemClient::Create(master_addrs, &txn_client));
+  ASSERT_OK(TxnSystemClient::Create(cluster_->master_rpc_addrs(), &txn_client));
   ASSERT_OK(txn_client->CreateTxnStatusTable(10));
   ASSERT_OK(txn_client->OpenTxnStatusTable());
 
diff --git a/src/kudu/integration-tests/client-negotiation-failover-itest.cc b/src/kudu/integration-tests/client-negotiation-failover-itest.cc
index 7b80bba..ee831b6 100644
--- a/src/kudu/integration-tests/client-negotiation-failover-itest.cc
+++ b/src/kudu/integration-tests/client-negotiation-failover-itest.cc
@@ -36,7 +36,6 @@
 #include "kudu/tablet/key_value_test_schema.h"
 #include "kudu/transactions/txn_system_client.h"
 #include "kudu/util/monotime.h"
-#include "kudu/util/net/net_util.h"
 #include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
@@ -207,12 +206,8 @@ TEST_F(ClientFailoverOnNegotiationTimeoutITest, TestTxnSystemClientRetryOnPause)
   cluster_opts_.num_tablet_servers = kNumTabletServers;
   ASSERT_OK(CreateAndStartCluster());
 
-  vector<string> master_addrs;
-  for (const auto& hp : cluster_->master_rpc_addrs()) {
-    master_addrs.emplace_back(hp.ToString());
-  }
   unique_ptr<TxnSystemClient> txn_client;
-  ASSERT_OK(TxnSystemClient::Create(master_addrs, &txn_client));
+  ASSERT_OK(TxnSystemClient::Create(cluster_->master_rpc_addrs(), &txn_client));
   ASSERT_OK(txn_client->CreateTxnStatusTable(100, kNumTabletServers));
   ASSERT_OK(txn_client->OpenTxnStatusTable());
 
diff --git a/src/kudu/integration-tests/master_authz-itest.cc b/src/kudu/integration-tests/master_authz-itest.cc
index bbce5c1..cf1864c 100644
--- a/src/kudu/integration-tests/master_authz-itest.cc
+++ b/src/kudu/integration-tests/master_authz-itest.cc
@@ -49,7 +49,6 @@
 #include "kudu/transactions/txn_system_client.h"
 #include "kudu/util/decimal_util.h"
 #include "kudu/util/monotime.h"
-#include "kudu/util/net/net_util.h"
 #include "kudu/util/slice.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
@@ -741,16 +740,12 @@ INSTANTIATE_TEST_CASE_P(AuthzProviders, MasterAuthzITest,
 TEST_P(MasterAuthzITest, TestCreateTransactionStatusTable) {
   // Create a transaction status table and add a range. Both requests should
   // succeed, despite no privileges being granted in Ranger.
-  vector<string> master_addrs;
-  for (const auto& hp : cluster_->master_rpc_addrs()) {
-    master_addrs.emplace_back(hp.ToString());
-  }
   ASSERT_OK(this->cluster_->kdc()->Kinit(kTestUser));
   // Attempting to call system client DDL as a non-admin should result in a
   // NotAuthorized error.
   {
     unique_ptr<TxnSystemClient> non_admin_client;
-    ASSERT_OK(TxnSystemClient::Create(master_addrs, &non_admin_client));
+    ASSERT_OK(TxnSystemClient::Create(cluster_->master_rpc_addrs(), &non_admin_client));
     Status s = non_admin_client->CreateTxnStatusTable(100);
     ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
     s = non_admin_client->AddTxnStatusTableRange(100, 200);
@@ -759,7 +754,7 @@ TEST_P(MasterAuthzITest, TestCreateTransactionStatusTable) {
   // But as service user, we should have no trouble making the calls.
   ASSERT_OK(this->cluster_->kdc()->Kinit(kAdminUser));
   unique_ptr<TxnSystemClient> txn_sys_client;
-  ASSERT_OK(TxnSystemClient::Create(master_addrs, &txn_sys_client));
+  ASSERT_OK(TxnSystemClient::Create(cluster_->master_rpc_addrs(), &txn_sys_client));
   ASSERT_OK(txn_sys_client->CreateTxnStatusTable(100));
   ASSERT_OK(txn_sys_client->AddTxnStatusTableRange(100, 200));
 }
diff --git a/src/kudu/integration-tests/master_hms-itest.cc b/src/kudu/integration-tests/master_hms-itest.cc
index 621fea9..bcb3911 100644
--- a/src/kudu/integration-tests/master_hms-itest.cc
+++ b/src/kudu/integration-tests/master_hms-itest.cc
@@ -45,7 +45,6 @@
 #include "kudu/thrift/client.h"
 #include "kudu/transactions/txn_system_client.h"
 #include "kudu/util/monotime.h"
-#include "kudu/util/net/net_util.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
@@ -677,12 +676,8 @@ TEST_F(MasterHmsTest, TestUppercaseIdentifiers) {
 // synchronized to the HMS.
 TEST_F(MasterHmsTest, TestTransactionStatusTableDoesntSync) {
   // Create a transaction status table.
-  vector<string> master_addrs;
-  for (const auto& hp : cluster_->master_rpc_addrs()) {
-    master_addrs.emplace_back(hp.ToString());
-  }
   unique_ptr<TxnSystemClient> txn_sys_client;
-  ASSERT_OK(TxnSystemClient::Create(master_addrs, &txn_sys_client));
+  ASSERT_OK(TxnSystemClient::Create(cluster_->master_rpc_addrs(), &txn_sys_client));
   ASSERT_OK(txn_sys_client->CreateTxnStatusTable(100));
 
   // We shouldn't see the table in the HMS catalog.
diff --git a/src/kudu/integration-tests/txn_status_table-itest.cc b/src/kudu/integration-tests/txn_status_table-itest.cc
index edeb4bb..aa94827 100644
--- a/src/kudu/integration-tests/txn_status_table-itest.cc
+++ b/src/kudu/integration-tests/txn_status_table-itest.cc
@@ -57,7 +57,6 @@
 #include "kudu/tserver/tablet_server.h"
 #include "kudu/tserver/ts_tablet_manager.h"
 #include "kudu/util/monotime.h"
-#include "kudu/util/net/net_util.h"
 #include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
@@ -108,11 +107,7 @@ class TxnStatusTableITest : public KuduTest {
     ASSERT_OK(cluster_->Start());
 
     // Create the txn system client with which to communicate with the cluster.
-    vector<string> master_addrs;
-    for (const auto& hp : cluster_->master_rpc_addrs()) {
-      master_addrs.emplace_back(hp.ToString());
-    }
-    ASSERT_OK(TxnSystemClient::Create(master_addrs, &txn_sys_client_));
+    ASSERT_OK(TxnSystemClient::Create(cluster_->master_rpc_addrs(), &txn_sys_client_));
   }
 
   // Ensures that all replicas have the right table type set.
@@ -631,11 +626,7 @@ class MultiServerTxnStatusTableITest : public TxnStatusTableITest {
     opts.num_tablet_servers = 4;
     cluster_.reset(new InternalMiniCluster(env_, std::move(opts)));
     ASSERT_OK(cluster_->Start());
-    vector<string> master_addrs;
-    for (const auto& hp : cluster_->master_rpc_addrs()) {
-      master_addrs.emplace_back(hp.ToString());
-    }
-    ASSERT_OK(TxnSystemClient::Create(master_addrs, &txn_sys_client_));
+    ASSERT_OK(TxnSystemClient::Create(cluster_->master_rpc_addrs(), &txn_sys_client_));
 
     // Create the initial transaction status table partitions and start an
     // initial transaction.
diff --git a/src/kudu/master/txn_manager.cc b/src/kudu/master/txn_manager.cc
index 7d6596d..4a016bc 100644
--- a/src/kudu/master/txn_manager.cc
+++ b/src/kudu/master/txn_manager.cc
@@ -247,12 +247,7 @@ Status TxnManager::Init() {
   }
   vector<HostPort> hostports;
   RETURN_NOT_OK(server_->GetMasterHostPorts(&hostports));
-  vector<string> master_addrs;
-  master_addrs.reserve(hostports.size());
-  for (const auto& hp : hostports) {
-    master_addrs.emplace_back(hp.ToString());
-  }
-  RETURN_NOT_OK(TxnSystemClient::Create(master_addrs, &txn_sys_client_));
+  RETURN_NOT_OK(TxnSystemClient::Create(hostports, &txn_sys_client_));
   DCHECK(txn_sys_client_);
   auto s = txn_sys_client_->CreateTxnStatusTable(
       FLAGS_txn_manager_status_table_range_partition_span,
diff --git a/src/kudu/transactions/txn_system_client.cc b/src/kudu/transactions/txn_system_client.cc
index cfefae4..2d999e1 100644
--- a/src/kudu/transactions/txn_system_client.cc
+++ b/src/kudu/transactions/txn_system_client.cc
@@ -40,6 +40,7 @@
 #include "kudu/transactions/transactions.pb.h"
 #include "kudu/tserver/tserver_admin.pb.h"
 #include "kudu/util/async_util.h"
+#include "kudu/util/net/net_util.h"
 
 using kudu::client::KuduClient;
 using kudu::client::KuduSchema;
@@ -57,11 +58,15 @@ using std::vector;
 namespace kudu {
 namespace transactions {
 
-Status TxnSystemClient::Create(const vector<string>& master_addrs,
+Status TxnSystemClient::Create(const vector<HostPort>& master_addrs,
                                unique_ptr<TxnSystemClient>* sys_client) {
+  vector<string> master_strings;
+  for (const auto& hp : master_addrs) {
+    master_strings.emplace_back(hp.ToString());
+  }
   DCHECK(!master_addrs.empty());
   KuduClientBuilder builder;
-  builder.master_server_addrs(master_addrs);
+  builder.master_server_addrs(master_strings);
   client::sp::shared_ptr<KuduClient> client;
   RETURN_NOT_OK(builder.Build(&client));
   sys_client->reset(new TxnSystemClient(std::move(client)));
diff --git a/src/kudu/transactions/txn_system_client.h b/src/kudu/transactions/txn_system_client.h
index f5b3a21..acf1d67 100644
--- a/src/kudu/transactions/txn_system_client.h
+++ b/src/kudu/transactions/txn_system_client.h
@@ -32,6 +32,8 @@
 #include "kudu/util/status_callback.h"
 
 namespace kudu {
+class HostPort;
+
 namespace client {
 class KuduClient;
 class KuduTable;
@@ -55,7 +57,7 @@ class TxnStatusEntryPB;
 // calls to various servers.
 class TxnSystemClient {
  public:
-  static Status Create(const std::vector<std::string>& master_addrs,
+  static Status Create(const std::vector<HostPort>& master_addrs,
                        std::unique_ptr<TxnSystemClient>* sys_client);
 
   // Creates the transaction status table with a single range partition of the