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 2018/10/23 18:35:27 UTC
[1/2] kudu git commit: [sentry] improve SentryAction
Repository: kudu
Updated Branches:
refs/heads/master 9c763a6a7 -> 7d65f495d
[sentry] improve SentryAction
A follow-up to Adar's suggestions in
https://gerrit.cloudera.org/#/c/11656/.
Change-Id: Icc6c0fb2a33a24745936c96363390e776b24513f
Reviewed-on: http://gerrit.cloudera.org:8080/11720
Tested-by: Hao Hao <ha...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/4abc211f
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/4abc211f
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/4abc211f
Branch: refs/heads/master
Commit: 4abc211f6af7598c338cea4b8e8f913cde89c326
Parents: 9c763a6
Author: Hao Hao <ha...@cloudera.com>
Authored: Fri Oct 19 12:41:21 2018 -0700
Committer: Hao Hao <ha...@cloudera.com>
Committed: Mon Oct 22 20:55:20 2018 +0000
----------------------------------------------------------------------
src/kudu/sentry/sentry_action-test.cc | 9 ++--
src/kudu/sentry/sentry_action.cc | 75 +++++++++++++++++++-----------
src/kudu/sentry/sentry_action.h | 24 ++++++----
3 files changed, 68 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/4abc211f/src/kudu/sentry/sentry_action-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/sentry_action-test.cc b/src/kudu/sentry/sentry_action-test.cc
index d646cc4..4679537 100644
--- a/src/kudu/sentry/sentry_action-test.cc
+++ b/src/kudu/sentry/sentry_action-test.cc
@@ -57,7 +57,7 @@ TEST(SentryActionTest, TestImplyAction) {
ASSERT_FALSE(metadata.Imply(action));
}
- actions.insert(actions.end(), metadata);
+ actions.emplace_back(metadata);
for (const auto& action : actions) {
// Action ALL implies all other actions.
ASSERT_TRUE(all.Imply(action));
@@ -72,16 +72,15 @@ TEST(SentryActionTest, TestImplyAction) {
TEST(SentryActionTest, TestFromString) {
// Action '*' equals to ALL.
- SentryAction wildcard_action;
- ASSERT_OK(wildcard_action.FromString(SentryAction::kWildCard));
- SentryAction wildcard(wildcard_action);
+ SentryAction wildcard;
+ ASSERT_OK(SentryAction::FromString(SentryAction::kWildCard, &wildcard));
SentryAction all(SentryAction::Action::ALL);
ASSERT_TRUE(all.Imply(wildcard));
ASSERT_TRUE(wildcard.Imply(all));
// Unsupported action, such as '+', throws invalid argument error.
SentryAction invalid_action;
- Status s = invalid_action.FromString("+");
+ Status s = SentryAction::FromString("+", &invalid_action);
ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/4abc211f/src/kudu/sentry/sentry_action.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/sentry_action.cc b/src/kudu/sentry/sentry_action.cc
index 2fe0a2a..73d314a 100644
--- a/src/kudu/sentry/sentry_action.cc
+++ b/src/kudu/sentry/sentry_action.cc
@@ -17,6 +17,7 @@
#include "kudu/sentry/sentry_action.h"
+#include <ostream>
#include <string>
#include <boost/algorithm/string/predicate.hpp>
@@ -30,6 +31,27 @@ using strings::Substitute;
namespace kudu {
namespace sentry {
+const char* ActionToString(SentryAction::Action action) {
+ switch (action) {
+ case SentryAction::Action::UNINITIALIZED: return "UNINITIALIZED";
+ case SentryAction::Action::ALL: return "ALL";
+ case SentryAction::Action::METADATA: return "METADATA";
+ case SentryAction::Action::SELECT: return "SELECT";
+ case SentryAction::Action::INSERT: return "INSERT";
+ case SentryAction::Action::UPDATE: return "UPDATE";
+ case SentryAction::Action::DELETE: return "DELETE";
+ case SentryAction::Action::ALTER: return "ALTER";
+ case SentryAction::Action::CREATE: return "CREATE";
+ case SentryAction::Action::DROP: return "DROP";
+ case SentryAction::Action::OWNER: return "OWNER";
+ }
+ return "<cannot reach here>";
+}
+
+std::ostream& operator<<(std::ostream& o, SentryAction::Action action) {
+ return o << ActionToString(action);
+}
+
const char* const SentryAction::kWildCard = "*";
SentryAction::SentryAction()
@@ -40,43 +62,42 @@ SentryAction::SentryAction(Action action)
: action_(action) {
}
-Status SentryAction::FromString(const string& action) {
+Status SentryAction::FromString(const string& str, SentryAction* action) {
// Consider action '*' equals to ALL to be compatible with the existing
// Java Sentry client.
//
// See org.apache.sentry.api.service.thrift.SentryPolicyServiceClientDefaultImpl.
- if (boost::iequals(action, "ALL") || action == kWildCard) {
- action_ = Action::ALL;
- } else if (boost::iequals(action, "METADATA")) {
- action_ = Action::METADATA;
- } else if (boost::iequals(action, "SELECT")) {
- action_ = Action::SELECT;
- } else if (boost::iequals(action, "INSERT")) {
- action_ = Action::INSERT;
- } else if (boost::iequals(action, "UPDATE")) {
- action_ = Action::UPDATE;
- } else if (boost::iequals(action, "DELETE")) {
- action_ = Action::DELETE;
- } else if (boost::iequals(action, "ALTER")) {
- action_ = Action::ALTER;
- } else if (boost::iequals(action, "CREATE")) {
- action_ = Action::CREATE;
- } else if (boost::iequals(action, "DROP")) {
- action_ = Action::DROP;
- } else if (boost::iequals(action, "OWNER")) {
- action_ = Action::OWNER;
+ if (boost::iequals(str, "ALL") || str == kWildCard) {
+ action->action_ = Action::ALL;
+ } else if (boost::iequals(str, "METADATA")) {
+ action->action_ = Action::METADATA;
+ } else if (boost::iequals(str, "SELECT")) {
+ action->action_ = Action::SELECT;
+ } else if (boost::iequals(str, "INSERT")) {
+ action->action_ = Action::INSERT;
+ } else if (boost::iequals(str, "UPDATE")) {
+ action->action_ = Action::UPDATE;
+ } else if (boost::iequals(str, "DELETE")) {
+ action->action_ = Action::DELETE;
+ } else if (boost::iequals(str, "ALTER")) {
+ action->action_ = Action::ALTER;
+ } else if (boost::iequals(str, "CREATE")) {
+ action->action_ = Action::CREATE;
+ } else if (boost::iequals(str, "DROP")) {
+ action->action_ = Action::DROP;
+ } else if (boost::iequals(str, "OWNER")) {
+ action->action_ = Action::OWNER;
} else {
- return Status::InvalidArgument(Substitute("unknown SentryAction: $0",
- action));
+ return Status::InvalidArgument(Substitute("unknown SentryAction: $0", str));
}
return Status::OK();
}
bool SentryAction::Imply(const SentryAction& other) const {
- // Any action must be initialized.
- CHECK(action() != Action::UNINITIALIZED);
- CHECK(other.action() != Action::UNINITIALIZED);
+ // Every action must be initialized.
+ CHECK_NE(action(), Action::UNINITIALIZED);
+ CHECK_NE(other.action(), Action::UNINITIALIZED);
// Action ALL and OWNER subsume every other action.
if (action() == Action::ALL ||
@@ -84,7 +105,7 @@ bool SentryAction::Imply(const SentryAction& other) const {
return true;
}
- // Any action subsumes Action METADATA
+ // Any action subsumes Action METADATA.
if (other.action() == Action::METADATA) {
return true;
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/4abc211f/src/kudu/sentry/sentry_action.h
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/sentry_action.h b/src/kudu/sentry/sentry_action.h
index 957ac56..313868a 100644
--- a/src/kudu/sentry/sentry_action.h
+++ b/src/kudu/sentry/sentry_action.h
@@ -17,6 +17,7 @@
#pragma once
+#include <iosfwd>
#include <string>
#include "kudu/util/status.h"
@@ -24,12 +25,15 @@
namespace kudu {
namespace sentry {
-// A replication of Sentry Action, which is the operation taken
-// on an authorizable/object. In this case, HiveSQL model is chosen
-// to define the actions. One action can imply another following rules
-// defined in Imply().
+// A carbon copy of Sentry Action, which is the operation taken
+// on an authorizable, and authorizable is linear hierarchical
+// structured resource. In this case, HiveSQL privilege model is
+// chosen to define authorizables (server → database → table → column)
+// and actions (create, drop, etc.) to authorize users' operations
+// (e.g. create a table, drop a database).
+// See org.apache.sentry.core.model.db.HivePrivilegeModel.
//
-// This class is not thread-safe.
+// One action can imply another following rules defined in Imply().
class SentryAction {
public:
static const char* const kWildCard;
@@ -37,7 +41,7 @@ class SentryAction {
// Actions that are supported. All actions are independent,
// except that ALL subsumes every other action, and every
// action subsumes METADATA. OWNER is a special action that
- // behaves like the ALL.
+ // behaves like ALL.
// Note that 'UNINITIALIZED' is not an actual operation but
// only to represent an action in uninitialized state.
//
@@ -65,9 +69,9 @@ class SentryAction {
}
// Create an Action from string.
- Status FromString(const std::string& action);
+ static Status FromString(const std::string& str, SentryAction* action);
- // Check if an action implies the other. In general,
+ // Check if this action implies 'other'. In general,
// 1. an action only implies itself.
// 2. with the exceptions that ALL, OWNER imply all other actions,
// and any action implies METADATA.
@@ -79,5 +83,9 @@ class SentryAction {
Action action_;
};
+const char* ActionToString(SentryAction::Action action);
+
+std::ostream& operator<<(std::ostream& o, SentryAction::Action action);
+
} // namespace sentry
} // namespace kudu
[2/2] kudu git commit: De-flake ASAN
TestSimultaneousLeaderTransferAndAbruptStepdown
Posted by ad...@apache.org.
De-flake ASAN TestSimultaneousLeaderTransferAndAbruptStepdown
This test is flaky in ASAN when run with the entire kudu-admin-test,
though not when run by itself. I'm not sure why that is, but to try to
improve things, I'm reducing the frequency of the simultaneous abrupt
stepdown and graceful transfer in ASAN mode so there's more time for
the tablet to make progress.
I ran the new test as part of the entire kudu-admin-test 1000 times in
ASAN and saw no failures. I ran kudu-admin-test 1000 times in ASAN with
8 stress threads and saw 4 failures, none of which were
TestSimultaneousLeaderTransferAndAbruptStepdown.
Change-Id: Ic3237f3fde48daeb2f307dfb78d457217df2beed
Reviewed-on: http://gerrit.cloudera.org:8080/11737
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Will Berkeley <wd...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/7d65f495
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/7d65f495
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/7d65f495
Branch: refs/heads/master
Commit: 7d65f495db9f663f9626caba9b6880288d6cd83b
Parents: 4abc211
Author: Will Berkeley <wd...@gmail.org>
Authored: Fri Oct 19 11:06:15 2018 -0700
Committer: Will Berkeley <wd...@gmail.com>
Committed: Mon Oct 22 21:58:51 2018 +0000
----------------------------------------------------------------------
src/kudu/tools/kudu-admin-test.cc | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/7d65f495/src/kudu/tools/kudu-admin-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-admin-test.cc b/src/kudu/tools/kudu-admin-test.cc
index 78adeeb..3293fca 100644
--- a/src/kudu/tools/kudu-admin-test.cc
+++ b/src/kudu/tools/kudu-admin-test.cc
@@ -1481,11 +1481,20 @@ TEST_F(AdminCliTest, TestSimultaneousLeaderTransferAndAbruptStepdown) {
workload.Setup();
workload.Start();
+ // Sometimes this test is flaky under ASAN and the writes are never able to
+ // complete, so we'll back off on how frequently we disrupt leadership to give
+ // time for progress to be made.
+ #if defined(ADDRESS_SANITIZER)
+ const auto leader_change_period_sec = MonoDelta::FromMilliseconds(5000);
+ #else
+ const auto leader_change_period_sec = MonoDelta::FromMilliseconds(1000);
+ #endif
+
const string& master_addr = cluster_->master()->bound_rpc_addr().ToString();
while (workload.rows_inserted() < 1000) {
- // Issue a graceful stepdown and then an abrupt stepdown, every second.
- // The results are ignored because the tools might fail due to the
- // constant leadership changes.
+ // Issue a graceful stepdown and then an abrupt stepdown, every
+ // 'leader_change_period_sec' seconds. The results are ignored because the
+ // tools might fail due to the constant leadership changes.
ignore_result(RunKuduTool({
"tablet",
"leader_step_down",
@@ -1499,7 +1508,7 @@ TEST_F(AdminCliTest, TestSimultaneousLeaderTransferAndAbruptStepdown) {
master_addr,
tablet_id_
}));
- SleepFor(MonoDelta::FromMilliseconds(1000));
+ SleepFor(leader_change_period_sec);
}
}