You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2018/10/11 19:54:14 UTC
[09/10] impala git commit: IMPALA-7688: Fix spurious error messages
when updating owner privileges
IMPALA-7688: Fix spurious error messages when updating owner privileges
Failure in updating owner privileges is not an issue since this could
mean a Sentry refresh occurred while updating owner privileges and
Sentry refresh itself will update all privileges including owner
privileges. This patch changes the code to log the failure in
updating owner privileges as WARN instead of ERROR.
Testing:
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: I4b46df01c5675ffed6528b7a73e68518664d8be0
Reviewed-on: http://gerrit.cloudera.org:8080/11649
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/0cbe37af
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/0cbe37af
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/0cbe37af
Branch: refs/heads/master
Commit: 0cbe37afc8f77fd8e0aef7e710e2d1b214c1c5b1
Parents: 6099255
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Mon Oct 8 22:34:43 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Thu Oct 11 08:44:02 2018 +0000
----------------------------------------------------------------------
.../impala/service/CatalogOpExecutor.java | 41 +++++++++++++++-----
1 file changed, 31 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/0cbe37af/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 36725df..aeb23df 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -1028,8 +1028,8 @@ public class CatalogOpExecutor {
* operations.
*/
private void updateOwnerPrivileges(String databaseName, String tableName,
- String serverName, String oldOwner, PrincipalType oldOwnerType, String newOwner,
- PrincipalType newOwnerType, TDdlExecResponse resp) {
+ String serverName, String oldOwner, PrincipalType oldOwnerType, String newOwner,
+ PrincipalType newOwnerType, TDdlExecResponse resp) {
if (catalog_.getSentryProxy() == null || !catalog_.getSentryProxy()
.isObjectOwnershipEnabled()) return;
Preconditions.checkNotNull(serverName);
@@ -1039,10 +1039,10 @@ public class CatalogOpExecutor {
} else {
filter = createTableOwnerPrivilegeFilter(databaseName, tableName, serverName);
}
- if(oldOwner != null && !oldOwner.isEmpty()) {
+ if (oldOwner != null && !oldOwner.isEmpty()) {
removePrivilegeFromCatalog(oldOwner, oldOwnerType, filter, resp);
}
- if(newOwner != null && !newOwner.isEmpty()) {
+ if (newOwner != null && !newOwner.isEmpty()) {
addPrivilegeToCatalog(newOwner, newOwnerType, filter, resp);
}
}
@@ -2865,8 +2865,11 @@ public class CatalogOpExecutor {
*/
private void removePrivilegeFromCatalog(String ownerString, PrincipalType ownerType,
TPrivilege filter, TDdlExecResponse response) {
+ Preconditions.checkNotNull(ownerString);
+ Preconditions.checkNotNull(ownerType);
+ Preconditions.checkNotNull(filter);
try {
- PrincipalPrivilege removedPrivilege;
+ PrincipalPrivilege removedPrivilege = null;
switch (ownerType) {
case ROLE:
removedPrivilege = catalog_.removeRolePrivilege(ownerString,
@@ -2877,14 +2880,21 @@ public class CatalogOpExecutor {
PrincipalPrivilege.buildPrivilegeName(filter));
break;
default:
- throw new CatalogException("Unexpected ownerType: " + ownerType.name());
+ Preconditions.checkArgument(false,
+ "Unexpected PrincipalType: " + ownerType.name());
}
if (removedPrivilege != null) {
response.result.addToRemoved_catalog_objects(removedPrivilege
.toTCatalogObject());
}
} catch (CatalogException e) {
- LOG.error("Error removing privilege: ", e);
+ // Failure removing an owner privilege is not an issue because it could be
+ // that Sentry refresh occurred while executing this method and this method
+ // is used as a a best-effort to do what Sentry refresh does to make the
+ // owner privilege available right away without having to wait for a Sentry
+ // refresh.
+ LOG.warn("Unable to remove owner privilege: " +
+ PrincipalPrivilege.buildPrivilegeName(filter), e);
}
}
@@ -2894,9 +2904,12 @@ public class CatalogOpExecutor {
*/
private void addPrivilegeToCatalog(String ownerString, PrincipalType ownerType,
TPrivilege filter, TDdlExecResponse response) {
+ Preconditions.checkNotNull(ownerString);
+ Preconditions.checkNotNull(ownerType);
+ Preconditions.checkNotNull(filter);
try {
Principal owner;
- PrincipalPrivilege cPrivilege;
+ PrincipalPrivilege cPrivilege = null;
if (ownerType == PrincipalType.USER) {
Reference<Boolean> existingUser = new Reference<>();
owner = catalog_.addUserIfNotExists(ownerString, existingUser);
@@ -2908,15 +2921,23 @@ public class CatalogOpExecutor {
}
} else if (ownerType == PrincipalType.ROLE) {
owner = catalog_.getAuthPolicy().getRole(ownerString);
+ Preconditions.checkNotNull(owner);
filter.setPrincipal_id(owner.getId());
filter.setPrincipal_type(TPrincipalType.ROLE);
cPrivilege = catalog_.addRolePrivilege(ownerString, filter);
} else {
- throw new CatalogException("Unexpected PrincipalType: " + ownerType.name());
+ Preconditions.checkArgument(false, "Unexpected PrincipalType: " +
+ ownerType.name());
}
response.result.addToUpdated_catalog_objects(cPrivilege.toTCatalogObject());
} catch (CatalogException e) {
- LOG.error("Error adding privilege: ", e);
+ // Failure adding an owner privilege is not an issue because it could be
+ // that Sentry refresh occurred while executing this method and this method
+ // is used as a a best-effort to do what Sentry refresh does to make the
+ // owner privilege available right away without having to wait for a Sentry
+ // refresh.
+ LOG.warn("Unable to add owner privilege: " +
+ PrincipalPrivilege.buildPrivilegeName(filter), e);
}
}