You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by va...@apache.org on 2017/06/14 00:57:00 UTC
[22/52] [abbrv] sentry git commit: SENTRY-1758: Improve Sentry memory
usage by interning object names (Alex Kolbasov,
reviewed by: Vamsee Yarlagadda and Misha Dmitriev)
SENTRY-1758: Improve Sentry memory usage by interning object names (Alex Kolbasov, reviewed by: Vamsee Yarlagadda and Misha Dmitriev)
Change-Id: I53f91ebb64f8e55461b7510b8e65aef6a7f2fd50
Reviewed-on: http://gerrit.sjc.cloudera.com:8080/22686
Tested-by: Jenkins User
Reviewed-by: Alexander Kolbasov <ak...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/a0efa352
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/a0efa352
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/a0efa352
Branch: refs/for/cdh5-1.5.1_ha
Commit: a0efa3526348e64d90f2d034cf2a519f259ba012
Parents: e1e1cd2
Author: Alexander Kolbasov <ak...@cloudera.com>
Authored: Mon May 15 18:44:09 2017 -0700
Committer: Alexander Kolbasov <ak...@cloudera.com>
Committed: Mon May 15 20:30:01 2017 -0700
----------------------------------------------------------------------
.../sentry/hdfs/FullUpdateInitializer.java | 31 +++++++++++-------
.../db/service/model/MAuthzPathsMapping.java | 2 +-
.../sentry/provider/db/service/model/MPath.java | 2 +-
.../db/service/model/MSentryGMPrivilege.java | 14 ++++----
.../provider/db/service/model/MSentryGroup.java | 6 +---
.../db/service/model/MSentryPrivilege.java | 34 +++++++++-----------
.../provider/db/service/model/MSentryRole.java | 2 +-
.../provider/db/service/model/MSentryUtil.java | 33 +++++++++++++++++++
.../db/service/model/MSentryVersion.java | 4 +--
9 files changed, 81 insertions(+), 47 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/sentry/blob/a0efa352/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java
index 2fe2bb5..efd3fa3 100644
--- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java
+++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java
@@ -132,7 +132,7 @@ public final class FullUpdateInitializer implements AutoCloseable {
}
ObjectMapping(String authObject, String path) {
- Set<String> values = Collections.singleton(path);
+ Set<String> values = Collections.singleton(safeIntern(path));
objects = ImmutableMap.of(authObject, values);
}
@@ -257,9 +257,9 @@ public final class FullUpdateInitializer implements AutoCloseable {
PartitionTask(String dbName, String tblName, String authName,
List<String> partNames) {
- this.dbName = dbName;
- this.tblName = tblName;
- this.authName = authName;
+ this.dbName = safeIntern(dbName);
+ this.tblName = safeIntern(tblName);
+ this.authName = safeIntern(authName);
this.partNames = partNames;
}
@@ -274,7 +274,7 @@ public final class FullUpdateInitializer implements AutoCloseable {
for (Partition part : tblParts) {
String partPath = pathFromURI(part.getSd().getLocation());
if (partPath != null) {
- partitionNames.add(partPath);
+ partitionNames.add(partPath.intern());
}
}
return new ObjectMapping(authName, partitionNames);
@@ -286,7 +286,7 @@ public final class FullUpdateInitializer implements AutoCloseable {
private final List<String> tableNames;
TableTask(Database db, List<String> tableNames) {
- dbName = db.getName();
+ dbName = safeIntern(db.getName());
this.tableNames = tableNames;
}
@@ -307,8 +307,8 @@ public final class FullUpdateInitializer implements AutoCloseable {
continue;
}
- String tableName = tbl.getTableName().toLowerCase();
- String authzObject = dbName + "." + tableName;
+ String tableName = safeIntern(tbl.getTableName().toLowerCase());
+ String authzObject = (dbName + "." + tableName).intern();
List<String> tblPartNames = client.listPartitionNames(dbName, tableName, (short) -1);
for (int i = 0; i < tblPartNames.size(); i += maxPartitionsPerCall) {
List<String> partsToFetch = tblPartNames.subList(i,
@@ -317,7 +317,7 @@ public final class FullUpdateInitializer implements AutoCloseable {
tableName, authzObject, partsToFetch);
results.add(threadPool.submit(partTask));
}
- String tblPath = pathFromURI(tbl.getSd().getLocation());
+ String tblPath = safeIntern(pathFromURI(tbl.getSd().getLocation()));
if (tblPath == null) {
continue;
}
@@ -338,7 +338,7 @@ public final class FullUpdateInitializer implements AutoCloseable {
DbTask(String dbName) {
//Database names are case insensitive
- this.dbName = dbName.toLowerCase();
+ this.dbName = safeIntern(dbName.toLowerCase());
}
@Override
@@ -356,7 +356,7 @@ public final class FullUpdateInitializer implements AutoCloseable {
Callable<CallResult> tableTask = new TableTask(db, tablesToFetch);
results.add(threadPool.submit(tableTask));
}
- String dbPath = pathFromURI(db.getLocationUri());
+ String dbPath = safeIntern(pathFromURI(db.getLocationUri()));
return (dbPath != null) ? new ObjectMapping(dbName, dbPath) :
emptyObjectMapping;
}
@@ -440,4 +440,13 @@ public final class FullUpdateInitializer implements AutoCloseable {
LOGGER.warn("Interrupted shutdown");
}
}
+
+ /**
+ * Intern a string but only if it is not null
+ * @param arg String to be interned, may be null
+ * @return interned string or null
+ */
+ static String safeIntern(String arg) {
+ return (arg != null) ? arg.intern() : null;
+ }
}
http://git-wip-us.apache.org/repos/asf/sentry/blob/a0efa352/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
index c710701..f51894b 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
@@ -34,7 +34,7 @@ public class MAuthzPathsMapping {
private long createTimeMs;
public MAuthzPathsMapping(String authzObjName, Iterable<String> paths) {
- this.authzObjName = authzObjName;
+ this.authzObjName = MSentryUtil.safeIntern(authzObjName);
this.paths = new HashSet<>();
for (String path : paths) {
this.paths.add(new MPath(path));
http://git-wip-us.apache.org/repos/asf/sentry/blob/a0efa352/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java
index c743846..b0eaff2 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java
@@ -27,7 +27,7 @@ public class MPath {
private String path;
public MPath(String path) {
- this.path = path;
+ this.path = MSentryUtil.safeIntern(path);
}
public String getPath() {
http://git-wip-us.apache.org/repos/asf/sentry/blob/a0efa352/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java
index c9b312a..762d13d 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java
@@ -79,11 +79,11 @@ public class MSentryGMPrivilege {
public MSentryGMPrivilege(String componentName, String serviceName,
List<? extends Authorizable> authorizables,
String action, Boolean grantOption) {
- this.componentName = componentName;
- this.serviceName = serviceName;
- this.action = action;
+ this.componentName = MSentryUtil.safeIntern(componentName);
+ this.serviceName = MSentryUtil.safeIntern(serviceName);
+ this.action = MSentryUtil.safeIntern(action);
this.grantOption = grantOption;
- this.roles = new HashSet<MSentryRole>();
+ this.roles = new HashSet<>();
this.createTime = System.currentTimeMillis();
setAuthorizables(authorizables);
}
@@ -97,9 +97,7 @@ public class MSentryGMPrivilege {
this.createTime = copy.createTime;
setAuthorizables(copy.getAuthorizables());
this.roles = new HashSet<MSentryRole>();
- for (MSentryRole role : copy.roles) {
- roles.add(role);
- }
+ roles.addAll(copy.roles);
}
public String getServiceName() {
@@ -309,7 +307,7 @@ public class MSentryGMPrivilege {
/**
* Return true if this privilege implies request privilege
* Otherwise, return false
- * @param other, other privilege
+ * @param request, other privilege
*/
public boolean implies(MSentryGMPrivilege request) {
//component check
http://git-wip-us.apache.org/repos/asf/sentry/blob/a0efa352/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGroup.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGroup.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGroup.java
index 32dbafc..04d51d9 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGroup.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGroup.java
@@ -38,7 +38,7 @@ public class MSentryGroup {
private long createTime;
public MSentryGroup(String groupName, long createTime, Set<MSentryRole> roles) {
- this.setGroupName(groupName);
+ this.groupName = MSentryUtil.safeIntern(groupName);
this.createTime = createTime;
this.roles = roles;
}
@@ -59,10 +59,6 @@ public class MSentryGroup {
return groupName;
}
- public void setGroupName(String groupName) {
- this.groupName = groupName;
- }
-
public void appendRole(MSentryRole role) {
if (roles.add(role)) {
role.appendGroup(this);
http://git-wip-us.apache.org/repos/asf/sentry/blob/a0efa352/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
index 1c68a0f..8a2584e 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
@@ -50,21 +50,21 @@ public class MSentryPrivilege {
private long createTime;
public MSentryPrivilege() {
- this.roles = new HashSet<MSentryRole>();
+ this.roles = new HashSet<>();
}
public MSentryPrivilege(String privilegeName, String privilegeScope,
String serverName, String dbName, String tableName, String columnName,
String URI, String action, Boolean grantOption) {
- this.privilegeScope = privilegeScope;
- this.serverName = serverName;
- this.dbName = SentryStore.toNULLCol(dbName);
- this.tableName = SentryStore.toNULLCol(tableName);
- this.columnName = SentryStore.toNULLCol(columnName);
- this.URI = SentryStore.toNULLCol(URI);
- this.action = SentryStore.toNULLCol(action);
+ this.privilegeScope = MSentryUtil.safeIntern(privilegeScope);
+ this.serverName = MSentryUtil.safeIntern(serverName);
+ this.dbName = SentryStore.toNULLCol(dbName).intern();
+ this.tableName = SentryStore.toNULLCol(tableName).intern();
+ this.columnName = SentryStore.toNULLCol(columnName).intern();
+ this.URI = SentryStore.toNULLCol(URI).intern();
+ this.action = SentryStore.toNULLCol(action).intern();
this.grantOption = grantOption;
- this.roles = new HashSet<MSentryRole>();
+ this.roles = new HashSet<>();
}
public MSentryPrivilege(String privilegeName, String privilegeScope,
@@ -77,16 +77,14 @@ public class MSentryPrivilege {
public MSentryPrivilege(MSentryPrivilege other) {
this.privilegeScope = other.privilegeScope;
this.serverName = other.serverName;
- this.dbName = SentryStore.toNULLCol(other.dbName);
- this.tableName = SentryStore.toNULLCol(other.tableName);
- this.columnName = SentryStore.toNULLCol(other.columnName);
- this.URI = SentryStore.toNULLCol(other.URI);
- this.action = SentryStore.toNULLCol(other.action);
+ this.dbName = SentryStore.toNULLCol(other.dbName).intern();
+ this.tableName = SentryStore.toNULLCol(other.tableName).intern();
+ this.columnName = SentryStore.toNULLCol(other.columnName).intern();
+ this.URI = SentryStore.toNULLCol(other.URI).intern();
+ this.action = SentryStore.toNULLCol(other.action).intern();
this.grantOption = other.grantOption;
- this.roles = new HashSet<MSentryRole>();
- for (MSentryRole role : other.roles) {
- roles.add(role);
- }
+ this.roles = new HashSet<>();
+ roles.addAll(other.roles);
}
public String getServerName() {
http://git-wip-us.apache.org/repos/asf/sentry/blob/a0efa352/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
index c4f9fb2..ae69c9d 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
@@ -44,7 +44,7 @@ public class MSentryRole {
private long createTime;
public MSentryRole(String roleName, long createTime) {
- this.roleName = roleName;
+ this.roleName = MSentryUtil.safeIntern(roleName);
this.createTime = createTime;
privileges = new HashSet<MSentryPrivilege>();
gmPrivileges = new HashSet<MSentryGMPrivilege>();
http://git-wip-us.apache.org/repos/asf/sentry/blob/a0efa352/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUtil.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUtil.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUtil.java
new file mode 100644
index 0000000..7558267
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUtil.java
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sentry.provider.db.service.model;
+
+/**
+ * Common utilities for model objects
+ */
+final class MSentryUtil {
+ /**
+ * Intern a string but only if it is not null
+ * @param arg String to be interned, may be null
+ * @return interned string or null
+ */
+ static String safeIntern(String arg) {
+ return (arg != null) ? arg.intern() : null;
+ }
+}
http://git-wip-us.apache.org/repos/asf/sentry/blob/a0efa352/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryVersion.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryVersion.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryVersion.java
index ff8830f..b0dbaf0 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryVersion.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryVersion.java
@@ -29,8 +29,8 @@ public class MSentryVersion {
}
public MSentryVersion(String schemaVersion, String versionComment) {
- this.schemaVersion = schemaVersion;
- this.versionComment = versionComment;
+ this.schemaVersion = schemaVersion.intern();
+ this.versionComment = versionComment.intern();
}
/**