You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ar...@apache.org on 2019/01/31 19:07:27 UTC
[impala] 01/07: IMPALA-7540. Intern most repetitive strings and
network addresses in catalog
This is an automated email from the ASF dual-hosted git repository.
arodoni pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit f20a03a7b1bc2a9bb6cd8b54b8afb9ce384538f1
Author: Todd Lipcon <to...@apache.org>
AuthorDate: Tue Aug 7 21:48:20 2018 -0700
IMPALA-7540. Intern most repetitive strings and network addresses in catalog
This adds interning to a bunch of repeated strings in catalog objects,
including:
- table name
- DB name
- owner
- column names
- input/output formats
- parameter keys
- common parameter values ("true", "false", etc)
- HBase column family names
Additionally, it interns TNetworkAddresses, so that each datanode host
is only stored once rather than having its own copy in each table.
I verified this patch using jxray on the development catalogd and
impalad. The following lines are removed entirely from the "duplicate
strings" report:
Overhead # char[]s # objects Value
164K (0.3%) 2,635 2,635 "127.0.0.1"
97K (0.2%) 1,038 1,038 "__HIVE_DEFAULT_PARTITION__"
95K (0.2%) 1,111 1,111 "transient_lastDdlTime"
92K (0.1%) 1,975 1,975 "d"
70K (0.1%) 997 997 "EXTERNAL_TABLE"
56K (< 0.1%) 1,201 1,201 "todd"
54K (< 0.1%) 998 998 "EXTERNAL"
46K (< 0.1%) 998 998 "TRUE"
44K (< 0.1%) 567 567 "numFilesErasureCoded"
38K (< 0.1%) 612 612 "totalSize"
30K (< 0.1%) 567 567 "numFiles"
The following are reduced substantially:
Before: 72K (0.1%) 1,543 1,543 "1"
After: 47K (< 0.1%) 1,009 1,009 "1"
A few large strings remain in the report that may be worth addressing, depending
on whether we think production catalogs exhibit the same repetitions:
1) Avro schemas, eg:
204K (0.3%) 3 3 "{"fields": [{"type": ["boolean", "null"], "name": "bool_col1"}, {"type": ["int", "null"], "name": "tinyint_col1"}, {"type": ...[length 52429]"
(in the development catalog there are multiple tables with the same Avro
schema)
2) Partition location suffixes, eg:
144K (0.2%) 1,234 1,234 "many_blocks_num_blocks_per_partition_1"
17K (< 0.1%) 230 230 "year=2009/month=2"
17K (< 0.1%) 230 230 "year=2009/month=3"
17K (< 0.1%) 230 230 "year=2009/month=1"
(in the development catalog lots of tables have the same partitioning
layout)
3) Unsure (jxray isn't reporting the reference chain, but seems likely
to be partition values):
49K (< 0.1%) 1,058 1,058 "2010"
28K (< 0.1%) 612 612 "2009"
27K (< 0.1%) 585 585 "0"
22K (< 0.1%) 71 899 ""
Change-Id: Ib3121aefa4391bcb1477d9dba0a49440d7000d26
Reviewed-on: http://gerrit.cloudera.org:8080/11158
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../apache/impala/catalog/CatalogInterners.java | 234 +++++++++++++++++++++
.../org/apache/impala/catalog/HBaseColumn.java | 2 +-
.../org/apache/impala/catalog/HdfsPartition.java | 12 +-
.../java/org/apache/impala/catalog/HdfsTable.java | 3 +-
.../main/java/org/apache/impala/catalog/Table.java | 5 +-
5 files changed, 250 insertions(+), 6 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/catalog/CatalogInterners.java b/fe/src/main/java/org/apache/impala/catalog/CatalogInterners.java
new file mode 100644
index 0000000..e34f8fa
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/CatalogInterners.java
@@ -0,0 +1,234 @@
+// 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
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// 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.impala.catalog;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.hadoop.hive.metastore.api.StorageDescriptor;
+import org.apache.impala.thrift.TColumn;
+import org.apache.impala.thrift.TNetworkAddress;
+import org.apache.impala.thrift.TTable;
+import org.apache.thrift.TException;
+import org.apache.thrift.protocol.TProtocol;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Interner;
+import com.google.common.collect.Interners;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
+/**
+ * Static utility methods for interning various objects used in the catalog. In many
+ * cases, there are common strings that show up in lots of objects (eg class names,
+ * database names, user names, property names), and interning these strings can result
+ * in significant memory savings.
+ */
+public abstract class CatalogInterners {
+ private static Interner<TNetworkAddress> NETWORK_ADDRESS_INTERNER =
+ Interners.newWeakInterner();
+
+ /**
+ * Interner used for relatively-low-cardinality strings that aren't quite
+ * low enough cardinality to be safe to use the JVM interner. The JVM's interner
+ * uses a hashtable with a fixed number of buckets, so if we put anything
+ * with unbounded cardinality in there, we risk a high number of collisions.
+ *
+ * See https://shipilev.net/jvm-anatomy-park/10-string-intern/ for more details
+ * on those risks.
+ */
+ private static Interner<String> STRING_INTERNER = Interners.newWeakInterner();
+
+ // Do not instantiate
+ private CatalogInterners() {}
+
+ /**
+ * Intern low-cardinality fields of a metastore table object in-place.
+ */
+ public static void internFieldsInPlace(
+ org.apache.hadoop.hive.metastore.api.Table msTable) {
+ if (msTable == null) return;
+ // Database name is typically low cardinality, but still potentially risky
+ // to put in the JVM string table, so we'll use our own interner instead.
+ if (msTable.isSetDbName()) {
+ msTable.setDbName(STRING_INTERNER.intern(msTable.getDbName()));
+ }
+ if (msTable.isSetOwner()) {
+ msTable.setOwner(STRING_INTERNER.intern(msTable.getOwner()));
+ }
+ if (msTable.isSetParameters()) {
+ msTable.setParameters(internParameters(msTable.getParameters()));
+ }
+ if (msTable.isSetTableType()) {
+ msTable.setTableType(STRING_INTERNER.intern(msTable.getTableType()));
+ }
+ if (msTable.isSetSd()) {
+ internFieldsInPlace(msTable.getSd());
+ }
+ if (msTable.isSetPartitionKeys()) {
+ for (FieldSchema fs : msTable.getPartitionKeys()) internFieldsInPlace(fs);
+ }
+ }
+
+ public static void internFieldsInPlace(TTable table) {
+ if (table == null) return;
+ internFieldsInPlace(table.getClustering_columns());
+ internFieldsInPlace(table.getColumns());
+ if (table.isSetHdfs_table()) {
+ table.hdfs_table.setNullPartitionKeyValue(
+ table.hdfs_table.getNullPartitionKeyValue().intern());
+ table.hdfs_table.setNullColumnValue(
+ table.hdfs_table.getNullColumnValue().intern());
+ table.hdfs_table.setNetwork_addresses(internAddresses(
+ table.hdfs_table.getNetwork_addresses()));
+ }
+ }
+
+ private static void internFieldsInPlace(List<TColumn> cols) {
+ if (cols == null) return;
+ for (TColumn col : cols) {
+ if (col.isSetColumnName()) {
+ col.setColumnName(STRING_INTERNER.intern(col.getColumnName()));
+ }
+ }
+ }
+
+ /**
+ * Intern low-cardinality fields in the given storage descriptor.
+ */
+ public static void internFieldsInPlace(StorageDescriptor sd) {
+ if (sd == null) return;
+ if (sd.isSetCols()) {
+ for (FieldSchema fs : sd.getCols()) internFieldsInPlace(fs);
+ }
+ if (sd.isSetInputFormat()) {
+ sd.setInputFormat(STRING_INTERNER.intern(sd.getInputFormat()));
+ }
+ if (sd.isSetOutputFormat()) {
+ sd.setOutputFormat(STRING_INTERNER.intern(sd.getOutputFormat()));
+ }
+ if (sd.isSetParameters()) {
+ sd.setParameters(internParameters(sd.getParameters()));
+ }
+ }
+
+ /**
+ * Intern low-cardinality fields in the given FieldSchema.
+ */
+ private static void internFieldsInPlace(FieldSchema fs) {
+ if (fs == null) return;
+ if (fs.isSetName()) {
+ fs.setName(STRING_INTERNER.intern(fs.getName()));
+ }
+ if (fs.isSetType()) {
+ fs.setType(STRING_INTERNER.intern(fs.getType()));
+ }
+ }
+
+ /**
+ * Transform the given parameters map (for table or partition parameters) to intern
+ * its keys and commonly-used values.
+ */
+ public static Map<String, String> internParameters(Map<String, String> parameters) {
+ if (parameters == null) return null;
+ Map<String, String> ret = Maps.newHashMapWithExpectedSize(parameters.size());
+ for (Map.Entry<String, String> e : parameters.entrySet()) {
+ // Intern values which we know will show up quite often. This is based on
+ // the results of the following SQL query against an HMS database:
+ //
+ // select PARAM_VALUE, count(*) from PARTITION_PARAMS where PARAM_KEY
+ // not like 'impala_%' group by PARAM_VALUE order by count(*) desc limit 100;
+ //
+ // In a large catalog from a production install, these represented about 68% of the
+ // entries.
+ String val = e.getValue();
+ if (val.isEmpty() ||
+ "-1".equals(val) ||
+ "0".equals(val) ||
+ "true".equalsIgnoreCase(val) ||
+ "false".equalsIgnoreCase(val) ||
+ "TASK".equals(val) ||
+ val.startsWith("impala_")) {
+ val = val.intern();
+ } else if (val.length() <= 2) {
+ // Very short values tend to be quite common -- for example most partitions
+ // have less than 99 files. But, potential cardinality is high enough that
+ // we avoid the JVM interner.
+ val = STRING_INTERNER.intern(val);
+ }
+
+ // Assume that the keys used in the HMS have a low cardinality, even if technically
+ // custom properties are allowed.
+ ret.put(STRING_INTERNER.intern(e.getKey()), val);
+ }
+ Preconditions.checkState(ret.size() == parameters.size());
+ return ret;
+ }
+
+ private static List<TNetworkAddress> internAddresses(List<TNetworkAddress> addrs) {
+ if (addrs == null) return null;
+ List<TNetworkAddress> ret = Lists.newArrayListWithCapacity(addrs.size());
+ for (TNetworkAddress addr : addrs) {
+ ret.add(CatalogInterners.internNetworkAddress(addr));
+ }
+ return ret;
+ }
+
+ /**
+ * Intern a shared string. Use this only for strings reasonably expected to be
+ * reused, such as an HBase column family, which are usually shared across many
+ * columns.
+ */
+ public static String internString(String value) {
+ if (value == null) return null;
+ return STRING_INTERNER.intern(value);
+ }
+
+ /**
+ * Intern the given network address object, and return an immutable version.
+ */
+ public static TNetworkAddress internNetworkAddress(TNetworkAddress addr) {
+ if (addr == null) return null;
+ // Intern an immutable subclass of the network address so that we don't
+ // accidentally modify them. This doesn't override every mutating method
+ // but it's likely someone would trip over one of these.
+ return NETWORK_ADDRESS_INTERNER.intern(new TNetworkAddress(addr) {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public void clear() {
+ throw new UnsupportedOperationException("immutable");
+ }
+
+ @Override
+ public TNetworkAddress setHostname(String hostname) {
+ throw new UnsupportedOperationException("immutable");
+ }
+
+ @Override
+ public TNetworkAddress setPort(int port) {
+ throw new UnsupportedOperationException("immutable");
+ }
+
+ @Override
+ public void read(TProtocol iprot) throws TException {
+ throw new UnsupportedOperationException("immutable");
+ }
+ });
+ }
+}
diff --git a/fe/src/main/java/org/apache/impala/catalog/HBaseColumn.java b/fe/src/main/java/org/apache/impala/catalog/HBaseColumn.java
index dae03fb..eef30dd 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HBaseColumn.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HBaseColumn.java
@@ -31,7 +31,7 @@ public class HBaseColumn extends Column implements Comparable<HBaseColumn> {
public HBaseColumn(String name, String columnFamily, String columnQualifier,
boolean binaryEncoded, Type type, String comment, int position) {
super(name, type, comment, position);
- columnFamily_ = columnFamily;
+ columnFamily_ = CatalogInterners.internString(columnFamily);
columnQualifier_ = columnQualifier;
binaryEncoded_ = binaryEncoded;
}
diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java b/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
index e87608f..8c5db82 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
@@ -305,7 +305,8 @@ public class HdfsPartition implements FeFsPartition, PrunablePartition {
String[] ip_port = location.split(":");
if (ip_port.length != 2) return null;
try {
- return new TNetworkAddress(ip_port[0], Integer.parseInt(ip_port[1]));
+ return CatalogInterners.internNetworkAddress(
+ new TNetworkAddress(ip_port[0], Integer.parseInt(ip_port[1])));
} catch (NumberFormatException e) {
return null;
}
@@ -765,6 +766,7 @@ public class HdfsPartition implements FeFsPartition, PrunablePartition {
org.apache.hadoop.hive.metastore.api.StorageDescriptor sd = null;
if (msPartition != null) {
sd = msPartition.getSd();
+ CatalogInterners.internFieldsInPlace(sd);
msCreateTime = msPartition.getCreateTime();
msLastAccessTime = msPartition.getLastAccessTime();
} else {
@@ -778,7 +780,8 @@ public class HdfsPartition implements FeFsPartition, PrunablePartition {
sdSerdeInfo = sd.getSerdeInfo();
sdBucketCols = ImmutableList.copyOf(sd.getBucketCols());
sdSortCols = ImmutableList.copyOf(sd.getSortCols());
- sdParameters = ImmutableMap.copyOf(sd.getParameters());
+ sdParameters = ImmutableMap.copyOf(CatalogInterners.internParameters(
+ sd.getParameters()));
} else {
sdInputFormat = "";
sdOutputFormat = "";
@@ -857,6 +860,8 @@ public class HdfsPartition implements FeFsPartition, PrunablePartition {
hmsParameters_ = new HashMap<>();
}
extractAndCompressPartStats();
+ // Intern parameters after removing the incremental stats
+ hmsParameters_ = CatalogInterners.internParameters(hmsParameters_);
}
public HdfsPartition(HdfsTable table,
@@ -951,7 +956,8 @@ public class HdfsPartition implements FeFsPartition, PrunablePartition {
}
if (thriftPartition.isSetHms_parameters()) {
- partition.hmsParameters_ = thriftPartition.getHms_parameters();
+ partition.hmsParameters_ = CatalogInterners.internParameters(
+ thriftPartition.getHms_parameters());
} else {
partition.hmsParameters_ = new HashMap<>();
}
diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
index 82b8ef3..7a16b3d 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
@@ -1211,7 +1211,8 @@ public class HdfsTable extends Table implements FeFsTable {
try {
if (loadTableSchema) {
// set nullPartitionKeyValue from the hive conf.
- nullPartitionKeyValue_ = MetaStoreUtil.getNullPartitionKeyValue(client);
+ nullPartitionKeyValue_ =
+ MetaStoreUtil.getNullPartitionKeyValue(client).intern();
loadSchema(msTbl);
loadAllColumnStats(client);
}
diff --git a/fe/src/main/java/org/apache/impala/catalog/Table.java b/fe/src/main/java/org/apache/impala/catalog/Table.java
index f506078..e6b4dbe 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Table.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Table.java
@@ -251,8 +251,9 @@ public abstract class Table extends CatalogObjectImpl implements FeTable {
*/
public static Table fromMetastoreTable(Db db,
org.apache.hadoop.hive.metastore.api.Table msTbl) {
- // Create a table of appropriate type
+ CatalogInterners.internFieldsInPlace(msTbl);
Table table = null;
+ // Create a table of appropriate type
if (TableType.valueOf(msTbl.getTableType()) == TableType.VIRTUAL_VIEW) {
table = new View(msTbl, db, msTbl.getTableName(), msTbl.getOwner());
} else if (HBaseTable.isHBaseTable(msTbl)) {
@@ -277,6 +278,7 @@ public abstract class Table extends CatalogObjectImpl implements FeTable {
*/
public static Table fromThrift(Db parentDb, TTable thriftTable)
throws TableLoadingException {
+ CatalogInterners.internFieldsInPlace(thriftTable);
Table newTable;
if (!thriftTable.isSetLoad_status() && thriftTable.isSetMetastore_table()) {
newTable = Table.fromMetastoreTable(parentDb, thriftTable.getMetastore_table());
@@ -501,6 +503,7 @@ public abstract class Table extends CatalogObjectImpl implements FeTable {
public void setMetaStoreTable(org.apache.hadoop.hive.metastore.api.Table msTbl) {
msTable_ = msTbl;
+ CatalogInterners.internFieldsInPlace(msTable_);
}
@Override // FeTable