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 2019/04/02 21:17:39 UTC
[kudu] branch master updated (23db50d -> de5e98b)
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 23db50d [curl] Fix error status of timeout
new 9a98aa0 docs: remove extraneous space
new 49f0196 client: remove a warning
new 0ce9b05 [TS heartbeater] avoid reconnecting to master too often
new de5e98b [master_sentry-itest] add description into AuthzFuncs
The 4 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:
docs/administration.adoc | 2 +-
src/kudu/client/schema.cc | 2 +-
src/kudu/integration-tests/master_sentry-itest.cc | 273 ++++++++++++----------
src/kudu/tserver/heartbeater.cc | 19 +-
4 files changed, 162 insertions(+), 134 deletions(-)
[kudu] 04/04: [master_sentry-itest] add description into AuthzFuncs
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 de5e98b4bbfc88f3cae20ba67231a5da83e4cbc7
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Apr 2 09:08:51 2019 -0700
[master_sentry-itest] add description into AuthzFuncs
Added 'description' field into AuthzFuncs structure so in case of
test failure it's easier to find which one failed. For example,
former message
[ FAILED ] AuthzFuncCombinations/AuthzErrorHandlingTest.TestNonExistentTable/0, where GetParam() = 64-byte object <60-CF 44-00 00-00 00-00 00-00 00-00 00-00 00-00 70-84 46-00 00-00 00-00 D0-83 46-00 00-00 00-00 B0-CF 44-00 00-00 00-00 00-00 00-00 00-00 00-00 70-84 46-00 00-00 00-00 D0-83 46-00 00-00 00-00>
became
[ FAILED ] AuthzFuncCombinations/AuthzErrorHandlingTest.TestNonExistentTable/0, where GetParam() = DeleteTable
This patch doesn't contain any functional changes.
Change-Id: I08934c8e74b2d6f72dd703a304e48befffd2e6fc
Reviewed-on: http://gerrit.cloudera.org:8080/12912
Reviewed-by: Hao Hao <ha...@cloudera.com>
Tested-by: Kudu Jenkins
---
src/kudu/integration-tests/master_sentry-itest.cc | 273 ++++++++++++----------
1 file changed, 144 insertions(+), 129 deletions(-)
diff --git a/src/kudu/integration-tests/master_sentry-itest.cc b/src/kudu/integration-tests/master_sentry-itest.cc
index 939a260..74b853f 100644
--- a/src/kudu/integration-tests/master_sentry-itest.cc
+++ b/src/kudu/integration-tests/master_sentry-itest.cc
@@ -17,6 +17,7 @@
#include <functional>
#include <memory>
+#include <ostream>
#include <string>
#include <unordered_set>
#include <utility>
@@ -71,6 +72,7 @@ using kudu::master::TabletLocationsPB;
using kudu::rpc::UserCredentials;
using kudu::sentry::SentryClient;
using std::function;
+using std::ostream;
using std::string;
using std::unique_ptr;
using std::unordered_set;
@@ -353,7 +355,12 @@ typedef function<Status(SentryITestBase*, const string&, const string&)> Privile
struct AuthzFuncs {
OperatorFunc do_action;
PrivilegeFunc grant_privileges;
+ string description;
};
+ostream& operator <<(ostream& out, const AuthzFuncs& d) {
+ out << d.description;
+ return out;
+}
// A description of an authorization process, including the protected resource (table),
// the operation function, as well as the privilege granting function.
@@ -363,6 +370,10 @@ struct AuthzDescriptor {
string table_name;
string new_table_name;
};
+ostream& operator <<(ostream& out, const AuthzDescriptor& d) {
+ out << d.funcs.description;
+ return out;
+}
// Test basic master authorization enforcement with Sentry and HMS integration
// enabled.
@@ -479,100 +490,102 @@ TEST_P(TestAuthzTable, TestAuthorizeTable) {
ASSERT_TRUE(s.IsNetworkError()) << s.ToString();
}
+static const AuthzDescriptor kAuthzCombinations[] = {
+ {
+ {
+ &SentryITestBase::CreateTable,
+ &SentryITestBase::GrantCreateTablePrivilege,
+ "CreateTable",
+ },
+ SentryITestBase::kDatabaseName,
+ "new_table",
+ ""
+ },
+ {
+ {
+ &SentryITestBase::DeleteTable,
+ &SentryITestBase::GrantDropTablePrivilege,
+ "DeleteTable",
+ },
+ SentryITestBase::kDatabaseName,
+ SentryITestBase::kTableName,
+ ""
+ },
+ {
+ {
+ &SentryITestBase::AlterTable,
+ &SentryITestBase::GrantAlterTablePrivilege,
+ "AlterTable",
+ },
+ SentryITestBase::kDatabaseName,
+ SentryITestBase::kTableName,
+ ""
+ },
+ {
+ {
+ &SentryITestBase::RenameTable,
+ &SentryITestBase::GrantRenameTablePrivilege,
+ "RenameTable",
+ },
+ SentryITestBase::kDatabaseName,
+ SentryITestBase::kTableName,
+ "new_table"
+ },
+ {
+ {
+ &SentryITestBase::GetTableSchema,
+ &SentryITestBase::GrantGetMetadataTablePrivilege,
+ "GetTableSchema",
+ },
+ SentryITestBase::kDatabaseName,
+ SentryITestBase::kTableName,
+ ""
+ },
+ {
+ {
+ &SentryITestBase::GetTableLocations,
+ &SentryITestBase::GrantGetMetadataTablePrivilege,
+ "GetTableLocations",
+ },
+ SentryITestBase::kDatabaseName,
+ SentryITestBase::kTableName,
+ ""
+ },
+ {
+ {
+ &SentryITestBase::GetTabletLocations,
+ &SentryITestBase::GrantGetMetadataTablePrivilege,
+ "GetTabletLocations",
+ },
+ SentryITestBase::kDatabaseName,
+ SentryITestBase::kTableName,
+ ""
+ },
+ {
+ {
+ &SentryITestBase::IsCreateTableDone,
+ &SentryITestBase::GrantGetMetadataTablePrivilege,
+ "IsCreateTableDone",
+ },
+ SentryITestBase::kDatabaseName,
+ SentryITestBase::kTableName,
+ ""
+ },
+ {
+ {
+ &SentryITestBase::IsAlterTableDone,
+ &SentryITestBase::GrantGetMetadataTablePrivilege,
+ "IsAlterTableDone",
+ },
+ SentryITestBase::kDatabaseName,
+ SentryITestBase::kTableName,
+ ""
+ },
+};
+
INSTANTIATE_TEST_CASE_P(AuthzCombinations,
TestAuthzTable,
- ::testing::Values(
-
- AuthzDescriptor {
- AuthzFuncs {
- &SentryITestBase::CreateTable,
- &SentryITestBase::GrantCreateTablePrivilege,
- },
- SentryITestBase::kDatabaseName,
- "new_table",
- "",
- },
-
- AuthzDescriptor {
- AuthzFuncs {
- &SentryITestBase::DeleteTable,
- &SentryITestBase::GrantDropTablePrivilege,
- },
- SentryITestBase::kDatabaseName,
- SentryITestBase::kTableName,
- "",
- },
-
- AuthzDescriptor {
- AuthzFuncs {
- &SentryITestBase::AlterTable,
- &SentryITestBase::GrantAlterTablePrivilege,
- },
- SentryITestBase::kDatabaseName,
- SentryITestBase::kTableName,
- "",
- },
-
- AuthzDescriptor{
- AuthzFuncs {
- &SentryITestBase::RenameTable,
- &SentryITestBase::GrantRenameTablePrivilege,
- },
- SentryITestBase::kDatabaseName,
- SentryITestBase::kTableName,
- "new_table",
- },
-
- AuthzDescriptor {
- AuthzFuncs {
- &SentryITestBase::GetTableSchema,
- &SentryITestBase::GrantGetMetadataTablePrivilege,
- },
- SentryITestBase::kDatabaseName,
- SentryITestBase::kTableName,
- "",
- },
-
- AuthzDescriptor {
- AuthzFuncs {
- &SentryITestBase::GetTableLocations,
- &SentryITestBase::GrantGetMetadataTablePrivilege,
- },
- SentryITestBase::kDatabaseName,
- SentryITestBase::kTableName,
- "",
- },
-
- AuthzDescriptor {
- AuthzFuncs {
- &SentryITestBase::GetTabletLocations,
- &SentryITestBase::GrantGetMetadataTablePrivilege,
- },
- SentryITestBase::kDatabaseName,
- SentryITestBase::kTableName,
- ""
- },
-
- AuthzDescriptor {
- AuthzFuncs {
- &SentryITestBase::IsCreateTableDone,
- &SentryITestBase::GrantGetMetadataTablePrivilege,
- },
- SentryITestBase::kDatabaseName,
- SentryITestBase::kTableName,
- "",
- },
-
- AuthzDescriptor {
- AuthzFuncs {
- &SentryITestBase::IsAlterTableDone,
- &SentryITestBase::GrantGetMetadataTablePrivilege,
- },
- SentryITestBase::kDatabaseName,
- SentryITestBase::kTableName,
- "",
- }
-));
+ ::testing::ValuesIn(kAuthzCombinations));
// Test that when the client passes a table identifier with the table name
// and table ID refer to different tables, the client needs permission on
@@ -639,43 +652,45 @@ TEST_P(AuthzErrorHandlingTest, TestNonExistentTable) {
ASSERT_TRUE(s.IsNotFound()) << s.ToString();
}
+static const AuthzFuncs kAuthzFuncCombinations[] = {
+ {
+ &SentryITestBase::DeleteTable,
+ &SentryITestBase::GrantDropTablePrivilege,
+ "DeleteTable"
+ },
+ {
+ &SentryITestBase::AlterTable,
+ &SentryITestBase::GrantAlterTablePrivilege,
+ "AlterTable"
+ },
+ {
+ &SentryITestBase::RenameTable,
+ &SentryITestBase::GrantRenameTablePrivilege,
+ "RenameTable"
+ },
+ {
+ &SentryITestBase::GetTableSchema,
+ &SentryITestBase::GrantGetMetadataTablePrivilege,
+ "GetTableSchema"
+ },
+ {
+ &SentryITestBase::GetTableLocations,
+ &SentryITestBase::GrantGetMetadataTablePrivilege,
+ "GetTableLocations"
+ },
+ {
+ &SentryITestBase::IsCreateTableDone,
+ &SentryITestBase::GrantGetMetadataTablePrivilege,
+ "IsCreateTableDone"
+ },
+ {
+ &SentryITestBase::IsAlterTableDone,
+ &SentryITestBase::GrantGetMetadataTablePrivilege,
+ "IsAlterTableDone"
+ },
+};
+
INSTANTIATE_TEST_CASE_P(AuthzFuncCombinations,
AuthzErrorHandlingTest,
- ::testing::Values(
-
- AuthzFuncs {
- &SentryITestBase::DeleteTable,
- &SentryITestBase::GrantDropTablePrivilege,
- },
-
- AuthzFuncs {
- &SentryITestBase::AlterTable,
- &SentryITestBase::GrantAlterTablePrivilege,
- },
-
- AuthzFuncs {
- &SentryITestBase::RenameTable,
- &SentryITestBase::GrantRenameTablePrivilege,
- },
-
- AuthzFuncs {
- &SentryITestBase::GetTableSchema,
- &SentryITestBase::GrantGetMetadataTablePrivilege,
- },
-
- AuthzFuncs {
- &SentryITestBase::GetTableLocations,
- &SentryITestBase::GrantGetMetadataTablePrivilege,
- },
-
- AuthzFuncs {
- &SentryITestBase::IsCreateTableDone,
- &SentryITestBase::GrantGetMetadataTablePrivilege,
- },
-
- AuthzFuncs {
- &SentryITestBase::IsAlterTableDone,
- &SentryITestBase::GrantGetMetadataTablePrivilege,
- }
-));
+ ::testing::ValuesIn(kAuthzFuncCombinations));
} // namespace kudu
[kudu] 01/04: docs: remove extraneous space
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 9a98aa061acc13b2636769047e930cd9bb6d4035
Author: Andrew Wong <aw...@apache.org>
AuthorDate: Mon Apr 1 10:20:12 2019 -0700
docs: remove extraneous space
Change-Id: I0a2d4b6b8e5e09affc4c1a239e3a61d4c58a72f8
Reviewed-on: http://gerrit.cloudera.org:8080/12906
Reviewed-by: Grant Henke <gr...@apache.org>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Andrew Wong <aw...@cloudera.com>
---
docs/administration.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/administration.adoc b/docs/administration.adoc
index 725b6c6..2794b40 100644
--- a/docs/administration.adoc
+++ b/docs/administration.adoc
@@ -1009,7 +1009,7 @@ following steps:
+
[source,bash]
----
-$ sudo -u kudu kudu fs update_dirs --force --fs_wal_dir=/wals --fs_data_dirs=/data/1,/data/2, /data/3
+$ sudo -u kudu kudu fs update_dirs --force --fs_wal_dir=/wals --fs_data_dirs=/data/1,/data/2,/data/3
----
+
[kudu] 03/04: [TS heartbeater] avoid reconnecting to master too
often
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 0ce9b05d046da01b7b098818fe5e42c1f40e9ac2
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Fri Mar 1 17:19:18 2019 -0800
[TS heartbeater] avoid reconnecting to master too often
With this patch, the heartbeater thread in tservers doesn't reset
its master proxy and reconnect to master (re-negotiating a connection)
every heartbeat under certain conditions. In particular, that happened
if the master was accepting connections and responding to Ping RPC
requests, but was not able to process TS heartbeats properly because
it was still bootstrapping.
E.g., when running RemoteKsckTest.TestClusterWithLocation test scenario
for TSAN builds, I sometimes saw log messages like the following
(the test sets FLAGS_heartbeat_interval_ms = 10):
I0301 20:29:11.932394 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:11.944639 3671 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:11.946904 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:11.960994 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:11.964995 3819 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:11.972220 3671 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:11.974987 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:11.988946 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:11.991653 3671 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.003091 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.017015 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.017540 3671 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.031175 3819 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.031175 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.046165 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.059644 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.073026 3819 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.075335 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.077802 3671 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.089138 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.101193 3671 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.102268 3819 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.104634 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.118392 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.132237 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.147235 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.165709 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.171120 3819 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.179481 3746 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
I0301 20:29:12.191591 3671 heartbeater.cc:345] Connected to a master server at 127.3.75.254:36221
It turned out the counter of the consecutively failed heartbeats kept
increasing because the master was responding with ServiceUnavailable
to incoming TS hearbeats. The prior version of the code did reset
the master proxy every failed heartbeat since
FLAGS_heartbeat_max_failures_before_backoff consecutive errors happened,
and that was the reason behind frequent re-connections to the cluster.
For testing, I just verified that the TS heartbeater no longer behaves
like described above under the same scenarios and conditions.
Change-Id: I961ae453ffd6ce343574ce58cb0e13fdad218078
Reviewed-on: http://gerrit.cloudera.org:8080/12647
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley <wd...@gmail.com>
---
src/kudu/tserver/heartbeater.cc | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/kudu/tserver/heartbeater.cc b/src/kudu/tserver/heartbeater.cc
index 0b1587f..62856fd 100644
--- a/src/kudu/tserver/heartbeater.cc
+++ b/src/kudu/tserver/heartbeater.cc
@@ -588,10 +588,23 @@ void Heartbeater::Thread::RunThread() {
<< Substitute("Failed to heartbeat to $0 ($1 consecutive failures): $2",
master_address_.ToString(), consecutive_failed_heartbeats_, err_msg);
consecutive_failed_heartbeats_++;
- // If we encountered a network error (e.g., connection
- // refused), try reconnecting.
+
+ // Reset master proxy if too many heartbeats failed in a row. The idea
+ // is to do so when HBs have already backed off from the 'fast HB retry'
+ // behavior. This might be useful in situations when NetworkError isn't
+ // going to be received from the remote side any soon, so resetting
+ // the proxy is a viable alternative to try.
+ //
+ // The 'num_failures_to_reset_proxy' is the number of consecutive errors
+ // to happen before the master proxy is reset again.
+ const auto num_failures_to_reset_proxy =
+ FLAGS_heartbeat_max_failures_before_backoff * 10;
+
+ // If we encountered a network error (e.g., connection refused) or
+ // there were too many consecutive errors while sending heartbeats since
+ // the proxy was reset last time, try reconnecting.
if (s.IsNetworkError() ||
- consecutive_failed_heartbeats_ >= FLAGS_heartbeat_max_failures_before_backoff) {
+ consecutive_failed_heartbeats_ % num_failures_to_reset_proxy == 0) {
proxy_.reset();
}
string msg;
[kudu] 02/04: client: remove a warning
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 49f019668c1d148565e85a8b1b396b018d932a9f
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Mon Apr 1 20:45:30 2019 -0700
client: remove a warning
../../src/kudu/client/schema.cc:624:27: warning: moving a temporary object prevents copy elision [-Wpessimizing-move]
std::move(comment ? boost::optional<string>(*comment) : boost::none));
^
../../src/kudu/client/schema.cc:624:27: note: remove std::move call here
std::move(comment ? boost::optional<string>(*comment) : boost::none));
^~~~~~~~~~ ~
1 warning generated.
Change-Id: I15b60c8a4b245fa694a1abc5c60bced66ed3039b
Reviewed-on: http://gerrit.cloudera.org:8080/12909
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
---
src/kudu/client/schema.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kudu/client/schema.cc b/src/kudu/client/schema.cc
index faa7c53..d6b90d2 100644
--- a/src/kudu/client/schema.cc
+++ b/src/kudu/client/schema.cc
@@ -596,7 +596,7 @@ KuduColumnSchema::KuduColumnSchema(const string &name,
is_nullable,
default_value, default_value, attr_private,
type_attr_private,
- std::move(comment ? boost::optional<string>(*comment) : boost::none));
+ comment ? boost::optional<string>(*comment) : boost::none);
}
KuduColumnSchema::KuduColumnSchema(const KuduColumnSchema& other)