Copilot commented on code in PR #2482:
URL: https://github.com/apache/phoenix/pull/2482#discussion_r3277818420
##########
phoenix-core/src/it/java/org/apache/phoenix/jdbc/ParallelPhoenixConnectionFallbackIT.java:
##########
@@ -108,9 +111,27 @@ public void testParallelConnectionBackoff() throws
Exception {
Future<Connection> futureConnB =
executor.submit(() -> DriverManager.getConnection(jdbcUrl, PROPERTIES));
- // The previous call of connection creation should fill the queue by half.
- waitFor(() ->
!PhoenixHAExecutorServiceProvider.hasCapacity(PROPERTIES).get(0)
- && !PhoenixHAExecutorServiceProvider.hasCapacity(PROPERTIES).get(1),
100, 5000);
+ // PHOENIX-7859: Poll actual queue state, not the hasCapacity() composite
— the multi-step
+ // calculation (size/capacity < threshold) had a race window. We now check
queue.size()
+ // directly, then verify hasCapacity() matches expectations.
+ // Note: queueSize >= 1 triggers !hasCapacity() because
HA_MAX_QUEUE_SIZE=2 and
+ // HA_THREADPOOL_QUEUE_BACKOFF_THRESHOLD=0.5, so 1/2 = 0.5 which is NOT <
0.5.
+ waitFor(() -> {
+ List<PhoenixHAExecutorServiceProvider.PhoenixHAClusterExecutorServices>
services =
+ PhoenixHAExecutorServiceProvider.get(PROPERTIES);
+ int queueSize1 = ((ThreadPoolExecutor)
services.get(0).getExecutorService()).getQueue().size();
+ int queueSize2 = ((ThreadPoolExecutor)
services.get(1).getExecutorService()).getQueue().size();
+
+ LOG.debug("Waiting for queues to fill: cluster1 queue={}, cluster2
queue={}",
+ queueSize1, queueSize2);
+
+ return queueSize1 >= 1 && queueSize2 >= 1;
+ }, 100, 5000);
+
+ // Verify that hasCapacity() now correctly reports no capacity
+ List<Boolean> capacity =
PhoenixHAExecutorServiceProvider.hasCapacity(PROPERTIES);
+ assertFalse("Cluster 1 should have no capacity after queues filled",
capacity.get(0).booleanValue());
+ assertFalse("Cluster 2 should have no capacity after queues filled",
capacity.get(1).booleanValue());
Review Comment:
The new assertFalse lines are long enough to trip the 100-character
Checkstyle limit. Wrapping the assertion arguments onto multiple lines (and
relying on auto-unboxing rather than .booleanValue()) will keep formatting
compliant.
##########
phoenix-core/src/it/java/org/apache/phoenix/jdbc/ParallelPhoenixConnectionFallbackIT.java:
##########
@@ -108,9 +111,27 @@ public void testParallelConnectionBackoff() throws
Exception {
Future<Connection> futureConnB =
executor.submit(() -> DriverManager.getConnection(jdbcUrl, PROPERTIES));
- // The previous call of connection creation should fill the queue by half.
- waitFor(() ->
!PhoenixHAExecutorServiceProvider.hasCapacity(PROPERTIES).get(0)
- && !PhoenixHAExecutorServiceProvider.hasCapacity(PROPERTIES).get(1),
100, 5000);
+ // PHOENIX-7859: Poll actual queue state, not the hasCapacity() composite
— the multi-step
+ // calculation (size/capacity < threshold) had a race window. We now check
queue.size()
+ // directly, then verify hasCapacity() matches expectations.
+ // Note: queueSize >= 1 triggers !hasCapacity() because
HA_MAX_QUEUE_SIZE=2 and
+ // HA_THREADPOOL_QUEUE_BACKOFF_THRESHOLD=0.5, so 1/2 = 0.5 which is NOT <
0.5.
+ waitFor(() -> {
+ List<PhoenixHAExecutorServiceProvider.PhoenixHAClusterExecutorServices>
services =
+ PhoenixHAExecutorServiceProvider.get(PROPERTIES);
+ int queueSize1 = ((ThreadPoolExecutor)
services.get(0).getExecutorService()).getQueue().size();
+ int queueSize2 = ((ThreadPoolExecutor)
services.get(1).getExecutorService()).getQueue().size();
Review Comment:
These lines likely violate the repo’s 100-character Checkstyle LineLength
rule (see src/main/config/checkstyle/checker.xml). Consider splitting the
ThreadPoolExecutor casts into intermediate variables (e.g., pool1/pool2) and
wrapping chained calls onto multiple lines to keep each line <= 100 chars.
--
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]
