You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2016/07/14 20:56:11 UTC
[1/2] incubator-kudu git commit: c++ client: various fixes to DDL
operations
Repository: incubator-kudu
Updated Branches:
refs/heads/master 71a5bb507 -> 2525ad094
c++ client: various fixes to DDL operations
1. Remove the num_attempts>1 workarounds found in some operations. The
CreateTable one was actually broken (the PartitionSchema::FromPB() call
triggered a CHECK) and this would be more robustly handled via the new
"exactly once" semantics.
2. In SyncLeaderMasterRpc(), take an extra ref on the master proxy.
Otherwise a concurrent master leader election could crash the client.
3. Add exponential backoff to SyncLeaderMasterRpc(). It's nothing fancy, but
I was tired of the tight loops and log spew.
Change-Id: I09768240bd04cca95d95aefe17c34d276075125b
Reviewed-on: http://gerrit.cloudera.org:8080/3608
Reviewed-by: Dan Burkert <da...@cloudera.com>
Tested-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/37f16047
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/37f16047
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/37f16047
Branch: refs/heads/master
Commit: 37f160475cc2de545c7f1dea93bc45057cfb8d7f
Parents: 71a5bb5
Author: Adar Dembo <ad...@cloudera.com>
Authored: Fri Jul 8 18:51:33 2016 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Thu Jul 14 20:54:51 2016 +0000
----------------------------------------------------------------------
src/kudu/client/client-internal.cc | 81 +++++++++-----------------------
src/kudu/client/client-internal.h | 13 +++--
src/kudu/client/client.cc | 2 -
src/kudu/client/scanner-internal.cc | 6 +--
4 files changed, 34 insertions(+), 68 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/37f16047/src/kudu/client/client-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-internal.cc b/src/kudu/client/client-internal.cc
index 66e141c..b525571 100644
--- a/src/kudu/client/client-internal.cc
+++ b/src/kudu/client/client-internal.cc
@@ -37,6 +37,7 @@
#include "kudu/master/master.proxy.h"
#include "kudu/rpc/rpc.h"
#include "kudu/rpc/rpc_controller.h"
+#include "kudu/rpc/rpc_header.pb.h"
#include "kudu/util/net/dns_resolver.h"
#include "kudu/util/net/net_util.h"
#include "kudu/util/thread_restrictions.h"
@@ -69,6 +70,7 @@ using master::ListTabletServersResponsePB;
using master::MasterErrorPB;
using master::MasterFeatures;
using master::MasterServiceProxy;
+using rpc::ErrorStatusPB;
using rpc::Rpc;
using rpc::RpcController;
using strings::Substitute;
@@ -132,7 +134,6 @@ Status KuduClient::Data::SyncLeaderMasterRpc(
KuduClient* client,
const ReqClass& req,
RespClass* resp,
- int* num_attempts,
const char* func_name,
const boost::function<Status(MasterServiceProxy*,
const ReqClass&,
@@ -141,9 +142,14 @@ Status KuduClient::Data::SyncLeaderMasterRpc(
vector<uint32_t> required_feature_flags) {
DCHECK(deadline.Initialized());
- while (true) {
+ for (int num_attempts = 0;; num_attempts++) {
RpcController rpc;
+ // Sleep if necessary.
+ if (num_attempts > 0) {
+ SleepFor(ComputeExponentialBackoff(num_attempts));
+ }
+
// Have we already exceeded our deadline?
MonoTime now = MonoTime::Now(MonoTime::FINE);
if (deadline.ComesBefore(now)) {
@@ -160,15 +166,23 @@ Status KuduClient::Data::SyncLeaderMasterRpc(
rpc_deadline.AddDelta(client->default_rpc_timeout());
rpc.set_deadline(MonoTime::Earliest(rpc_deadline, deadline));
- if (num_attempts != nullptr) {
- ++*num_attempts;
- }
-
for (uint32_t required_feature_flag : required_feature_flags) {
rpc.RequireServerFeature(required_feature_flag);
}
- Status s = func(master_proxy_.get(), req, resp, &rpc);
+ // Take a ref to the proxy in case it disappears from underneath us.
+ shared_ptr<MasterServiceProxy> proxy(master_proxy());
+
+ Status s = func(proxy.get(), req, resp, &rpc);
+ if (s.IsRemoteError()) {
+ const ErrorStatusPB* err = rpc.error_response();
+ if (err &&
+ err->has_code() &&
+ err->code() == ErrorStatusPB::ERROR_SERVER_TOO_BUSY) {
+ continue;
+ }
+ }
+
if (s.IsNetworkError()) {
LOG(WARNING) << "Unable to send the request (" << req.ShortDebugString()
<< ") to leader Master (" << leader_master_hostport().ToString()
@@ -221,7 +235,6 @@ Status KuduClient::Data::SyncLeaderMasterRpc(
KuduClient* client,
const ListTablesRequestPB& req,
ListTablesResponsePB* resp,
- int* num_attempts,
const char* func_name,
const boost::function<Status(MasterServiceProxy*,
const ListTablesRequestPB&,
@@ -234,7 +247,6 @@ Status KuduClient::Data::SyncLeaderMasterRpc(
KuduClient* client,
const ListTabletServersRequestPB& req,
ListTabletServersResponsePB* resp,
- int* num_attempts,
const char* func_name,
const boost::function<Status(MasterServiceProxy*,
const ListTabletServersRequestPB&,
@@ -349,52 +361,15 @@ Status KuduClient::Data::CreateTable(KuduClient* client,
bool has_range_partition_bounds) {
CreateTableResponsePB resp;
- int attempts = 0;
vector<uint32_t> features;
if (has_range_partition_bounds) {
features.push_back(MasterFeatures::RANGE_PARTITION_BOUNDS);
}
Status s = SyncLeaderMasterRpc<CreateTableRequestPB, CreateTableResponsePB>(
- deadline, client, req, &resp, &attempts, "CreateTable", &MasterServiceProxy::CreateTable,
+ deadline, client, req, &resp, "CreateTable", &MasterServiceProxy::CreateTable,
features);
RETURN_NOT_OK(s);
if (resp.has_error()) {
- if (resp.error().code() == MasterErrorPB::TABLE_ALREADY_PRESENT && attempts > 1) {
- // If the table already exists and the number of attempts is >
- // 1, then it means we may have succeeded in creating the
- // table, but client didn't receive the successful
- // response (e.g., due to failure before the successful
- // response could be sent back, or due to a I/O pause or a
- // network blip leading to a timeout, etc...)
- KuduSchema actual_schema;
- string table_id;
- PartitionSchema actual_partition_schema;
- RETURN_NOT_OK_PREPEND(
- GetTableSchema(client, req.name(), deadline, &actual_schema,
- &actual_partition_schema, &table_id),
- Substitute("Unable to check the schema of table $0", req.name()));
- if (!schema.Equals(actual_schema)) {
- string msg = Substitute("Table $0 already exists with a different "
- "schema. Requested schema was: $1, actual schema is: $2",
- req.name(), schema.schema_->ToString(), actual_schema.schema_->ToString());
- LOG(ERROR) << msg;
- return Status::AlreadyPresent(msg);
- } else {
- PartitionSchema partition_schema;
- RETURN_NOT_OK(PartitionSchema::FromPB(req.partition_schema(),
- *schema.schema_, &partition_schema));
- if (!partition_schema.Equals(actual_partition_schema)) {
- string msg = Substitute("Table $0 already exists with a different partition schema. "
- "Requested partition schema was: $1, actual partition schema is: $2",
- req.name(), partition_schema.DebugString(*schema.schema_),
- actual_partition_schema.DebugString(*actual_schema.schema_));
- LOG(ERROR) << msg;
- return Status::AlreadyPresent(msg);
- } else {
- return Status::OK();
- }
- }
- }
return StatusFromPB(resp.error().status());
}
return Status::OK();
@@ -416,7 +391,6 @@ Status KuduClient::Data::IsCreateTableInProgress(KuduClient* client,
client,
req,
&resp,
- nullptr,
"IsCreateTableDone",
&MasterServiceProxy::IsCreateTableDone,
{});
@@ -447,20 +421,13 @@ Status KuduClient::Data::DeleteTable(KuduClient* client,
const MonoTime& deadline) {
DeleteTableRequestPB req;
DeleteTableResponsePB resp;
- int attempts = 0;
req.mutable_table()->set_table_name(table_name);
Status s = SyncLeaderMasterRpc<DeleteTableRequestPB, DeleteTableResponsePB>(
deadline, client, req, &resp,
- &attempts, "DeleteTable", &MasterServiceProxy::DeleteTable, {});
+ "DeleteTable", &MasterServiceProxy::DeleteTable, {});
RETURN_NOT_OK(s);
if (resp.has_error()) {
- if (resp.error().code() == MasterErrorPB::TABLE_NOT_FOUND && attempts > 1) {
- // A prior attempt to delete the table has succeeded, but
- // appeared as a failure to the client due to, e.g., an I/O or
- // network issue.
- return Status::OK();
- }
return StatusFromPB(resp.error().status());
}
return Status::OK();
@@ -476,7 +443,6 @@ Status KuduClient::Data::AlterTable(KuduClient* client,
client,
req,
&resp,
- nullptr,
"AlterTable",
&MasterServiceProxy::AlterTable,
{});
@@ -506,7 +472,6 @@ Status KuduClient::Data::IsAlterTableInProgress(KuduClient* client,
client,
req,
&resp,
- nullptr,
"IsAlterTableDone",
&MasterServiceProxy::IsAlterTableDone,
{});
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/37f16047/src/kudu/client/client-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-internal.h b/src/kudu/client/client-internal.h
index 861b30f..7bf001a 100644
--- a/src/kudu/client/client-internal.h
+++ b/src/kudu/client/client-internal.h
@@ -17,6 +17,7 @@
#ifndef KUDU_CLIENT_CLIENT_INTERNAL_H
#define KUDU_CLIENT_CLIENT_INTERNAL_H
+#include <algorithm>
#include <boost/function.hpp>
#include <set>
#include <string>
@@ -168,9 +169,6 @@ class KuduClient::Data {
// errors, timeouts, or leadership issues.
// 3) 'deadline' (if initialized) elapses.
//
- // If 'num_attempts' is not NULL, it will be incremented on every
- // attempt (successful or not) to call 'func'.
- //
// NOTE: 'rpc_timeout' is a per-call timeout, while 'deadline' is a
// per operation deadline. If 'deadline' is not initialized, 'func' is
// retried forever. If 'deadline' expires, 'func_name' is included in
@@ -181,13 +179,20 @@ class KuduClient::Data {
KuduClient* client,
const ReqClass& req,
RespClass* resp,
- int* num_attempts,
const char* func_name,
const boost::function<Status(master::MasterServiceProxy*,
const ReqClass&, RespClass*,
rpc::RpcController*)>& func,
std::vector<uint32_t> required_feature_flags);
+ // Exponential backoff with jitter anchored between 10ms and 20ms, and an
+ // upper bound between 2.5s and 5s.
+ static MonoDelta ComputeExponentialBackoff(int num_attempts) {
+ return MonoDelta::FromMilliseconds(
+ (10 + rand() % 10) * static_cast<int>(
+ std::pow(2.0, std::min(8, num_attempts - 1))));
+ }
+
// The unique id of this client.
std::string client_id_;
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/37f16047/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index c884351..8507817 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -309,7 +309,6 @@ Status KuduClient::ListTabletServers(vector<KuduTabletServer*>* tablet_servers)
this,
req,
&resp,
- nullptr,
"ListTabletServers",
&MasterServiceProxy::ListTabletServers,
{});
@@ -343,7 +342,6 @@ Status KuduClient::ListTables(vector<string>* tables,
this,
req,
&resp,
- nullptr,
"ListTables",
&MasterServiceProxy::ListTables,
{});
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/37f16047/src/kudu/client/scanner-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/scanner-internal.cc b/src/kudu/client/scanner-internal.cc
index 858799a..68c53bc 100644
--- a/src/kudu/client/scanner-internal.cc
+++ b/src/kudu/client/scanner-internal.cc
@@ -122,10 +122,8 @@ Status KuduScanner::Data::HandleError(const ScanRpcStatus& err,
}
if (backoff) {
- // Exponential backoff with jitter anchored between 10ms and 20ms, and an
- // upper bound between 2.5s and 5s.
- MonoDelta sleep = MonoDelta::FromMilliseconds(
- (10 + rand() % 10) * static_cast<int>(std::pow(2.0, std::min(8, scan_attempts_ - 1))));
+ MonoDelta sleep =
+ KuduClient::Data::ComputeExponentialBackoff(scan_attempts_);
MonoTime now = MonoTime::Now(MonoTime::FINE);
now.AddDelta(sleep);
if (deadline.ComesBefore(now)) {
[2/2] incubator-kudu git commit: master: fix initialization race with
consensus RPCs
Posted by ad...@apache.org.
master: fix initialization race with consensus RPCs
The master initialization order is such that the various RPC services are
brought up before the catalog manager. With multiple masters, it's possible
for a master to receive a consensus-related RPC at this delicate time,
causing a crash.
I spent some time trying to unravel this mess but it proved too thorny, so I
relaxed the CHECK instead. The other masters appear to cope with this error.
There's no explicit test here, but this path is exercised by a stress test
in a follow-on patch.
Change-Id: I3d1276dd4d3c2f555d63d97d7a16d54181a352b7
Reviewed-on: http://gerrit.cloudera.org:8080/3605
Reviewed-by: Dan Burkert <da...@cloudera.com>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/2525ad09
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/2525ad09
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/2525ad09
Branch: refs/heads/master
Commit: 2525ad094234e6bc901b8bc544801ca00e8f411e
Parents: 37f1604
Author: Adar Dembo <ad...@cloudera.com>
Authored: Fri Jul 8 18:52:38 2016 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Thu Jul 14 20:55:22 2016 +0000
----------------------------------------------------------------------
src/kudu/master/catalog_manager.cc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/2525ad09/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 819b3a3..07d8487 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -1764,7 +1764,9 @@ Status CatalogManager::GetTabletPeer(const string& tablet_id,
// Note: CatalogManager has only one table, 'sys_catalog', with only
// one tablet.
shared_lock<LockType> l(lock_);
- CHECK(sys_catalog_.get() != nullptr) << "sys_catalog_ must be initialized!";
+ if (!sys_catalog_) {
+ return Status::ServiceUnavailable("Systable not yet initialized");
+ }
if (sys_catalog_->tablet_id() == tablet_id) {
*tablet_peer = sys_catalog_->tablet_peer();
} else {