PHOENIX-3434 Avoid creating new Configuration in ClientAggregatePlan to improve performance
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/dcebfc2d Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/dcebfc2d Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/dcebfc2d Branch: refs/heads/encodecolumns2 Commit: dcebfc2dd60ab31ea4f4812fb62a1e9897f64883 Parents: ecb9360 Author: James Taylor <jamestay...@apache.org> Authored: Wed Nov 2 11:42:41 2016 -0700 Committer: James Taylor <jamestay...@apache.org> Committed: Wed Nov 2 13:24:49 2016 -0700 ---------------------------------------------------------------------- .../apache/phoenix/execute/ClientAggregatePlan.java | 14 +++++++++----- .../phoenix/expression/aggregator/Aggregators.java | 5 ++++- .../expression/aggregator/ServerAggregators.java | 2 -- .../expression/function/SingleAggregateFunction.java | 6 +++--- .../apache/phoenix/query/ConnectionQueryServices.java | 3 +++ .../phoenix/query/ConnectionQueryServicesImpl.java | 5 +++++ .../query/ConnectionlessQueryServicesImpl.java | 8 +++++++- .../query/DelegateConnectionQueryServices.java | 6 ++++++ 8 files changed, 37 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/dcebfc2d/phoenix-core/src/main/java/org/apache/phoenix/execute/ClientAggregatePlan.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/execute/ClientAggregatePlan.java b/phoenix-core/src/main/java/org/apache/phoenix/execute/ClientAggregatePlan.java index 9251724..8ef1f8d 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/execute/ClientAggregatePlan.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/execute/ClientAggregatePlan.java @@ -38,6 +38,7 @@ import org.apache.phoenix.coprocessor.BaseScannerRegionObserver; import org.apache.phoenix.expression.Expression; import org.apache.phoenix.expression.OrderByExpression; import org.apache.phoenix.expression.aggregator.Aggregators; +import org.apache.phoenix.expression.aggregator.ClientAggregators; import org.apache.phoenix.expression.aggregator.ServerAggregators; import org.apache.phoenix.iterate.AggregatingResultIterator; import org.apache.phoenix.iterate.BaseGroupedAggregatingResultIterator; @@ -68,18 +69,21 @@ import com.google.common.collect.Lists; public class ClientAggregatePlan extends ClientProcessingPlan { private final GroupBy groupBy; private final Expression having; - private final Aggregators serverAggregators; - private final Aggregators clientAggregators; + private final ServerAggregators serverAggregators; + private final ClientAggregators clientAggregators; public ClientAggregatePlan(StatementContext context, FilterableStatement statement, TableRef table, RowProjector projector, Integer limit, Integer offset, Expression where, OrderBy orderBy, GroupBy groupBy, Expression having, QueryPlan delegate) { super(context, statement, table, projector, limit, offset, where, orderBy, delegate); this.groupBy = groupBy; this.having = having; - this.serverAggregators = - ServerAggregators.deserialize(context.getScan() - .getAttribute(BaseScannerRegionObserver.AGGREGATORS), QueryServicesOptions.withDefaults().getConfiguration()); this.clientAggregators = context.getAggregationManager().getAggregators(); + // We must deserialize rather than clone based off of client aggregators because + // upon deserialization we create the server-side aggregators instead of the client-side + // aggregators. We use the Configuration directly here to avoid the expense of creating + // another one. + this.serverAggregators = ServerAggregators.deserialize(context.getScan() + .getAttribute(BaseScannerRegionObserver.AGGREGATORS), context.getConnection().getQueryServices().getConfiguration()); } @Override http://git-wip-us.apache.org/repos/asf/phoenix/blob/dcebfc2d/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/Aggregators.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/Aggregators.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/Aggregators.java index cf77c8e..b1dc658 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/Aggregators.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/Aggregators.java @@ -18,7 +18,6 @@ package org.apache.phoenix.expression.aggregator; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; - import org.apache.phoenix.expression.function.SingleAggregateFunction; import org.apache.phoenix.schema.KeyValueSchema; import org.apache.phoenix.schema.KeyValueSchema.KeyValueSchemaBuilder; @@ -58,6 +57,10 @@ abstract public class Aggregators { return schema; } + public int getMinNullableIndex() { + return schema.getMinNullable(); + } + @Override public String toString() { StringBuilder buf = new StringBuilder(this.getClass().getName() + " [" + functions.length + "]:"); http://git-wip-us.apache.org/repos/asf/phoenix/blob/dcebfc2d/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/ServerAggregators.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/ServerAggregators.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/ServerAggregators.java index 01ca733..366bbc6 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/ServerAggregators.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/aggregator/ServerAggregators.java @@ -37,8 +37,6 @@ import org.apache.phoenix.schema.tuple.Tuple; * * Aggregators that execute on the server-side * - * - * @since 0.1 */ public class ServerAggregators extends Aggregators { public static final ServerAggregators EMPTY_AGGREGATORS = new ServerAggregators(new SingleAggregateFunction[0], new Aggregator[0], new Expression[0], 0); http://git-wip-us.apache.org/repos/asf/phoenix/blob/dcebfc2d/phoenix-core/src/main/java/org/apache/phoenix/expression/function/SingleAggregateFunction.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/SingleAggregateFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/SingleAggregateFunction.java index 6155e1d..458ef87 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/SingleAggregateFunction.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/SingleAggregateFunction.java @@ -30,8 +30,8 @@ import org.apache.phoenix.expression.Expression; import org.apache.phoenix.expression.LiteralExpression; import org.apache.phoenix.expression.aggregator.Aggregator; import org.apache.phoenix.expression.visitor.ExpressionVisitor; -import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.tuple.Tuple; +import org.apache.phoenix.schema.types.PDataType; /** @@ -92,7 +92,7 @@ abstract public class SingleAggregateFunction extends AggregateFunction { private SingleAggregateFunction(List<Expression> children, boolean isConstant) { super(children); - this.isConstant = children.get(0) instanceof LiteralExpression; + this.isConstant = isConstant; this.aggregator = newClientAggregator(); } @@ -143,7 +143,7 @@ abstract public class SingleAggregateFunction extends AggregateFunction { return agg; } - public void readFields(DataInput input, Configuration conf) throws IOException { + public final void readFields(DataInput input, Configuration conf) throws IOException { super.readFields(input); aggregator = newServerAggregator(conf); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/dcebfc2d/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java index 0478e07..51716d0 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Properties; import java.util.Set; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HRegionLocation; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.client.HBaseAdmin; @@ -142,4 +143,6 @@ public interface ConnectionQueryServices extends QueryServices, MetaDataMutated boolean isUpgradeRequired(); void upgradeSystemTables(String url, Properties props) throws SQLException; + + public Configuration getConfiguration(); } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/phoenix/blob/dcebfc2d/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java index 356f0b8..3405564 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java @@ -4112,4 +4112,9 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement public boolean isUpgradeRequired() { return upgradeRequired.get(); } + + @Override + public Configuration getConfiguration() { + return config; + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/phoenix/blob/dcebfc2d/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java index e69a32f..6398a23 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java @@ -114,6 +114,7 @@ public class ConnectionlessQueryServicesImpl extends DelegateQueryServices imple private volatile SQLException initializationException; private final Map<String, List<HRegionLocation>> tableSplits = Maps.newHashMap(); private final GuidePostsCache guidePostsCache; + private final Configuration config; public ConnectionlessQueryServicesImpl(QueryServices services, ConnectionInfo connInfo, Properties info) { super(services); @@ -137,7 +138,7 @@ public class ConnectionlessQueryServicesImpl extends DelegateQueryServices imple // Without making a copy of the configuration we cons up, we lose some of our properties // on the server side during testing. - config = HBaseFactoryProvider.getConfigurationFactory().getConfiguration(config); + this.config = HBaseFactoryProvider.getConfigurationFactory().getConfiguration(config); TransactionManager txnManager = new TransactionManager(config); this.txSystemClient = new InMemoryTxSystemClient(txnManager); this.guidePostsCache = new GuidePostsCache(this, config); @@ -662,4 +663,9 @@ public class ConnectionlessQueryServicesImpl extends DelegateQueryServices imple @Override public void upgradeSystemTables(String url, Properties props) throws SQLException {} + + @Override + public Configuration getConfiguration() { + return config; + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/dcebfc2d/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java b/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java index 3a06ee2..685e583 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Properties; import java.util.Set; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HRegionLocation; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.client.HBaseAdmin; @@ -355,4 +356,9 @@ public class DelegateConnectionQueryServices extends DelegateQueryServices imple public void upgradeSystemTables(String url, Properties props) throws SQLException { getDelegate().upgradeSystemTables(url, props); } + + @Override + public Configuration getConfiguration() { + return getDelegate().getConfiguration(); + } } \ No newline at end of file