Repository: sentry Updated Branches: refs/heads/sentry-ha-redesign 60e6244f2 -> ed47850a7
SENTRY-1758: Improve Sentry memory usage by interning object names (Alex Kolbasov, reviewed by: Vamsee Yarlagadda and Misha Dmitriev) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/ed47850a Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/ed47850a Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/ed47850a Branch: refs/heads/sentry-ha-redesign Commit: ed47850a797f14c20c47dccf99cb02ef5aaad635 Parents: 60e6244 Author: Alexander Kolbasov <[email protected]> Authored: Mon May 15 18:37:30 2017 -0700 Committer: Alexander Kolbasov <[email protected]> Committed: Mon May 15 18:37:30 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 | 10 +++--- .../provider/db/service/model/MSentryUser.java | 6 +--- .../provider/db/service/model/MSentryUtil.java | 33 +++++++++++++++++++ .../db/service/model/MSentryVersion.java | 4 +-- 10 files changed, 86 insertions(+), 56 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/ed47850a/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/ed47850a/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/ed47850a/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/ed47850a/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 749af2a..0e8fb06 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() { @@ -322,7 +320,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/ed47850a/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 7e41c93..e0db741 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/ed47850a/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 4c3af79..73fa4ff 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 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 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/ed47850a/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 f9da589..fb8f5d2 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 @@ -46,12 +46,12 @@ 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>(); - groups = new HashSet<MSentryGroup>(); - users = new HashSet<MSentryUser>(); + privileges = new HashSet<>(); + gmPrivileges = new HashSet<>(); + groups = new HashSet<>(); + users = new HashSet<>(); } public MSentryRole(String roleName) { http://git-wip-us.apache.org/repos/asf/sentry/blob/ed47850a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java index ff57249..f468a46 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java @@ -38,7 +38,7 @@ public class MSentryUser { private long createTime; public MSentryUser(String userName, long createTime, Set<MSentryRole> roles) { - this.setUserName(userName); + this.userName = MSentryUtil.safeIntern(userName); this.createTime = createTime; this.roles = roles; } @@ -59,10 +59,6 @@ public class MSentryUser { return userName; } - public void setUserName(String userName) { - this.userName = userName; - } - public void appendRole(MSentryRole role) { if (roles.add(role)) { role.appendUser(this); http://git-wip-us.apache.org/repos/asf/sentry/blob/ed47850a/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/ed47850a/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(); } /**
