You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/05/06 06:15:27 UTC

[GitHub] [phoenix] vmeka2020 opened a new pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

vmeka2020 opened a new pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224


   PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries 


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-837640932


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 27s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  15m 49s |  master passed  |
   | +0 |  hbaserecompile  |  22m 34s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  5s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   3m  4s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 52s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m  8s |  phoenix-core in master has 959 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   8m 10s |  the patch passed  |
   | +0 |  hbaserecompile  |  18m 11s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 12s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 12s |  the patch passed  |
   | -1 :x: |  checkstyle  |   3m 26s |  phoenix-core: The patch generated 690 new + 6087 unchanged - 74 fixed = 6777 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 49s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 14s |  phoenix-core generated 6 new + 959 unchanged - 0 fixed = 965 total (was 959)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 111m 51s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 47s |  The patch does not generate ASF License warnings.  |
   |  |   | 170m 55s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  org.apache.phoenix.monitoring.NoOpTableMetricsManager.noOpsTableMetricManager isn't final but should be  At NoOpTableMetricsManager.java:be  At NoOpTableMetricsManager.java:[line 25] |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 87] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 56] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 68] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 60] |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/7/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux b61178f59d99 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 6c22b43 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/7/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/7/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/7/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/7/testReport/ |
   | Max. process+thread count | 15443 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/7/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi merged pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi merged pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629742377



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/JmxMetricProvider.java
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.monitoring;
+
+import org.apache.hadoop.hbase.metrics.Gauge;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.metrics.MetricRegistry;
+import org.apache.hadoop.hbase.metrics.MetricRegistryInfo;
+
+import java.util.Map;
+

Review comment:
       i saw the class level comment on 4.x branch, can you add it here thanks
   

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/JmxMetricProvider.java
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.monitoring;
+
+import org.apache.hadoop.hbase.metrics.Gauge;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.metrics.MetricRegistry;
+import org.apache.hadoop.hbase.metrics.MetricRegistryInfo;
+
+import java.util.Map;
+

Review comment:
       /**
    * This class implements the JMX based default Metric publishing
    * of Metrics to JMX end point.
    * This class is defined in phoenix/phoenix-core/src/main/resources/META-INF/services/org.apache.phoenix.monitoring.MetricPublisherSupplierFactory
    */




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-840241076


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 34s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  16m  8s |  master passed  |
   | +0 |  hbaserecompile  |  22m 53s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  4s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   3m  6s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m  5s |  phoenix-core in master has 960 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   8m 12s |  the patch passed  |
   | +0 |  hbaserecompile  |  18m  8s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  2s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  2s |  the patch passed  |
   | -1 :x: |  checkstyle  |   3m 29s |  phoenix-core: The patch generated 701 new + 6087 unchanged - 74 fixed = 6788 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  1s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 49s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 20s |  phoenix-core generated 5 new + 960 unchanged - 0 fixed = 965 total (was 960)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 113m  1s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 46s |  The patch does not generate ASF License warnings.  |
   |  |   | 172m 30s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 88] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 69] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 58] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 61] |
   | Failed junit tests | phoenix.end2end.PermissionNSEnabledWithCustomAccessControllerIT |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/11/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux f66894b3dc95 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / df32aec |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/11/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/11/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/11/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/11/testReport/ |
   | Max. process+thread count | 16027 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/11/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] vmeka2020 commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
vmeka2020 commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629782451



##########
File path: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java
##########
@@ -0,0 +1,1239 @@
+/*
+ * 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.monitoring;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.client.RetriesExhaustedWithDetailsException;
+import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
+import org.apache.phoenix.end2end.BaseUniqueNamesOwnClusterIT;
+import org.apache.phoenix.exception.PhoenixIOException;
+import org.apache.phoenix.exception.SQLExceptionInfo;
+import org.apache.phoenix.execute.CommitException;
+import org.apache.phoenix.jdbc.PhoenixConnection;
+import org.apache.phoenix.jdbc.PhoenixEmbeddedDriver;
+import org.apache.phoenix.jdbc.PhoenixTestDriver;
+import org.apache.phoenix.query.BaseTest;
+import org.apache.phoenix.query.ConfigurationFactory;
+import org.apache.phoenix.query.ConnectionQueryServices;
+import org.apache.phoenix.query.ConnectionQueryServicesImpl;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.query.QueryServicesTestImpl;
+import org.apache.phoenix.util.EnvironmentEdge;
+import org.apache.phoenix.util.EnvironmentEdgeManager;
+import org.apache.phoenix.util.InstanceResolver;
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.apache.phoenix.util.DelayedOrFailingRegionServer;
+import org.apache.phoenix.util.ReadOnlyProps;
+
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.phoenix.exception.SQLExceptionCode.DATA_EXCEEDS_MAX_CAPACITY;
+import static org.apache.phoenix.exception.SQLExceptionCode.GET_TABLE_REGIONS_FAIL;
+import static org.apache.phoenix.exception.SQLExceptionCode.OPERATION_TIMED_OUT;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_AGGREGATE_FAILURE_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_AGGREGATE_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_BATCH_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_BATCH_FAILED_SIZE;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_COMMIT_TIME;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_MUTATION_BYTES;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_MUTATION_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_SQL_QUERY_TIME;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.MUTATION_BATCH_FAILED_SIZE;
+import static org.apache.phoenix.monitoring.MetricType.MUTATION_BATCH_SIZE;
+import static org.apache.phoenix.monitoring.MetricType.MUTATION_BYTES;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_POINTLOOKUP_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_POINTLOOKUP_TIMEOUT_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_SCAN_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_SCAN_TIMEOUT_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_TIMEOUT_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.RESULT_SET_TIME_MS;
+import static org.apache.phoenix.monitoring.MetricType.SCAN_BYTES;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_AGGREGATE_FAILURE_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_AGGREGATE_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_POINTLOOKUP_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_POINTLOOKUP_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SCAN_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SCAN_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SQL_QUERY_TIME;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_AGGREGATE_FAILURE_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_AGGREGATE_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_BATCH_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_BATCH_FAILED_SIZE;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_COMMIT_TIME;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_MUTATION_BYTES;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_MUTATION_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_SQL_QUERY_TIME;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.POINT_LOOKUP_SELECT_QUERY;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.RANGE_SCAN_SELECT_QUERY;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.createTableAndInsertValues;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.doPointDeleteFromTable;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.doDeleteAllFromTable;
+import static org.apache.phoenix.util.DelayedOrFailingRegionServer.INJECTED_EXCEPTION_STRING;
+import static org.apache.phoenix.util.PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR;
+import static org.apache.phoenix.util.PhoenixRuntime.clearTableLevelMetrics;
+import static org.apache.phoenix.util.PhoenixRuntime.getOverAllReadRequestMetricInfo;
+import static org.apache.phoenix.util.PhoenixRuntime.getPhoenixTableClientMetrics;
+import static org.apache.phoenix.util.PhoenixRuntime.getRequestReadMetricInfo;
+import static org.apache.phoenix.util.PhoenixRuntime.getWriteMetricInfoForMutationsSinceLastReset;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicLong;
+
+public class PhoenixTableLevelMetricsIT extends BaseUniqueNamesOwnClusterIT {

Review comment:
       Added.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-837813137


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  16m  8s |  master passed  |
   | +0 |  hbaserecompile  |  22m 30s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  4s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   3m  6s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m  7s |  phoenix-core in master has 959 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   8m 13s |  the patch passed  |
   | +0 |  hbaserecompile  |  18m  2s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  5s |  the patch passed  |
   | -1 :x: |  checkstyle  |   3m 27s |  phoenix-core: The patch generated 694 new + 6087 unchanged - 74 fixed = 6781 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  1s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 52s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 21s |  phoenix-core generated 6 new + 959 unchanged - 0 fixed = 965 total (was 959)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 111m 37s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 46s |  The patch does not generate ASF License warnings.  |
   |  |   | 170m 33s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  org.apache.phoenix.monitoring.NoOpTableMetricsManager.noOpsTableMetricManager isn't final but should be  At NoOpTableMetricsManager.java:be  At NoOpTableMetricsManager.java:[line 30] |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 88] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 69] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 58] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 61] |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/8/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux 5d9237fe425a 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 6c22b43 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/8/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/8/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/8/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/8/testReport/ |
   | Max. process+thread count | 15295 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/8/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-833795430


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  15m 41s |  master passed  |
   | +0 |  hbaserecompile  |  22m 31s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  1s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   3m  6s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 52s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m  7s |  phoenix-core in master has 959 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   8m 13s |  the patch passed  |
   | +0 |  hbaserecompile  |  18m 14s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  2s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  2s |  the patch passed  |
   | -1 :x: |  checkstyle  |   3m 29s |  phoenix-core: The patch generated 723 new + 6054 unchanged - 107 fixed = 6777 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  1s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 49s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 12s |  phoenix-core generated 6 new + 959 unchanged - 0 fixed = 965 total (was 959)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 113m  6s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 46s |  The patch does not generate ASF License warnings.  |
   |  |   | 171m 58s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  org.apache.phoenix.monitoring.NoOpTableMetricsManager.noOpsTableMetricManager isn't final but should be  At NoOpTableMetricsManager.java:be  At NoOpTableMetricsManager.java:[line 25] |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 87] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 56] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 68] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 60] |
   | Failed junit tests | phoenix.end2end.PermissionNSEnabledWithCustomAccessControllerIT |
   |   | phoenix.end2end.AuditLoggingIT |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux 6e26c388b449 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 5265164 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/2/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/2/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/2/testReport/ |
   | Max. process+thread count | 13827 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/2/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629746729



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java
##########
@@ -0,0 +1,289 @@
+/**
+ * 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.monitoring;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.phoenix.query.QueryServicesOptions;
+import org.apache.phoenix.thirdparty.com.google.common.base.Strings;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Central place where we keep track of all the Table Level metrics. Register each tableMetrics and
+ * store the instance of it associated with TableName in a map
+ * This class exposes following functions as static methods to help catch all execptions
+ * 1.clearTableLevelMetricsMethod
+ * 2.getTableMetricsMethod
+ * 3.pushMetricsFromConnInstanceMethod
+ * 4.updateMetricsMethod
+ */
+
+public class TableMetricsManager {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(TableMetricsManager.class);
+    private static final Set<String> allowedListOfTableNames = new HashSet<>();
+    private static volatile boolean isTableLevelMetricsEnabled;
+    private static volatile boolean isMetricPublisherEnabled;
+    private static volatile ConcurrentMap<String, TableClientMetrics> tableClientMetricsMapping = null;
+    // Singleton object
+    private static volatile TableMetricsManager tableMetricsManager = null;
+    private static volatile MetricPublisherSupplierFactory mPublisher = null;
+    private static volatile QueryServicesOptions options = null;
+
+    public TableMetricsManager(QueryServicesOptions ops) {
+        options = ops;
+        isTableLevelMetricsEnabled = options.isTableLevelMetricsEnabled();
+        LOGGER.info(String.format("Phoenix Table metrics enabled status: %s",
+                isTableLevelMetricsEnabled));
+        tableClientMetricsMapping = new ConcurrentHashMap<>();
+
+        String tableNamesList = options.getAllowedListTableNames();
+        if (tableNamesList != null && !tableNamesList.isEmpty()) {
+            for (String tableName : tableNamesList.split(",")) {
+                allowedListOfTableNames.add(tableName);
+            }
+        }
+        isMetricPublisherEnabled = options.isMetricPublisherEnabled();
+        LOGGER.info(String.format("Phoenix table level metrics publisher enabled status %s",
+                isMetricPublisherEnabled));
+    }
+
+    public TableMetricsManager() {
+
+    }
+
+    /**
+     * Method to provide instance of TableMetricsManager(Create if needed in thread safe manner)
+     *
+     * @return
+     */
+    private static TableMetricsManager getInstance() {
+
+        TableMetricsManager localRef = tableMetricsManager;
+        if (localRef == null) {
+            synchronized (TableMetricsManager.class) {
+                if (localRef == null) {
+                    QueryServicesOptions options = QueryServicesOptions.withDefaults();
+                    if (!options.isTableLevelMetricsEnabled()) {
+                        localRef = tableMetricsManager =
+                                NoOpTableMetricsManager.noOpsTableMetricManager;
+                        return localRef;
+                    }
+                    localRef = tableMetricsManager = new TableMetricsManager(options);
+                    LOGGER.info("Phoenix Table metrics created object for metrics manager");
+                    if (isMetricPublisherEnabled) {
+                        String className = options.getMetricPublisherClass();
+                        if (className != null) {
+                            MetricServiceResolver mResolver = new MetricServiceResolver();
+                            LOGGER.info(String.format(
+                                    "Phoenix table level metrics publisher className %s",
+                                    className));
+                            try {
+                                mPublisher = mResolver.instantiate(className);
+                                mPublisher.registerMetricProvider();
+                            } catch (Throwable e) {
+                                LOGGER.error("The exception from metric publish Function", e);
+                            }
+
+                        } else {
+                            LOGGER.error(
+                                    "Phoenix table level metrics publisher className cannot be null");
+                        }
+
+                    }
+                }
+            }
+        }
+        return localRef;
+    }
+
+   public static void setInstance(TableMetricsManager metricsManager) {

Review comment:
       @VisibleForTesting has been added on 4.x branch but not here




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-834819371


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 34s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  16m 31s |  master passed  |
   | +0 |  hbaserecompile  |  24m 11s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  8s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   2m 44s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 55s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m 27s |  phoenix-core in master has 959 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   9m 45s |  the patch passed  |
   | +0 |  hbaserecompile  |  20m 15s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m 11s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 11s |  the patch passed  |
   | -1 :x: |  checkstyle  |   3m  7s |  phoenix-core: The patch generated 690 new + 6087 unchanged - 74 fixed = 6777 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 55s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 34s |  phoenix-core generated 6 new + 959 unchanged - 0 fixed = 965 total (was 959)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 112m 16s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 46s |  The patch does not generate ASF License warnings.  |
   |  |   | 175m  1s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  org.apache.phoenix.monitoring.NoOpTableMetricsManager.noOpsTableMetricManager isn't final but should be  At NoOpTableMetricsManager.java:be  At NoOpTableMetricsManager.java:[line 25] |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 87] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 56] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 68] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 60] |
   | Failed junit tests | phoenix.end2end.PermissionNSEnabledWithCustomAccessControllerIT |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux 3134b10f3b05 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 5265164 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/3/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/3/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/3/testReport/ |
   | Max. process+thread count | 15887 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/3/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-839525714


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  16m 46s |  master passed  |
   | +0 |  hbaserecompile  |  23m  9s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  2s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   3m  6s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m  5s |  phoenix-core in master has 959 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   8m  6s |  the patch passed  |
   | +0 |  hbaserecompile  |  17m 31s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  4s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  4s |  the patch passed  |
   | -1 :x: |  checkstyle  |   3m 19s |  phoenix-core: The patch generated 694 new + 6087 unchanged - 74 fixed = 6781 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  1s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 52s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 18s |  phoenix-core generated 5 new + 959 unchanged - 0 fixed = 964 total (was 959)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 114m 14s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 49s |  The patch does not generate ASF License warnings.  |
   |  |   | 173m 38s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 88] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 69] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 58] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 61] |
   | Failed junit tests | phoenix.end2end.BackwardCompatibilityIT |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/9/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux b03a595dfbf2 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 6c22b43 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/9/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/9/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/9/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/9/testReport/ |
   | Max. process+thread count | 14460 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/9/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] vmeka2020 commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
vmeka2020 commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-833260828


   @yanxinyi 


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629742621



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/JmxMetricProvider.java
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.monitoring;
+
+import org.apache.hadoop.hbase.metrics.Gauge;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.metrics.MetricRegistry;
+import org.apache.hadoop.hbase.metrics.MetricRegistryInfo;
+
+import java.util.Map;
+
+public class JmxMetricProvider implements MetricPublisherSupplierFactory {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(JmxMetricProvider.class);
+    private MetricRegistry metricRegistry;
+
+    @Override public void registerMetricProvider() {
+        metricRegistry = createMetricRegistry();
+        GlobalMetricRegistriesAdapter.getInstance().registerMetricRegistry(metricRegistry);
+    }
+
+    @Override public void unregisterMetricProvider() {
+
+    }
+
+    private  MetricRegistry createMetricRegistry() {
+        LOGGER.info("Creating Metric Registry for Phoenix Table Level Metrics");
+        MetricRegistryInfo
+                registryInfo =
+                new MetricRegistryInfo("PHOENIX-TableLevel", "Phoenix Client Metrics",
+                        "phoenixTableLevel", "Phoenix,sub=CLIENT", true);
+        return MetricRegistries.global().create(registryInfo);
+    }
+
+    private static class PhoenixMetricGauge implements Gauge<Long> {
+        private final PhoenixTableMetric metric;
+
+        public PhoenixMetricGauge(PhoenixTableMetric metric) {
+            this.metric = metric;
+        }
+
+        @Override public Long getValue() {
+            return metric.getValue();
+        }
+    }
+
+    private String getMetricNameFromMetricType(MetricType type, String tableName) {
+        return tableName + "_table_" + type;
+    }
+
+    @Override public void registerMetrics(TableClientMetrics tInstance) {
+        for (Map.Entry<MetricType, PhoenixTableMetric> entry : tInstance.getMetricRegistry()
+                .entrySet()) {
+            metricRegistry
+                    .register(getMetricNameFromMetricType(entry.getKey(), tInstance.getTableName()),
+                            new PhoenixMetricGauge(entry.getValue()));
+        }
+    }
+
+    @Override public void unRegisterMetrics(TableClientMetrics tInstance) {
+

Review comment:
       the 4.x branch has a logic as following:
   
   for (Map.Entry<MetricType, PhoenixTableMetric> entry : tInstance.getMetricRegistry()
                   .entrySet()) {
               metricRegistry
                       .remove(getMetricNameFromMetricType(entry.getKey(), tInstance.getTableName()));
           }
   
   
   why you didn't have it on the master?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629745734



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/NoOpTableMetricsManager.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.monitoring;
+
+import java.util.List;
+import java.util.Map;
+

Review comment:
       missing the class level comments
   
   /**
    * TableMetricsManager will be replaced by this case
    * incase of tableMetrics flag is set to  false.
    */
   
   




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-837306423


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  16m 29s |  master passed  |
   | +0 |  hbaserecompile  |  22m 54s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  6s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   3m  2s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m  4s |  phoenix-core in master has 959 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   8m 13s |  the patch passed  |
   | +0 |  hbaserecompile  |  18m  6s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  3s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  3s |  the patch passed  |
   | -1 :x: |  checkstyle  |   3m 26s |  phoenix-core: The patch generated 690 new + 6087 unchanged - 74 fixed = 6777 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 23s |  phoenix-core generated 6 new + 959 unchanged - 0 fixed = 965 total (was 959)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 114m  6s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 47s |  The patch does not generate ASF License warnings.  |
   |  |   | 173m 30s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  org.apache.phoenix.monitoring.NoOpTableMetricsManager.noOpsTableMetricManager isn't final but should be  At NoOpTableMetricsManager.java:be  At NoOpTableMetricsManager.java:[line 25] |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 87] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 56] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 68] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 60] |
   | Failed junit tests | phoenix.end2end.PermissionNSEnabledWithCustomAccessControllerIT |
   |   | phoenix.end2end.AuditLoggingIT |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/6/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux a7b49767605b 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 88a0128 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/6/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/6/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/6/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/6/testReport/ |
   | Max. process+thread count | 15417 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/6/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629747101



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/QueryServicesOptions.java
##########
@@ -684,6 +692,33 @@ public boolean isGlobalMetricsEnabled() {
         return config.getBoolean(GLOBAL_METRICS_ENABLED, DEFAULT_IS_GLOBAL_METRICS_ENABLED);
     }
 
+    public String getMetricPublisherClass() {
+        return config.get(METRIC_PUBLISHER_CLASS_NAME, DEFAULT_METRIC_PUBLISHER_CLASS_NAME);
+    }
+
+    public String getAllowedListTableNames() {
+        return config.get(ALLOWED_LIST_FOR_TABLE_LEVEL_METRICS,
+                DEFAULT_ALLOWED_LIST_FOR_TABLE_LEVEL_METRICS);
+    }
+
+    public boolean isTableLevelMetricsEnabled() {
+        return config
+                .getBoolean(TABLE_LEVEL_METRICS_ENABLED, DEFAULT_IS_TABLE_LEVEL_METRICS_ENABLED);
+    }
+
+    public void setTableLevelMetricsEnabled() {
+        set(TABLE_LEVEL_METRICS_ENABLED, true);
+    }
+
+    public boolean isMetricPublisherEnabled() {
+        return config.getBoolean(METRIC_PUBLISHER_ENABLED, DEFAULT_IS_METRIC_PUBLISHER_ENABLED);
+    }
+
+    public void setAllowedListForTableLevelMetrics(String tableNameList){

Review comment:
       VisibleForTesting tag has been added on 4.x branch but not here 




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-835167837


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  16m 44s |  master passed  |
   | +0 |  hbaserecompile  |  23m 34s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  3s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   3m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m  7s |  phoenix-core in master has 959 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   8m 15s |  the patch passed  |
   | +0 |  hbaserecompile  |  18m 15s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  3s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  3s |  the patch passed  |
   | -1 :x: |  checkstyle  |   3m 27s |  phoenix-core: The patch generated 690 new + 6087 unchanged - 74 fixed = 6777 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  1s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 19s |  phoenix-core generated 6 new + 959 unchanged - 0 fixed = 965 total (was 959)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 113m 28s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   1m 15s |  The patch does not generate ASF License warnings.  |
   |  |   | 175m 42s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  org.apache.phoenix.monitoring.NoOpTableMetricsManager.noOpsTableMetricManager isn't final but should be  At NoOpTableMetricsManager.java:be  At NoOpTableMetricsManager.java:[line 25] |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 87] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 56] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 68] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 60] |
   | Failed junit tests | phoenix.end2end.PermissionNSEnabledWithCustomAccessControllerIT |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux 0aeb069af3e5 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 3dcf95d |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/4/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/4/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/4/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/4/testReport/ |
   | Max. process+thread count | 14769 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/4/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629748118



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableClientMetricsTest.java
##########
@@ -0,0 +1,219 @@
+/**
+ * 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.
+ */
+

Review comment:
       missing the class level comments 
   
   /**
    * This test does UT for TableClientMetrics class
    * This class has following API
    * 1. changeMetricValue
    * 2.getTableName
    * 3.getMetricMap
    */
   




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629750681



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java
##########
@@ -0,0 +1,212 @@
+/**
+ * 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.monitoring;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
+import org.apache.phoenix.query.QueryServices;

Review comment:
       unused import 




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-837031468


   @vmeka2020 can you please rebase one more time, 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629747889



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java
##########
@@ -1367,6 +1369,22 @@ private static Expression getFirstPKColumnExpression(PTable table) throws SQLExc
     public static Collection<GlobalMetric> getGlobalPhoenixClientMetrics() {
         return GlobalClientMetrics.getMetrics();
     }
+
+    /**
+     * This function will be called mainly in Metric Publisher methods.
+     * Its the only way Metric publisher will be able to access the metrics in phoenix system.
+     * @return map of TableName to List of GlobalMetric's.
+     */
+    public static Map<String,List<PhoenixTableMetric>> getPhoenixTableClientMetrics() {
+        return TableMetricsManager.getTableMetricsMethod();
+    }
+
+    /**
+     * This is only used in testcases to reset the tableLevel Metrics data
+     */
+    public static void clearTableLevelMetrics(){

Review comment:
       VisibleForTesting has been added on 4.x but not here




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-836144670


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m  0s |  Docker mode activated.  |
   | -1 :x: |  patch  |   0m  4s |  https://github.com/apache/phoenix/pull/1224 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help.  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/5/console |
   | versions | git=2.17.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629735488



##########
File path: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java
##########
@@ -0,0 +1,1239 @@
+/*
+ * 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.monitoring;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.client.RetriesExhaustedWithDetailsException;
+import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
+import org.apache.phoenix.end2end.BaseUniqueNamesOwnClusterIT;
+import org.apache.phoenix.exception.PhoenixIOException;
+import org.apache.phoenix.exception.SQLExceptionInfo;
+import org.apache.phoenix.execute.CommitException;
+import org.apache.phoenix.jdbc.PhoenixConnection;
+import org.apache.phoenix.jdbc.PhoenixEmbeddedDriver;
+import org.apache.phoenix.jdbc.PhoenixTestDriver;
+import org.apache.phoenix.query.BaseTest;
+import org.apache.phoenix.query.ConfigurationFactory;
+import org.apache.phoenix.query.ConnectionQueryServices;
+import org.apache.phoenix.query.ConnectionQueryServicesImpl;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.query.QueryServicesTestImpl;
+import org.apache.phoenix.util.EnvironmentEdge;
+import org.apache.phoenix.util.EnvironmentEdgeManager;
+import org.apache.phoenix.util.InstanceResolver;
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.apache.phoenix.util.DelayedOrFailingRegionServer;
+import org.apache.phoenix.util.ReadOnlyProps;
+
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.phoenix.exception.SQLExceptionCode.DATA_EXCEEDS_MAX_CAPACITY;
+import static org.apache.phoenix.exception.SQLExceptionCode.GET_TABLE_REGIONS_FAIL;
+import static org.apache.phoenix.exception.SQLExceptionCode.OPERATION_TIMED_OUT;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_AGGREGATE_FAILURE_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_AGGREGATE_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_BATCH_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_BATCH_FAILED_SIZE;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_COMMIT_TIME;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_MUTATION_BYTES;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_MUTATION_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_SQL_QUERY_TIME;
+import static org.apache.phoenix.monitoring.MetricType.DELETE_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.MUTATION_BATCH_FAILED_SIZE;
+import static org.apache.phoenix.monitoring.MetricType.MUTATION_BATCH_SIZE;
+import static org.apache.phoenix.monitoring.MetricType.MUTATION_BYTES;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_POINTLOOKUP_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_POINTLOOKUP_TIMEOUT_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_SCAN_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_SCAN_TIMEOUT_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.QUERY_TIMEOUT_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.RESULT_SET_TIME_MS;
+import static org.apache.phoenix.monitoring.MetricType.SCAN_BYTES;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_AGGREGATE_FAILURE_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_AGGREGATE_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_POINTLOOKUP_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_POINTLOOKUP_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SCAN_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SCAN_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SQL_QUERY_TIME;
+import static org.apache.phoenix.monitoring.MetricType.SELECT_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_AGGREGATE_FAILURE_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_AGGREGATE_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_BATCH_FAILED_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_BATCH_FAILED_SIZE;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_COMMIT_TIME;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_FAILED_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_MUTATION_BYTES;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_MUTATION_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_SQL_QUERY_TIME;
+import static org.apache.phoenix.monitoring.MetricType.UPSERT_SUCCESS_SQL_COUNTER;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.POINT_LOOKUP_SELECT_QUERY;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.RANGE_SCAN_SELECT_QUERY;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.createTableAndInsertValues;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.doPointDeleteFromTable;
+import static org.apache.phoenix.monitoring.PhoenixMetricsIT.doDeleteAllFromTable;
+import static org.apache.phoenix.util.DelayedOrFailingRegionServer.INJECTED_EXCEPTION_STRING;
+import static org.apache.phoenix.util.PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR;
+import static org.apache.phoenix.util.PhoenixRuntime.clearTableLevelMetrics;
+import static org.apache.phoenix.util.PhoenixRuntime.getOverAllReadRequestMetricInfo;
+import static org.apache.phoenix.util.PhoenixRuntime.getPhoenixTableClientMetrics;
+import static org.apache.phoenix.util.PhoenixRuntime.getRequestReadMetricInfo;
+import static org.apache.phoenix.util.PhoenixRuntime.getWriteMetricInfoForMutationsSinceLastReset;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicLong;
+
+public class PhoenixTableLevelMetricsIT extends BaseUniqueNamesOwnClusterIT {

Review comment:
       @Category(NeedsOwnMiniClusterTest.class) is missing on the master branch compares to the 4.x




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629745504



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/MutationMetricQueue.java
##########
@@ -119,14 +178,57 @@ public CombinableMetric getNumOfIndexCommitFailedMutations() {
             return numOfIndexCommitFailMutations;
         }
 
+        public CombinableMetric getUpsertMutationsSizeBytes() {
+            return upsertMutationsSizeBytes;
+        }
+
+        public CombinableMetric getDeleteMutationsSizeBytes() {
+            return deleteMutationsSizeBytes;
+        }
+
+        public CombinableMetric getUpsertMutationSqlCounterSuccess() {
+            return upsertMutationSqlCounterSuccess;
+        }
+
+        public CombinableMetric getDeleteMutationSqlCounterSuccess() {
+            return deleteMutationSqlCounterSuccess;
+        }
+
+        public CombinableMetric getUpsertBatchFailedSize() {
+            return upsertBatchFailedSize;
+        }
+
+        public CombinableMetric getUpsertBatchFailedCounter() {
+            return upsertBatchFailedCounter;
+        }
+
+        public CombinableMetric getDeleteBatchFailedSize() {
+            return deleteBatchFailedSize;
+        }
+
+        public CombinableMetric getDeleteBatchFailedCounter() {
+            return deleteBatchFailedCounter;
+        }
+
         public void combineMetric(MutationMetric other) {
             this.numMutations.combine(other.numMutations);
-            this.mutationsSizeBytes.combine(other.mutationsSizeBytes);
+            this.totalCommitTimeForUpserts.combine(other.totalCommitTimeForUpserts);
+            this.totalCommitTimeForDeletes.combine(other.totalCommitTimeForDeletes);
             this.totalCommitTimeForMutations.combine(other.totalCommitTimeForMutations);
             this.numFailedMutations.combine(other.numFailedMutations);
             this.numOfIndexCommitFailMutations.combine(other.numOfIndexCommitFailMutations);
+            this.upsertMutationsSizeBytes.combine(other.upsertMutationsSizeBytes);
+            this.deleteMutationsSizeBytes.combine(other.deleteMutationsSizeBytes);
+            this.mutationsSizeBytes.combine(other.mutationsSizeBytes);
+            this.upsertMutationSqlCounterSuccess.combine(other.upsertMutationSqlCounterSuccess);
+            this.deleteMutationSqlCounterSuccess.combine(other.deleteMutationSqlCounterSuccess);
+            this.upsertBatchFailedSize.combine(other.upsertBatchFailedSize);
+            this.upsertBatchFailedCounter.combine(other.upsertBatchFailedCounter);
+            this.deleteBatchFailedSize.combine(other.deleteBatchFailedSize);
+            this.deleteBatchFailedCounter.combine(other.deleteBatchFailedCounter);
         }
 
+

Review comment:
       nit:extra line




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-833366780


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   4m 18s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  16m 14s |  master passed  |
   | +0 |  hbaserecompile  |  23m  5s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  4s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   3m  6s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 53s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m  5s |  phoenix-core in master has 959 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   8m  6s |  the patch passed  |
   | +0 |  hbaserecompile  |  17m 45s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  2s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  2s |  the patch passed  |
   | -1 :x: |  checkstyle  |   3m 27s |  phoenix-core: The patch generated 690 new + 6087 unchanged - 74 fixed = 6777 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  1s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 18s |  phoenix-core generated 6 new + 959 unchanged - 0 fixed = 965 total (was 959)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 111m 38s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 45s |  The patch does not generate ASF License warnings.  |
   |  |   | 174m 34s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  org.apache.phoenix.monitoring.NoOpTableMetricsManager.noOpsTableMetricManager isn't final but should be  At NoOpTableMetricsManager.java:be  At NoOpTableMetricsManager.java:[line 25] |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 87] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 56] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 68] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 60] |
   | Failed junit tests | phoenix.end2end.PermissionNSEnabledWithCustomAccessControllerIT |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux 8bc81b8daec8 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 5265164 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/1/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/1/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/1/testReport/ |
   | Max. process+thread count | 14526 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/1/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] stoty commented on pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#issuecomment-840008005


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 40s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  17m  8s |  master passed  |
   | +0 |  hbaserecompile  |  24m 17s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  4s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   2m 42s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  master passed  |
   | +0 :ok: |  spotbugs  |   3m  9s |  phoenix-core in master has 959 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   8m 11s |  the patch passed  |
   | +0 |  hbaserecompile  |  18m 48s |  HBase recompiled.  |
   | +1 :green_heart: |  compile  |   1m  7s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  7s |  the patch passed  |
   | -1 :x: |  checkstyle  |   2m 55s |  phoenix-core: The patch generated 694 new + 6087 unchanged - 74 fixed = 6781 total (was 6161)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  1s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  the patch passed  |
   | -1 :x: |  spotbugs  |   3m 39s |  phoenix-core generated 5 new + 959 unchanged - 0 fixed = 964 total (was 959)  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 113m 26s |  phoenix-core in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 45s |  The patch does not generate ASF License warnings.  |
   |  |   | 174m 34s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:phoenix-core |
   |  |  Load of known null value in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:in org.apache.phoenix.monitoring.TableMetricsManager.getInstance()  At TableMetricsManager.java:[line 88] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.options from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 57] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isMetricPublisherEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 69] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.isTableLevelMetricsEnabled from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 58] |
   |  |  Write to static field org.apache.phoenix.monitoring.TableMetricsManager.tableClientMetricsMapping from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:from instance method new org.apache.phoenix.monitoring.TableMetricsManager(QueryServicesOptions)  At TableMetricsManager.java:[line 61] |
   | Failed junit tests | phoenix.end2end.PermissionNSEnabledWithCustomAccessControllerIT |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/10/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/phoenix/pull/1224 |
   | Optional Tests | dupname asflicense javac javadoc unit xml compile spotbugs hbaserebuild hbaseanti checkstyle |
   | uname | Linux 4f3a0c5839f8 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev/phoenix-personality.sh |
   | git revision | master / 6c22b43 |
   | Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
   | checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/10/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
   | spotbugs | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/10/artifact/yetus-general-check/output/new-spotbugs-phoenix-core.html |
   | unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/10/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/10/testReport/ |
   | Max. process+thread count | 15826 (vs. ulimit of 30000) |
   | modules | C: phoenix-core U: phoenix-core |
   | Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1224/10/console |
   | versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629751033



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java
##########
@@ -0,0 +1,212 @@
+/**
+ * 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.monitoring;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.query.QueryServicesOptions;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import org.mockito.Mockito;

Review comment:
       unused import
   
   




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r629751816



##########
File path: phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java
##########
@@ -0,0 +1,212 @@
+/**
+ * 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.monitoring;
+
+import org.apache.hadoop.conf.Configuration;

Review comment:
       unused import
   
   




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] yanxinyi commented on a change in pull request #1224: PHOENIX-6397 Implement TableMetricsManager class and its associated functions for select. upsert and Delete Queries

Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #1224:
URL: https://github.com/apache/phoenix/pull/1224#discussion_r631458168



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/JmxMetricProvider.java
##########
@@ -0,0 +1,90 @@
+/*
+ * 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.monitoring;
+
+import org.apache.hadoop.hbase.metrics.Gauge;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.metrics.MetricRegistry;
+import org.apache.hadoop.hbase.metrics.MetricRegistryInfo;
+
+import java.util.Map;
+
+/**
+ * This class implements the JMX based default Metric publishing
+ * of Metrics to JMX end point.
+ * This class is defined in phoenix/phoenix-core/src/main/resources/META-INF/services/org.apache.phoenix.monitoring.MetricPublisherSupplierFactory
+ */
+public class JmxMetricProvider implements MetricPublisherSupplierFactory {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(JmxMetricProvider.class);
+    private MetricRegistry metricRegistry;
+
+    @Override public void registerMetricProvider() {
+        metricRegistry = createMetricRegistry();
+        GlobalMetricRegistriesAdapter.getInstance().registerMetricRegistry(metricRegistry);
+    }
+
+    @Override public void unregisterMetricProvider() {
+
+    }
+
+    private  MetricRegistry createMetricRegistry() {
+        LOGGER.info("Creating Metric Registry for Phoenix Table Level Metrics");
+        MetricRegistryInfo
+                registryInfo =
+                new MetricRegistryInfo("PHOENIX-TableLevel", "Phoenix Client Metrics",
+                        "phoenixTableLevel", "Phoenix,sub=CLIENT", true);

Review comment:
       please do not hardcode the string




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org