PHOENIX-3434 Avoid creating new Configuration in ClientAggregatePlan to improve performance
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.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 @@
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
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 @@
return schema;
}
+ public int getMinNullableIndex() {
+ return schema.getMinNullable();
+ }
+
@Override
public String toString() {
StringBuilder buf = new StringBuilder(this.getClass().getName() + " [" + functions.length + "]:");
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 @@
*
* 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);
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 6155e1dc..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.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 @@
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 @@
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);
}
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.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 @@
boolean isUpgradeRequired();
void upgradeSystemTables(String url, Properties props) throws SQLException;
+
+ public Configuration getConfiguration();
}
\ No newline at end of file
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 boolean isUpgradeRequired() {
return upgradeRequired.get();
}
+
+ @Override
+ public Configuration getConfiguration() {
+ return config;
+ }
}
\ No newline at end of file
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 @@
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 @@
// 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 @@
@Override
public void upgradeSystemTables(String url, Properties props) throws SQLException {}
+
+ @Override
+ public Configuration getConfiguration() {
+ return config;
+ }
}
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.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 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
