stoty commented on code in PR #1690:
URL: https://github.com/apache/phoenix/pull/1690#discussion_r1345218731


##########
phoenix-coprocessors/pom.xml:
##########
@@ -0,0 +1,220 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0";
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+    <parent>
+        <artifactId>phoenix</artifactId>
+        <groupId>org.apache.phoenix</groupId>
+        <version>5.2.0-SNAPSHOT</version>
+    </parent>
+    <modelVersion>4.0.0</modelVersion>
+
+    <artifactId>phoenix-coprocessors</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.phoenix</groupId>
+            <artifactId>phoenix-core</artifactId>
+        </dependency>
+
+        <!-- shaded thirdparty dependencies -->
+        <dependency>
+            <groupId>org.apache.phoenix.thirdparty</groupId>
+            <artifactId>phoenix-shaded-guava</artifactId>
+        </dependency>
+
+        <!-- HBase compat dependency -->
+        <dependency>
+            <groupId>org.apache.phoenix</groupId>
+            
<artifactId>phoenix-hbase-compat-${hbase.compat.version}</artifactId>
+            <optional>true</optional>
+        </dependency>
+
+        <!-- Hadoop dependencies -->
+        <dependency>
+            <groupId>org.apache.hadoop</groupId>
+            <artifactId>hadoop-common</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hadoop</groupId>
+            <artifactId>hadoop-yarn-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hadoop</groupId>
+            <artifactId>hadoop-mapreduce-client-core</artifactId>
+        </dependency>
+
+        <!-- HBase dependencies -->
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-common</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-mapreduce</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-metrics</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-server</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-zookeeper</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-metrics-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-client</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-protocol</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-protocol-shaded</artifactId>
+        </dependency>
+
+        <!-- HBase Adjacent Dependencies -->
+        <dependency>
+            <groupId>org.apache.zookeeper</groupId>
+            <artifactId>zookeeper</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.zookeeper</groupId>
+            <artifactId>zookeeper-jute</artifactId>
+        </dependency>
+
+        <!-- Omid dependencies -->
+        <dependency>
+            <groupId>org.apache.omid</groupId>
+            <artifactId>omid-hbase-coprocessor</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.omid</groupId>
+            <artifactId>omid-commit-table</artifactId>
+        </dependency>
+
+
+        <!-- Other dependencies -->
+        <dependency>
+            <groupId>org.antlr</groupId>
+            <artifactId>antlr-runtime</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.github.stephenc.findbugs</groupId>
+            <artifactId>findbugs-annotations</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.htrace</groupId>
+            <artifactId>htrace-core</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.google.protobuf</groupId>
+            <artifactId>protobuf-java</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-annotations</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>joda-time</groupId>
+            <artifactId>joda-time</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-csv</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.github.stephenc.findbugs</groupId>
+            <artifactId>findbugs-annotations</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.phoenix.thirdparty</groupId>
+            <artifactId>phoenix-shaded-commons-cli</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-core</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-databind</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-lang3</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>commons-codec</groupId>
+            <artifactId>commons-codec</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.google.code.findbugs</groupId>
+            <artifactId>jsr305</artifactId>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.codehaus.mojo</groupId>
+                <artifactId>build-helper-maven-plugin</artifactId>
+            </plugin>
+            <plugin>
+                <artifactId>maven-dependency-plugin</artifactId>
+                <configuration>
+                    <ignoreNonCompile>true</ignoreNonCompile>
+                    <!--           <ignoredUnusedDeclaredDependencies> -->
+                    <!--             <ignoredUnusedDeclaredDependency> -->
+                    <!--               org.apache.hbase:hbase-it -->
+                    <!--             </ignoredUnusedDeclaredDependency> -->
+                    <!--             <ignoredUnusedDeclaredDependency> -->
+                    <!--               
org.apache.phoenix:phoenix-hbase-compat-${hbase.compat.version} -->
+                    <!--             </ignoredUnusedDeclaredDependency> -->
+                    <!--           </ignoredUnusedDeclaredDependencies> -->
+                </configuration>
+                <executions>
+                    <execution>
+                        <!-- generates the file that will be used by the 
sandbox script

Review Comment:
   Please check that the sandbox environment still works.



##########
phoenix-coprocessors/src/main/java/org/apache/phoenix/schema/stats/DefaultStatisticsCollector.java:
##########
@@ -40,11 +40,11 @@
 import org.apache.hadoop.hbase.regionserver.Region;
 import org.apache.hadoop.hbase.regionserver.Store;
 import org.apache.hadoop.hbase.util.Pair;
-import org.apache.phoenix.coprocessor.MetaDataProtocol;
 import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr;
 import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
 import org.apache.phoenix.query.QueryServices;
 import org.apache.phoenix.query.QueryServicesOptions;
+import org.apache.phoenix.scanattributes.MetaDataProtocol;

Review Comment:
   Change not needed



##########
phoenix-core/src/main/java/org/apache/phoenix/hbase/index/IndexerStatic.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.phoenix.hbase.index;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.hadoop.hbase.client.CoprocessorDescriptorBuilder;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.phoenix.query.QueryConstants;
+
+public class IndexerStatic {

Review Comment:
   Move to IndexUtil or rename to IndexerUtil ?



##########
phoenix-coprocessors/pom.xml:
##########
@@ -0,0 +1,220 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0";
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+    <parent>
+        <artifactId>phoenix</artifactId>
+        <groupId>org.apache.phoenix</groupId>
+        <version>5.2.0-SNAPSHOT</version>
+    </parent>
+    <modelVersion>4.0.0</modelVersion>
+
+    <artifactId>phoenix-coprocessors</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.phoenix</groupId>
+            <artifactId>phoenix-core</artifactId>
+        </dependency>
+
+        <!-- shaded thirdparty dependencies -->
+        <dependency>
+            <groupId>org.apache.phoenix.thirdparty</groupId>
+            <artifactId>phoenix-shaded-guava</artifactId>
+        </dependency>
+
+        <!-- HBase compat dependency -->
+        <dependency>
+            <groupId>org.apache.phoenix</groupId>
+            
<artifactId>phoenix-hbase-compat-${hbase.compat.version}</artifactId>
+            <optional>true</optional>
+        </dependency>
+
+        <!-- Hadoop dependencies -->
+        <dependency>
+            <groupId>org.apache.hadoop</groupId>
+            <artifactId>hadoop-common</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hadoop</groupId>
+            <artifactId>hadoop-yarn-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hadoop</groupId>
+            <artifactId>hadoop-mapreduce-client-core</artifactId>
+        </dependency>
+
+        <!-- HBase dependencies -->
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-common</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-mapreduce</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-metrics</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-server</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-zookeeper</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-metrics-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-client</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-protocol</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hbase</groupId>
+            <artifactId>hbase-protocol-shaded</artifactId>
+        </dependency>
+
+        <!-- HBase Adjacent Dependencies -->
+        <dependency>
+            <groupId>org.apache.zookeeper</groupId>
+            <artifactId>zookeeper</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.zookeeper</groupId>
+            <artifactId>zookeeper-jute</artifactId>
+        </dependency>
+
+        <!-- Omid dependencies -->
+        <dependency>
+            <groupId>org.apache.omid</groupId>
+            <artifactId>omid-hbase-coprocessor</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.omid</groupId>
+            <artifactId>omid-commit-table</artifactId>
+        </dependency>
+
+
+        <!-- Other dependencies -->
+        <dependency>
+            <groupId>org.antlr</groupId>
+            <artifactId>antlr-runtime</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.github.stephenc.findbugs</groupId>
+            <artifactId>findbugs-annotations</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.htrace</groupId>
+            <artifactId>htrace-core</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.google.protobuf</groupId>
+            <artifactId>protobuf-java</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-annotations</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>joda-time</groupId>
+            <artifactId>joda-time</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-csv</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.github.stephenc.findbugs</groupId>
+            <artifactId>findbugs-annotations</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.phoenix.thirdparty</groupId>
+            <artifactId>phoenix-shaded-commons-cli</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-core</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-databind</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-lang3</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>commons-codec</groupId>
+            <artifactId>commons-codec</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.google.code.findbugs</groupId>
+            <artifactId>jsr305</artifactId>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.codehaus.mojo</groupId>
+                <artifactId>build-helper-maven-plugin</artifactId>
+            </plugin>
+            <plugin>
+                <artifactId>maven-dependency-plugin</artifactId>
+                <configuration>
+                    <ignoreNonCompile>true</ignoreNonCompile>
+                    <!--           <ignoredUnusedDeclaredDependencies> -->

Review Comment:
   You can delete these



##########
phoenix-core/src/main/java/org/apache/phoenix/execute/PhoenixTxIndexMutationGenerator.java:
##########
@@ -64,6 +62,7 @@
 import org.apache.phoenix.jdbc.PhoenixConnection;
 import org.apache.phoenix.query.ConnectionQueryServices;
 import org.apache.phoenix.query.KeyRange;
+import org.apache.phoenix.scanattributes.MetaDataProtocol;

Review Comment:
   change not needed



##########
phoenix-coprocessors/src/main/java/org/apache/phoenix/schema/transform/Transform.java:
##########
@@ -17,19 +17,14 @@
  */
 package org.apache.phoenix.schema.transform;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
 import org.apache.hadoop.hbase.util.Bytes;
-import org.apache.phoenix.coprocessor.TableInfo;
+import org.apache.phoenix.scanattributes.TableInfo;

Review Comment:
   Find some other package name, this is not a scan attribute.
   (I know it's because I asked you to rename the package to scanatributes.)



##########
phoenix-coprocessors/src/main/java/org/apache/phoenix/coprocessor/GlobalIndexRegionScanner.java:
##########
@@ -87,7 +90,7 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 
-import static 
org.apache.phoenix.hbase.index.IndexRegionObserver.removeEmptyColumn;
+import static 
org.apache.phoenix.hbase.index.IndexRegionObserverConstants.removeEmptyColumn;

Review Comment:
   This is not a contstant.
   Move to IndexUtil% instead ?



##########
phoenix-coprocessors/src/main/java/org/apache/phoenix/schema/transform/Transform.java:
##########
@@ -67,192 +57,24 @@
 import java.util.List;
 import java.util.Map;
 
-import static 
org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_TABLE_TIMESTAMP;
+import static 
org.apache.phoenix.scanattributes.MetaDataProtocol.MIN_TABLE_TIMESTAMP;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.ENCODING_SCHEME;
 import static 
org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.IMMUTABLE_STORAGE_SCHEME;
 import static 
org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.PHYSICAL_TABLE_NAME;
-import static 
org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.SYSTEM_TRANSFORM_NAME;
-import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TRANSFORM_STATUS;
 import static 
org.apache.phoenix.mapreduce.util.PhoenixConfigurationUtil.MAPREDUCE_TENANT_ID;
 import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY;
 import static 
org.apache.phoenix.query.QueryConstants.ENCODED_CQ_COUNTER_INITIAL_VALUE;
-import static 
org.apache.phoenix.query.QueryServices.INDEX_CREATE_DEFAULT_STATE;
 import static org.apache.phoenix.query.QueryServices.MUTATE_BATCH_SIZE_ATTRIB;
 import static org.apache.phoenix.schema.ColumnMetaDataOps.addColumnMutation;
-import static org.apache.phoenix.schema.MetaDataClient.CREATE_LINK;
 import static 
org.apache.phoenix.schema.MetaDataClient.UPDATE_INDEX_STATE_TO_ACTIVE;
 import static 
org.apache.phoenix.schema.PTable.ImmutableStorageScheme.SINGLE_CELL_ARRAY_WITH_OFFSETS;
 import static org.apache.phoenix.schema.PTableType.INDEX;
-import static org.apache.phoenix.schema.PTableType.VIEW;
 
-public class Transform {
+public class Transform extends TransformHelper {

Review Comment:
   Would it mke sense to call TransaformHelper TransformClient ?



##########
phoenix-coprocessors/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java:
##########
@@ -612,15 +485,9 @@ public void 
setScanOptionsForFlushesAndCompactionsWhenPhoenixTTLIsDisabled(Confi
         options.setMinVersions(getMinVersions(options, cfDescriptor));
     }
 
-    public static long getMaxLookbackInMillis(Configuration conf){
-        //config param is in seconds, switch to millis
-        return conf.getLong(PHOENIX_MAX_LOOKBACK_AGE_CONF_KEY,
-            DEFAULT_PHOENIX_MAX_LOOKBACK_AGE) * 1000;
-    }
-
     public static boolean isMaxLookbackTimeEnabled(Configuration conf){
-        return 
isMaxLookbackTimeEnabled(conf.getLong(PHOENIX_MAX_LOOKBACK_AGE_CONF_KEY,

Review Comment:
   This is not a scan attribute.
   It may be better to move this to QueryServicesOptions.



##########
phoenix-coprocessors/src/main/java/org/apache/phoenix/schema/stats/StatisticsWriter.java:
##########
@@ -52,13 +52,13 @@
 import org.apache.hadoop.hbase.regionserver.Region;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.util.Bytes;
-import org.apache.phoenix.coprocessor.MetaDataProtocol;
 import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr;
 import org.apache.phoenix.jdbc.PhoenixConnection;
 import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
 import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.query.QueryServices;
 import org.apache.phoenix.query.QueryServicesOptions;
+import org.apache.phoenix.scanattributes.MetaDataProtocol;

Review Comment:
   change not needed



##########
phoenix-coprocessors/src/main/java/org/apache/phoenix/mapreduce/transform/PhoenixTransformWithViewsInputFormat.java:
##########
@@ -27,11 +27,11 @@
 import org.apache.phoenix.compile.MutationPlan;
 import org.apache.phoenix.compile.QueryPlan;
 import org.apache.phoenix.compile.ServerBuildTransformingTableCompiler;
-import org.apache.phoenix.coprocessor.TableInfo;
 import org.apache.phoenix.jdbc.PhoenixConnection;
 import org.apache.phoenix.mapreduce.PhoenixServerBuildIndexInputFormat;
 import org.apache.phoenix.mapreduce.util.ConnectionUtil;
 import org.apache.phoenix.mapreduce.util.PhoenixConfigurationUtil;
+import org.apache.phoenix.scanattributes.TableInfo;

Review Comment:
   only import order changes here, not needed.



##########
phoenix-coprocessors/src/main/java/org/apache/phoenix/coprocessor/AddColumnMutator.java:
##########
@@ -443,16 +435,15 @@ public MetaDataMutationResult 
validateAndAddMetadata(PTable table, byte[][] rowK
     }
 
     private MetaDataMutationResult compareWithPkColumns(byte[] colName,
-            PTable table, byte[] familyName) {
+                                                        PTable table, byte[] 
familyName) {

Review Comment:
   nit:
   This looks worse than before



##########
phoenix-coprocessors/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java:
##########
@@ -56,142 +56,15 @@
 import org.apache.phoenix.iterate.RegionScannerFactory;
 import org.apache.phoenix.query.QueryServices;
 import org.apache.phoenix.query.QueryServicesOptions;
+import org.apache.phoenix.scanattributes.BaseScannerRegionObserverConstants;
 import org.apache.phoenix.schema.StaleRegionBoundaryCacheException;
-import org.apache.phoenix.schema.types.PUnsignedTinyint;
+import org.apache.phoenix.util.ClientUtil;
 import org.apache.phoenix.util.ScanUtil;
 import org.apache.phoenix.util.ServerUtil;
 
 import static org.apache.phoenix.util.ScanUtil.getPageSizeMsForFilter;
 
 abstract public class BaseScannerRegionObserver implements RegionObserver {
-
-    public static final String AGGREGATORS = "_Aggs";
-    public static final String UNORDERED_GROUP_BY_EXPRESSIONS = 
"_UnorderedGroupByExpressions";
-    public static final String KEY_ORDERED_GROUP_BY_EXPRESSIONS = 
"_OrderedGroupByExpressions";
-    public static final String ESTIMATED_DISTINCT_VALUES = 
"_EstDistinctValues";
-    public static final String NON_AGGREGATE_QUERY = "_NonAggregateQuery";
-    public static final String TOPN = "_TopN";
-    public static final String UNGROUPED_AGG = "_UngroupedAgg";
-    public static final String DELETE_AGG = "_DeleteAgg";
-    public static final String UPSERT_SELECT_TABLE = "_UpsertSelectTable";
-    public static final String UPSERT_SELECT_EXPRS = "_UpsertSelectExprs";
-    public static final String DELETE_CQ = "_DeleteCQ";
-    public static final String DELETE_CF = "_DeleteCF";
-    public static final String EMPTY_CF = "_EmptyCF";
-    public static final String EMPTY_COLUMN_QUALIFIER = 
"_EmptyColumnQualifier";
-    public static final String SPECIFIC_ARRAY_INDEX = "_SpecificArrayIndex";
-    public static final String GROUP_BY_LIMIT = "_GroupByLimit";
-    public static final String LOCAL_INDEX = "_LocalIndex";
-    public static final String LOCAL_INDEX_BUILD = "_LocalIndexBuild";
-    public static final String UNCOVERED_GLOBAL_INDEX = 
"_UncoveredGlobalIndex";
-    public static final String INDEX_REBUILD_PAGING = "_IndexRebuildPaging";
-    // The number of index rows to be rebuild in one RPC call
-    public static final String INDEX_REBUILD_PAGE_ROWS = 
"_IndexRebuildPageRows";
-    public static final String INDEX_PAGE_ROWS = "_IndexPageRows";
-    public static final String SERVER_PAGE_SIZE_MS = "_ServerPageSizeMs";
-    // Index verification type done by the index tool
-    public static final String INDEX_REBUILD_VERIFY_TYPE = 
"_IndexRebuildVerifyType";
-    public static final String INDEX_RETRY_VERIFY = "_IndexRetryVerify";
-    public static final String INDEX_REBUILD_DISABLE_LOGGING_VERIFY_TYPE =
-        "_IndexRebuildDisableLoggingVerifyType";
-    public static final String 
INDEX_REBUILD_DISABLE_LOGGING_BEYOND_MAXLOOKBACK_AGE =
-        "_IndexRebuildDisableLoggingBeyondMaxLookbackAge";
-    @Deprecated
-    public static final String LOCAL_INDEX_FILTER = "_LocalIndexFilter";
-    @Deprecated
-    public static final String LOCAL_INDEX_LIMIT = "_LocalIndexLimit";
-    @Deprecated
-    public static final String LOCAL_INDEX_FILTER_STR = "_LocalIndexFilterStr";
-    public static final String INDEX_FILTER = "_IndexFilter";
-    public static final String INDEX_LIMIT = "_IndexLimit";
-    public static final String INDEX_FILTER_STR = "_IndexFilterStr";
-
-    /* 
-    * Attribute to denote that the index maintainer has been serialized using 
its proto-buf presentation.
-    * Needed for backward compatibility purposes. TODO: get rid of this in 
next major release.
-    */
-    public static final String LOCAL_INDEX_BUILD_PROTO = "_LocalIndexBuild"; 
-    public static final String LOCAL_INDEX_JOIN_SCHEMA = 
"_LocalIndexJoinSchema";
-    public static final String DATA_TABLE_COLUMNS_TO_JOIN = 
"_DataTableColumnsToJoin";
-    public static final String COLUMNS_STORED_IN_SINGLE_CELL = 
"_ColumnsStoredInSingleCell";
-    public static final String VIEW_CONSTANTS = "_ViewConstants";
-    public static final String EXPECTED_UPPER_REGION_KEY = 
"_ExpectedUpperRegionKey";
-    public static final String REVERSE_SCAN = "_ReverseScan";
-    public static final String ANALYZE_TABLE = "_ANALYZETABLE";
-    public static final String REBUILD_INDEXES = "_RebuildIndexes";
-    public static final String DO_TRANSFORMING = "_DoTransforming";
-    public static final String TX_STATE = "_TxState";
-    public static final String GUIDEPOST_WIDTH_BYTES = 
"_GUIDEPOST_WIDTH_BYTES";
-    public static final String GUIDEPOST_PER_REGION = "_GUIDEPOST_PER_REGION";
-    public static final String UPGRADE_DESC_ROW_KEY = "_UPGRADE_DESC_ROW_KEY";
-    public static final String SCAN_REGION_SERVER = "_SCAN_REGION_SERVER";
-    public static final String RUN_UPDATE_STATS_ASYNC_ATTRIB = 
"_RunUpdateStatsAsync";
-    public static final String SKIP_REGION_BOUNDARY_CHECK = 
"_SKIP_REGION_BOUNDARY_CHECK";
-    public static final String TX_SCN = "_TxScn";
-    public static final String PHOENIX_TTL = "_PhoenixTTL";
-    public static final String MASK_PHOENIX_TTL_EXPIRED = "_MASK_TTL_EXPIRED";
-    public static final String DELETE_PHOENIX_TTL_EXPIRED = 
"_DELETE_TTL_EXPIRED";
-    public static final String PHOENIX_TTL_SCAN_TABLE_NAME = 
"_PhoenixTTLScanTableName";
-    public static final String SCAN_ACTUAL_START_ROW = "_ScanActualStartRow";
-    public static final String REPLAY_WRITES = "_IGNORE_NEWER_MUTATIONS";
-    public final static String SCAN_OFFSET = "_RowOffset";
-    public static final String SCAN_START_ROW_SUFFIX = "_ScanStartRowSuffix";
-    public static final String SCAN_STOP_ROW_SUFFIX = "_ScanStopRowSuffix";
-    public final static String MIN_QUALIFIER = "_MinQualifier";
-    public final static String MAX_QUALIFIER = "_MaxQualifier";
-    public final static String USE_NEW_VALUE_COLUMN_QUALIFIER = 
"_UseNewValueColumnQualifier";
-    public final static String QUALIFIER_ENCODING_SCHEME = 
"_QualifierEncodingScheme";
-    public final static String IMMUTABLE_STORAGE_ENCODING_SCHEME = 
"_ImmutableStorageEncodingScheme";
-    public final static String USE_ENCODED_COLUMN_QUALIFIER_LIST = 
"_UseEncodedColumnQualifierList";
-    public static final String CLIENT_VERSION = "_ClientVersion";
-    public static final String CHECK_VERIFY_COLUMN = "_CheckVerifyColumn";
-    public static final String PHYSICAL_DATA_TABLE_NAME = 
"_PhysicalDataTableName";
-    public static final String EMPTY_COLUMN_FAMILY_NAME = "_EmptyCFName";
-    public static final String EMPTY_COLUMN_QUALIFIER_NAME = "_EmptyCQName";
-    public static final String INDEX_ROW_KEY = "_IndexRowKey";
-    public static final String READ_REPAIR_TRANSFORMING_TABLE = 
"_ReadRepairTransformingTable";
-    
-    public final static byte[] REPLAY_TABLE_AND_INDEX_WRITES = 
PUnsignedTinyint.INSTANCE.toBytes(1);
-    public final static byte[] REPLAY_ONLY_INDEX_WRITES = 
PUnsignedTinyint.INSTANCE.toBytes(2);
-    // In case of Index Write failure, we need to determine that Index mutation
-    // is part of normal client write or Index Rebuilder. # PHOENIX-5080
-    public final static byte[] REPLAY_INDEX_REBUILD_WRITES = 
PUnsignedTinyint.INSTANCE.toBytes(3);
-
-    public static final String PHOENIX_MAX_LOOKBACK_AGE_CONF_KEY =
-            "phoenix.max.lookback.age.seconds";
-    public static final int DEFAULT_PHOENIX_MAX_LOOKBACK_AGE = 0;
-
-    public enum ReplayWrite {
-        TABLE_AND_INDEX,
-        INDEX_ONLY,
-        REBUILD_INDEX_ONLY;
-        
-        public static ReplayWrite fromBytes(byte[] replayWriteBytes) {
-            if (replayWriteBytes == null) {
-                return null;
-            }
-            if (Bytes.compareTo(REPLAY_TABLE_AND_INDEX_WRITES, 
replayWriteBytes) == 0) {
-                return TABLE_AND_INDEX;
-            }
-            if (Bytes.compareTo(REPLAY_ONLY_INDEX_WRITES, replayWriteBytes) == 
0) {
-                return INDEX_ONLY;
-            }
-            if (Bytes.compareTo(REPLAY_INDEX_REBUILD_WRITES, replayWriteBytes) 
== 0) {
-                return REBUILD_INDEX_ONLY;
-            }
-            throw new IllegalArgumentException("Unknown ReplayWrite code of " 
+ Bytes.toStringBinary(replayWriteBytes));
-        }
-    };
-    
-    /**
-     * Attribute name used to pass custom annotations in Scans and Mutations 
(later). Custom annotations
-     * are used to augment log lines emitted by Phoenix. See 
https://issues.apache.org/jira/browse/PHOENIX-1198.
-     */
-    public static final String CUSTOM_ANNOTATIONS = "_Annot";
-
-    /** Exposed for testing */
-    public static final String SCANNER_OPENED_TRACE_INFO = "Scanner opened on 
server";

Review Comment:
   This is not a scanner attribute.
   Can we leave it, or move it elsewhere ?



##########
phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java:
##########
@@ -22,11 +22,11 @@
 import java.sql.SQLTimeoutException;
 import java.util.Map;
 
-import org.apache.phoenix.coprocessor.MetaDataProtocol;
 import org.apache.phoenix.hbase.index.util.IndexManagementUtil;
 import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
 import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.scanattributes.MetaDataProtocol;

Review Comment:
   change not needed



##########
phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java:
##########
@@ -51,15 +51,14 @@
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.phoenix.compile.QueryPlan;
 import org.apache.phoenix.compile.ScanRanges;
-import org.apache.phoenix.coprocessor.MetaDataProtocol;
-import org.apache.phoenix.coprocessor.ServerCachingProtocol.ServerCacheFactory;
+import org.apache.phoenix.scanattributes.MetaDataProtocol;

Review Comment:
   scanattributes package name is a poor fit for these.
   maybe we should add another package for the non-attributes, or move them 
into another package.



##########
phoenix-core/src/main/java/org/apache/phoenix/cache/TenantCache.java:
##########
@@ -21,10 +21,9 @@
 import java.sql.SQLException;
 
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
-import org.apache.phoenix.coprocessor.ServerCachingProtocol.ServerCacheFactory;
 import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr;
 import org.apache.phoenix.memory.MemoryManager;
-
+import 
org.apache.phoenix.scanattributes.ServerCachingProtocol.ServerCacheFactory;

Review Comment:
   ditto



##########
phoenix-core/src/main/java/org/apache/phoenix/filter/SystemCatalogViewIndexIdFilter.java:
##########
@@ -36,10 +36,10 @@
 import java.util.List;
 
 
-import static 
org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
 import static 
org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
 import static 
org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
 import static 
org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static 
org.apache.phoenix.scanattributes.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;

Review Comment:
   change not needed



##########
phoenix-core/src/main/java/org/apache/phoenix/util/ClientUtil.java:
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.phoenix.util;
+
+import java.io.IOException;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.hadoop.hbase.DoNotRetryIOException;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.NotServingRegionException;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.RetriesExhaustedWithDetailsException;
+import org.apache.phoenix.coprocessor.HashJoinCacheNotFoundException;
+import org.apache.phoenix.exception.PhoenixIOException;
+import org.apache.phoenix.exception.SQLExceptionCode;
+import org.apache.phoenix.exception.SQLExceptionInfo;
+import org.apache.phoenix.schema.StaleRegionBoundaryCacheException;
+
+public class ClientUtil {

Review Comment:
   Thanks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to