PHOENIX-4854 Make LoggingPhoenixResultSet idempotent when logging metrics

Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/08a3cf0d
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/08a3cf0d
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/08a3cf0d

Branch: refs/heads/4.x-cdh5.15
Commit: 08a3cf0d324cd8d5f1f8e8eadd13e1a30f5d96ff
Parents: 9d07afa
Author: Karan Mehta <karanmeht...@gmail.com>
Authored: Mon Aug 20 18:12:37 2018 +0100
Committer: Pedro Boado <pbo...@apache.org>
Committed: Wed Oct 17 22:49:38 2018 +0100

----------------------------------------------------------------------
 .../monitoring/PhoenixLoggingMetricsIT.java     | 49 +++++++++++---------
 .../phoenix/jdbc/LoggingPhoenixResultSet.java   | 15 ++++--
 2 files changed, 38 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/08a3cf0d/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixLoggingMetricsIT.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixLoggingMetricsIT.java
 
b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixLoggingMetricsIT.java
index 97b2c5d..7e56902 100644
--- 
a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixLoggingMetricsIT.java
+++ 
b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixLoggingMetricsIT.java
@@ -26,6 +26,7 @@ import org.junit.Test;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.Map;
 
@@ -44,6 +45,8 @@ public class PhoenixLoggingMetricsIT extends 
BasePhoenixMetricsIT {
     private String tableName2;
     private LoggingPhoenixConnection loggedConn;
     private String loggedSql;
+    private int logOverAllReadRequestMetricsFuncCallCount;
+    private int logRequestReadMetricsFuncCallCount;
 
     @Before
     public void beforeTest() throws Exception {
@@ -69,17 +72,7 @@ public class PhoenixLoggingMetricsIT extends 
BasePhoenixMetricsIT {
     public void testPhoenixMetricsLoggedOnCommit() throws Exception {
         // run SELECT to verify read metrics are logged
         String query = "SELECT * FROM " + tableName1;
-        Statement stmt = loggedConn.createStatement();
-        ResultSet rs = stmt.executeQuery(query);
-        while (rs.next()) {
-        }
-        rs.close();
-        assertTrue("Read metrics for not found for " + tableName1,
-                requestReadMetricsMap.get(tableName1).size() > 0);
-        assertTrue("Logged query doesn't match actual query", 
loggedSql.equals(query));
-
-        assertTrue("Overall read metrics for not found ", 
overAllQueryMetricsMap.size() > 0);
-        assertTrue("Logged query doesn't match actual query", 
loggedSql.equals(query));
+        verifyQueryLevelMetricsLogging(query);
 
         // run UPSERT SELECT to verify mutation metrics are logged
         String upsertSelect = "UPSERT INTO " + tableName2 + " SELECT * FROM " 
+ tableName1;
@@ -117,17 +110,7 @@ public class PhoenixLoggingMetricsIT extends 
BasePhoenixMetricsIT {
     public void testPhoenixMetricsLoggedOnClose() throws Exception {
         // run SELECT to verify read metrics are logged
         String query = "SELECT * FROM " + tableName1;
-        Statement stmt = loggedConn.createStatement();
-        ResultSet rs = stmt.executeQuery(query);
-        while (rs.next()) {
-        }
-        rs.close();
-        assertTrue("Read metrics for not found for " + tableName1,
-                requestReadMetricsMap.get(tableName1).size() > 0);
-        assertTrue("Logged query doesn't match actual query", 
loggedSql.equals(query));
-
-        assertTrue("Overall read metrics for not found ", 
overAllQueryMetricsMap.size() > 0);
-        assertTrue("Logged query doesn't match actual query", 
loggedSql.equals(query));
+        verifyQueryLevelMetricsLogging(query);
 
         // run UPSERT SELECT to verify mutation metrics are logged
         String upsertSelect = "UPSERT INTO " + tableName2 + " SELECT * FROM " 
+ tableName1;
@@ -151,6 +134,26 @@ public class PhoenixLoggingMetricsIT extends 
BasePhoenixMetricsIT {
                 mutationReadMetricsMap.size() == 0);
     }
 
+    private void verifyQueryLevelMetricsLogging(String query) throws 
SQLException {
+        Statement stmt = loggedConn.createStatement();
+        ResultSet rs = stmt.executeQuery(query);
+        while (rs.next()) {
+        }
+        rs.close();
+        assertTrue("Read metrics for not found for " + tableName1,
+                requestReadMetricsMap.get(tableName1).size() > 0);
+        assertTrue("Logged query doesn't match actual query", 
loggedSql.equals(query));
+        assertTrue(logOverAllReadRequestMetricsFuncCallCount == 1);
+        assertTrue(logRequestReadMetricsFuncCallCount == 1);
+
+        assertTrue("Overall read metrics for not found ", 
overAllQueryMetricsMap.size() > 0);
+        assertTrue("Logged query doesn't match actual query", 
loggedSql.equals(query));
+
+        rs.close();
+        assertTrue(logOverAllReadRequestMetricsFuncCallCount == 1);
+        assertTrue(logRequestReadMetricsFuncCallCount == 1);
+    }
+
     void clearAllTestMetricMaps() {
         overAllQueryMetricsMap.clear();
         requestReadMetricsMap.clear();
@@ -165,6 +168,7 @@ public class PhoenixLoggingMetricsIT extends 
BasePhoenixMetricsIT {
                     Map<MetricType, Long> overAllQueryMetrics, String sql) {
                 overAllQueryMetricsMap.putAll(overAllQueryMetrics);
                 loggedSql = sql;
+                logOverAllReadRequestMetricsFuncCallCount++;
             }
 
             @Override
@@ -172,6 +176,7 @@ public class PhoenixLoggingMetricsIT extends 
BasePhoenixMetricsIT {
                     Map<String, Map<MetricType, Long>> requestReadMetrics, 
String sql) {
                 requestReadMetricsMap.putAll(requestReadMetrics);
                 loggedSql = sql;
+                logRequestReadMetricsFuncCallCount++;
             }
 
             @Override

http://git-wip-us.apache.org/repos/asf/phoenix/blob/08a3cf0d/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixResultSet.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixResultSet.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixResultSet.java
index 53f5cb4..4ecde32 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixResultSet.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixResultSet.java
@@ -26,19 +26,26 @@ public class LoggingPhoenixResultSet extends 
DelegateResultSet {
     
     private PhoenixMetricsLog phoenixMetricsLog;
     private String sql;
+    private boolean areMetricsLogged;
 
     public LoggingPhoenixResultSet(ResultSet rs, PhoenixMetricsLog 
phoenixMetricsLog, String sql) {
         super(rs);
         this.phoenixMetricsLog = phoenixMetricsLog;
         this.sql = sql;
+        this.areMetricsLogged = false;
     }
     
     @Override
     public void close() throws SQLException {
-        
phoenixMetricsLog.logOverAllReadRequestMetrics(PhoenixRuntime.getOverAllReadRequestMetricInfo(rs),
 sql);
-        
phoenixMetricsLog.logRequestReadMetrics(PhoenixRuntime.getRequestReadMetricInfo(rs),
 sql);
-        PhoenixRuntime.resetMetrics(rs);
-        super.close();
+        if (!rs.isClosed()) {
+            super.close();
+        }
+        if (!this.areMetricsLogged) {
+            
phoenixMetricsLog.logOverAllReadRequestMetrics(PhoenixRuntime.getOverAllReadRequestMetricInfo(rs),
 sql);
+            
phoenixMetricsLog.logRequestReadMetrics(PhoenixRuntime.getRequestReadMetricInfo(rs),
 sql);
+            PhoenixRuntime.resetMetrics(rs);
+            this.areMetricsLogged = true;
+        }
     }
 
 }

Reply via email to