You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ja...@apache.org on 2016/11/02 20:22:36 UTC

[2/2] phoenix git commit: PHOENIX-3434 Avoid creating new Configuration in ClientAggregatePlan to improve performance

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/25ea0ba4
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/25ea0ba4
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/25ea0ba4

Branch: refs/heads/4.x-HBase-1.1
Commit: 25ea0ba4c5ff7e32c6206ccbe6dfc9b7a9b23aa2
Parents: 0c2e7dd
Author: James Taylor <ja...@apache.org>
Authored: Wed Nov 2 11:42:41 2016 -0700
Committer: James Taylor <ja...@apache.org>
Committed: Wed Nov 2 13:21:32 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/25ea0ba4/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/25ea0ba4/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/25ea0ba4/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/25ea0ba4/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/25ea0ba4/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/25ea0ba4/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 4e31cf0..ea25141 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
@@ -4120,4 +4120,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/25ea0ba4/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 1b1e429..b61b6f0 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
     public boolean isUpgradeRequired() {
         return false;
     }
+
+    @Override
+    public Configuration getConfiguration() {
+        return config;
+    }
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/25ea0ba4/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 7466e9c..725af2b 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 boolean isUpgradeRequired() {
         return getDelegate().isUpgradeRequired();
     }
+
+    @Override
+    public Configuration getConfiguration() {
+        return getDelegate().getConfiguration();
+    }
 }
\ No newline at end of file