apurtell commented on code in PR #2482:
URL: https://github.com/apache/phoenix/pull/2482#discussion_r3278752939
##########
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;
Review Comment:
If any future change adjusts `HA_MAX_QUEUE_SIZE` this test may pass when it
should fail (the wait completes earlier than the fallback signal actually
flips) or fail in confusing ways. Consider deriving the trigger from the
configured values, e.g.:
```java
int maxQueue = Integer.parseInt(
PROPERTIES.getProperty(PhoenixHAExecutorServiceProvider.HA_MAX_QUEUE_SIZE));
double threshold = Double.parseDouble(
PROPERTIES.getProperty(
PhoenixHAExecutorServiceProvider.HA_THREADPOOL_QUEUE_BACKOFF_THRESHOLD));
int trigger = (int) Math.ceil(threshold * maxQueue);
```
... or just keep `hasCapacity()` as the wait predicate
--
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]
