PHOENIX-7528 Add metrics for overall wall clock query wait time in Phoenix client thread pool and total time spent in executing HBase scan tasks #2077
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tkhurana
reviewed
Mar 15, 2025
| long vpk3 = 1000; | ||
| try(Connection conn = DriverManager.getConnection(getUrl())) { | ||
| PreparedStatement stmt = conn.prepareStatement(upsertRows); | ||
| stmt.execute(creatTableDdl); |
There was a problem hiding this comment.
This is a slightly weird pattern. Creating a table with a PreparedStatement for upsert rows. Generally, we just do conn.createStatement().execute(ddl)
tkhurana
reviewed
Mar 15, 2025
| SecurityException, IllegalArgumentException, IllegalAccessException { | ||
| private void changeInternalStateForTesting(PhoenixResultSet rs, | ||
| ReadMetricQueue testMetricsQueue) throws | ||
| NoSuchFieldException, SecurityException, IllegalArgumentException, |
There was a problem hiding this comment.
Maybe just change it to throws Exception
tkhurana
reviewed
Mar 15, 2025
| @@ -1298,5 +1301,225 @@ public Connection call() throws Exception { | |||
| } | |||
| } | |||
|
|
|||
| @Test | |||
| public void testPhoenixClientQueueWaitTimeAndEndToEndTime() throws SQLException, | |||
| NoSuchFieldException, | |||
There was a problem hiding this comment.
Just change it to throws Exception
tkhurana
reviewed
Mar 15, 2025
| } | ||
| getRows += ")"; | ||
| final String upsertRows = "UPSERT INTO " + tableName + " VALUES(?, ?, ?, ?, ?, ?)"; | ||
| String creatTableDdl = "CREATE TABLE IF NOT EXISTS " + tableName + " (\n" + |
There was a problem hiding this comment.
Generally we put the table schema first and then the upsert statements and the select statements. This makes it easy to understand the code.
tkhurana
reviewed
Mar 15, 2025
Comment on lines
+1475
to
+1476
| int[] taskQueueWaitTime = {10, 5, 20, 7}; | ||
| int[] taskEndToEndTime = {20, 15, 41, 16}; |
There was a problem hiding this comment.
Pass these as parameters in the constructor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the change:
JobCallableinRoundRobinResultItr.when submitting next batches of scans as scanner cache of previous batches of scan has been exhausted. By using JobCallable we can track total wall clock time spent by a query waiting in Phoenix client thread pool queue along with total wall clock time spent in executing tasks (end to end).