sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ak...@apache.org
Subject sentry git commit: SENTRY-1758: Improve Sentry memory usage by interning object names (Alex Kolbasov, reviewed by: Vamsee Yarlagadda and Misha Dmitriev)
Date Tue, 16 May 2017 01:37:48 GMT
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 <akolb@cloudera.com>
Authored: Mon May 15 18:37:30 2017 -0700
Committer: Alexander Kolbasov <akolb@cloudera.com>
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();
   }
 
   /**


Mime
View raw message