d-c-manning commented on code in PR #2286:
URL: https://github.com/apache/phoenix/pull/2286#discussion_r2345625277
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/QueryServicesOptions.java:
##########
@@ -314,8 +314,8 @@ public class QueryServicesOptions {
public static final boolean DEFAULT_EXPLAIN_CHUNK_COUNT = true;
public static final boolean DEFAULT_EXPLAIN_ROW_COUNT = true;
public static final boolean DEFAULT_ALLOW_ONLINE_TABLE_SCHEMA_UPDATE = true;
- public static final int DEFAULT_RETRIES_FOR_SCHEMA_UPDATE_CHECK = 10;
- public static final long DEFAULT_DELAY_FOR_SCHEMA_UPDATE_CHECK = 5 * 1000;
// 5 seconds.
+ public static final int DEFAULT_RETRIES_FOR_SCHEMA_UPDATE_CHECK = 50;
+ public static final long DEFAULT_DELAY_FOR_SCHEMA_UPDATE_CHECK = 1000; // 1
second.
Review Comment:
> @virajjasani Shouldn't we go to sleep only when success is false
https://github.com/apache/phoenix/blob/master/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java#L1751
Right, I'm not intimately familiar with this part of the code, but it seemed
like it was just a bug that it was sleeping even in the success case. Maybe a 5
second sleep is fine if there was an exception... especially if the exception
code path is truly exceptional and usually we just succeed.
In that case, you may not even have to change these values.
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -1748,7 +1748,9 @@ private void checkAndRetry(RetriableOperation op) throws
InterruptedException, T
}
}
numTries++;
- Thread.sleep(sleepInterval);
+ if (!success) {
+ Thread.sleep(sleepInterval);
+ }
} while (numTries < maxRetries && !success);
Review Comment:
I mean... while you're here... this seems wrong too?
The first time we come into this while condition, `numTries` will be `2`. So
only if your retries are 3+ will you attempt even 1 retry? The default value of
10 for `NUM_RETRIES_FOR_SCHEMA_UPDATE_CHECK` means you will have done 9
attempts (8 retries). It's misleading.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
