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; + } } }
