PHOENIX-4870 LoggingPhoenixConnection should log metrics when AutoCommit is set to True.
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/6ea2110b Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/6ea2110b Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/6ea2110b Branch: refs/heads/4.x-cdh5.15 Commit: 6ea2110bacd97943b17dbbe4484bf8a6da9dde7a Parents: bb297e7 Author: s.kadam <s.ka...@gus.com> Authored: Thu Sep 6 01:00:03 2018 +0100 Committer: Pedro Boado <pbo...@apache.org> Committed: Wed Oct 17 22:49:38 2018 +0100 ---------------------------------------------------------------------- .../monitoring/PhoenixLoggingMetricsIT.java | 61 +++++++++++++++++++- .../phoenix/jdbc/LoggingPhoenixConnection.java | 37 +++++++----- .../jdbc/LoggingPhoenixPreparedStatement.java | 25 +++++++- .../phoenix/jdbc/LoggingPhoenixStatement.java | 28 +++++++-- 4 files changed, 125 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/6ea2110b/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 5d5524c..483d341 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 @@ -102,7 +102,8 @@ public class PhoenixLoggingMetricsIT extends BasePhoenixMetricsIT { public void testPhoenixMetricsLoggedOnCommit() throws Exception { // run SELECT to verify read metrics are logged String query = "SELECT * FROM " + tableName1; - verifyQueryLevelMetricsLogging(query); + ResultSet rs = upsertRows(query); + verifyQueryLevelMetricsLogging(query, rs); // run UPSERT SELECT to verify mutation metrics are logged String upsertSelect = "UPSERT INTO " + tableName2 + " SELECT * FROM " + tableName1; @@ -140,7 +141,9 @@ public class PhoenixLoggingMetricsIT extends BasePhoenixMetricsIT { public void testPhoenixMetricsLoggedOnClose() throws Exception { // run SELECT to verify read metrics are logged String query = "SELECT * FROM " + tableName1; - verifyQueryLevelMetricsLogging(query); + + ResultSet rs = upsertRows(query); + verifyQueryLevelMetricsLogging(query, rs); // run UPSERT SELECT to verify mutation metrics are logged String upsertSelect = "UPSERT INTO " + tableName2 + " SELECT * FROM " + tableName1; @@ -164,13 +167,61 @@ public class PhoenixLoggingMetricsIT extends BasePhoenixMetricsIT { mutationReadMetricsMap.size() == 0); } + /** + * This test is added to verify if metrics are being logged in case + * auto commit is set to true. + */ + @Test + public void testPhoenixMetricsLoggedOnAutoCommitTrue() throws Exception { + loggedConn.setAutoCommit(true); + + String query = "SELECT * FROM " + tableName1; + ResultSet rs = upsertRows(query); + verifyQueryLevelMetricsLogging(query, rs); + + // run UPSERT SELECT to verify mutation metrics are logged + String upsertSelect = "UPSERT INTO " + tableName2 + " SELECT * FROM " + tableName1; + loggedConn.createStatement().executeUpdate(upsertSelect); + + assertTrue("Mutation write metrics are not logged for " + tableName2, + mutationWriteMetricsMap.get(tableName2).size() > 0); + assertTrue("Mutation read metrics are not found for " + tableName1, + mutationReadMetricsMap.get(tableName1).size() > 0); + + clearAllTestMetricMaps(); + + loggedConn.createStatement().execute(query); + assertTrue("Read metrics found for " + tableName1, + mutationReadMetricsMap.size() == 0); + loggedConn.createStatement().execute(upsertSelect); + + assertTrue("Mutation write metrics are not logged for " + tableName2 + + " in createStatement",mutationWriteMetricsMap.get(tableName2).size() > 0); + assertTrue("Mutation read metrics are not found for " + tableName1 + + " in createStatement",mutationReadMetricsMap.get(tableName1).size() > 0); + + clearAllTestMetricMaps(); + + loggedConn.prepareStatement(query).executeQuery(); + assertTrue("Read metrics found for " + tableName1, + mutationReadMetricsMap.size() == 0); + + loggedConn.prepareStatement(upsertSelect).executeUpdate(); + assertTrue("Mutation write metrics are not logged for " + tableName2 + + " in prepareStatement",mutationWriteMetricsMap.get(tableName2).size() > 0); + assertTrue("Mutation read metrics are not found for " + tableName1 + + " in prepareStatement",mutationReadMetricsMap.get(tableName1).size() > 0); + + + } + private ResultSet executeAndGetResultSet(String query) throws Exception { Statement stmt = loggedConn.createStatement(); stmt.execute(query); return stmt.getResultSet(); } - private void verifyQueryLevelMetricsLogging(String query) throws SQLException { + private ResultSet upsertRows(String query) throws SQLException { Statement stmt = loggedConn.createStatement(); ResultSet rs = stmt.executeQuery(query); assertTrue(rs instanceof LoggingPhoenixResultSet); @@ -180,6 +231,10 @@ public class PhoenixLoggingMetricsIT extends BasePhoenixMetricsIT { } rs.close(); assertTrue(rowsRetrievedCounter == NUM_ROWS); + return rs; + } + + private void verifyQueryLevelMetricsLogging(String query , ResultSet rs) throws SQLException { assertTrue("Read metrics for not found for " + tableName1, requestReadMetricsMap.get(tableName1).size() > 0); assertTrue("Logged query doesn't match actual query", loggedSql.equals(query)); http://git-wip-us.apache.org/repos/asf/phoenix/blob/6ea2110b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixConnection.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixConnection.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixConnection.java index 37917e2..af0f803 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixConnection.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixConnection.java @@ -40,14 +40,16 @@ public class LoggingPhoenixConnection extends DelegateConnection { @Override public Statement createStatement() throws SQLException { - return new LoggingPhoenixStatement(super.createStatement(), phoenixMetricsLog); + return new LoggingPhoenixStatement(super.createStatement(), phoenixMetricsLog, + this); } @Override public Statement createStatement(int resultSetType, int resultSetConcurrency) throws SQLException { return new LoggingPhoenixStatement( - super.createStatement(resultSetType, resultSetConcurrency), phoenixMetricsLog); + super.createStatement(resultSetType, resultSetConcurrency), phoenixMetricsLog, + this); } @Override @@ -55,13 +57,13 @@ public class LoggingPhoenixConnection extends DelegateConnection { int resultSetHoldability) throws SQLException { return new LoggingPhoenixStatement( super.createStatement(resultSetType, resultSetConcurrency, resultSetHoldability), - phoenixMetricsLog); + phoenixMetricsLog, this); } @Override public PreparedStatement prepareStatement(String sql) throws SQLException { return new LoggingPhoenixPreparedStatement(super.prepareStatement(sql), - phoenixMetricsLog, sql); + phoenixMetricsLog, sql, this); } @Override @@ -69,52 +71,57 @@ public class LoggingPhoenixConnection extends DelegateConnection { int resultSetConcurrency) throws SQLException { return new LoggingPhoenixPreparedStatement( super.prepareStatement(sql, resultSetType, resultSetConcurrency), - phoenixMetricsLog, sql); + phoenixMetricsLog, sql, this); } @Override public PreparedStatement prepareStatement(String sql, int resultSetType, int resultSetConcurrency, int resultSetHoldability) throws SQLException { return new LoggingPhoenixPreparedStatement(super.prepareStatement(sql, resultSetType, - resultSetConcurrency, resultSetHoldability), phoenixMetricsLog, sql); + resultSetConcurrency, resultSetHoldability), phoenixMetricsLog, sql, this); } @Override public PreparedStatement prepareStatement(String sql, int autoGeneratedKeys) throws SQLException { return new LoggingPhoenixPreparedStatement(super.prepareStatement(sql, autoGeneratedKeys), - phoenixMetricsLog, sql); + phoenixMetricsLog, sql, this); } @Override public PreparedStatement prepareStatement(String sql, int[] columnIndexes) throws SQLException { return new LoggingPhoenixPreparedStatement(super.prepareStatement(sql, columnIndexes), - phoenixMetricsLog, sql); + phoenixMetricsLog, sql, this); } @Override public PreparedStatement prepareStatement(String sql, String[] columnNames) throws SQLException { return new LoggingPhoenixPreparedStatement(super.prepareStatement(sql, columnNames), - phoenixMetricsLog, sql); + phoenixMetricsLog, sql, this); } @Override public void commit() throws SQLException { super.commit(); - phoenixMetricsLog.logWriteMetricsfoForMutationsSinceLastReset(PhoenixRuntime.getWriteMetricInfoForMutationsSinceLastReset(conn)); - phoenixMetricsLog.logReadMetricInfoForMutationsSinceLastReset(PhoenixRuntime.getReadMetricInfoForMutationsSinceLastReset(conn)); - PhoenixRuntime.resetMetrics(conn); + loggingMetricsHelper(); } @Override public void close() throws SQLException { try { - phoenixMetricsLog.logWriteMetricsfoForMutationsSinceLastReset(PhoenixRuntime.getWriteMetricInfoForMutationsSinceLastReset(conn)); - phoenixMetricsLog.logReadMetricInfoForMutationsSinceLastReset(PhoenixRuntime.getReadMetricInfoForMutationsSinceLastReset(conn)); - PhoenixRuntime.resetMetrics(conn); + loggingMetricsHelper(); } finally { super.close(); } } + + public void loggingMetricsHelper() throws SQLException { + + phoenixMetricsLog.logWriteMetricsfoForMutationsSinceLastReset( + PhoenixRuntime.getWriteMetricInfoForMutationsSinceLastReset(conn)); + phoenixMetricsLog.logReadMetricInfoForMutationsSinceLastReset( + PhoenixRuntime.getReadMetricInfoForMutationsSinceLastReset(conn)); + PhoenixRuntime.resetMetrics(conn); + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/6ea2110b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixPreparedStatement.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixPreparedStatement.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixPreparedStatement.java index dbeea0d..12edde9 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixPreparedStatement.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixPreparedStatement.java @@ -21,16 +21,21 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.SQLFeatureNotSupportedException; +import java.sql.Connection; + public class LoggingPhoenixPreparedStatement extends DelegatePreparedStatement { private PhoenixMetricsLog phoenixMetricsLog; private String sql; + private Connection conn; - public LoggingPhoenixPreparedStatement(PreparedStatement stmt, PhoenixMetricsLog phoenixMetricsLog, String sql) { + public LoggingPhoenixPreparedStatement(PreparedStatement stmt, + PhoenixMetricsLog phoenixMetricsLog, String sql, Connection conn) { super(stmt); this.phoenixMetricsLog = phoenixMetricsLog; this.sql = sql; + this.conn = conn; } @Override @@ -40,7 +45,9 @@ public class LoggingPhoenixPreparedStatement extends DelegatePreparedStatement { @Override public ResultSet executeQuery() throws SQLException { - return new LoggingPhoenixResultSet(super.executeQuery(), phoenixMetricsLog, sql); + ResultSet rs = new LoggingPhoenixResultSet(super.executeQuery(), phoenixMetricsLog, sql); + this.loggingAutoCommitHelper(); + return rs; } @Override @@ -50,10 +57,22 @@ public class LoggingPhoenixPreparedStatement extends DelegatePreparedStatement { return (resultSet == null) ? null : new LoggingPhoenixResultSet(resultSet, phoenixMetricsLog, sql); } + + @Override + public int executeUpdate() throws SQLException { + int res = super.executeUpdate(); + this.loggingAutoCommitHelper(); + return res; + } @Override public ResultSet getGeneratedKeys() throws SQLException { return new LoggingPhoenixResultSet(super.getGeneratedKeys(), phoenixMetricsLog, sql); } - + + private void loggingAutoCommitHelper() throws SQLException { + if(conn.getAutoCommit() && (conn instanceof LoggingPhoenixConnection)) { + ((LoggingPhoenixConnection)conn).loggingMetricsHelper(); + } + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/6ea2110b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixStatement.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixStatement.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixStatement.java index de33893..d31f521 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixStatement.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/LoggingPhoenixStatement.java @@ -17,6 +17,8 @@ */ package org.apache.phoenix.jdbc; + +import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; @@ -25,28 +27,38 @@ public class LoggingPhoenixStatement extends DelegateStatement { private PhoenixMetricsLog phoenixMetricsLog; private String sql; - - public LoggingPhoenixStatement(Statement stmt, PhoenixMetricsLog phoenixMetricsLog) { + private Connection conn; + + public LoggingPhoenixStatement(Statement stmt, PhoenixMetricsLog phoenixMetricsLog, Connection conn) { super(stmt); this.phoenixMetricsLog = phoenixMetricsLog; + this.conn = conn; } @Override public boolean execute(String sql) throws SQLException { + boolean result; this.sql = sql; - return super.execute(sql); + result = super.execute(sql); + this.loggingAutoCommitHelper(); + return result; } @Override public ResultSet executeQuery(String sql) throws SQLException { this.sql = sql; - return new LoggingPhoenixResultSet(super.executeQuery(sql), phoenixMetricsLog, this.sql); + ResultSet rs = new LoggingPhoenixResultSet(super.executeQuery(sql), phoenixMetricsLog, this.sql); + this.loggingAutoCommitHelper(); + return rs; } @Override public int executeUpdate(String sql) throws SQLException { + int result; this.sql = sql; - return super.executeUpdate(sql); + result = super.executeUpdate(sql); + this.loggingAutoCommitHelper(); + return result; } @Override @@ -62,4 +74,10 @@ public class LoggingPhoenixStatement extends DelegateStatement { return new LoggingPhoenixResultSet(super.getGeneratedKeys(), phoenixMetricsLog, this.sql); } + private void loggingAutoCommitHelper() throws SQLException { + if(conn.getAutoCommit() && (conn instanceof LoggingPhoenixConnection)) { + ((LoggingPhoenixConnection)conn).loggingMetricsHelper(); + } + } + }