Copilot commented on code in PR #254:
URL: https://github.com/apache/cassandra-accord/pull/254#discussion_r2355356021
##########
accord-maelstrom/src/main/java/accord/maelstrom/Main.java:
##########
@@ -189,7 +188,8 @@ MaelstromAgent.INSTANCE, new DefaultRandom(), scheduler,
SizeOfIntersectionSorte
DefaultRemoteListeners::new, DefaultTimeouts::new,
DefaultProgressLogs::new, DefaultLocalListeners.Factory::new,
InMemoryCommandStores.SingleThread::new, new
CoordinationAdapter.DefaultFactory(),
DurableBefore.NOOP_PERSISTER, journal);
- awaitUninterruptibly(on.unsafeStart());
+ CountDownLatch latch = new CountDownLatch(1);
+ on.unsafeStart().invoke(latch::countDown);
Review Comment:
The CountDownLatch is created but never awaited. This means the code
continues execution without waiting for the async operation to complete, which
likely breaks the intended synchronization behavior.
```suggestion
on.unsafeStart().invoke(latch::countDown);
try {
latch.await();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new RuntimeException("Node initialization
interrupted", e);
}
```
##########
accord-core/src/test/java/accord/utils/async/AsyncChainsTest.java:
##########
@@ -150,42 +146,21 @@ void reduceTest()
@Test
void beginSeesException()
{
- AsyncChains.ofCallable(ignore -> {
- throw new RejectedExecutionException();
- }, () -> 42)
+ AsyncExecutor rejecting = ignore -> { throw new
RejectedExecutionException(); };
Review Comment:
The AsyncExecutor interface expects an execute(Runnable) method, but this
lambda only implements the chain method. This will cause a ClassCastException
or compilation error.
```suggestion
AsyncExecutor rejecting = runnable -> { throw new
RejectedExecutionException(); };
```
##########
accord-core/src/test/java/accord/utils/async/AsyncChainsTest.java:
##########
@@ -331,7 +306,7 @@ protected Cancellable start(BiConsumer<? super Integer,
Throwable> callback)
topLevel.add(() -> {
AsyncResult.Settable<Integer> settable = AsyncResults.settable();
settable.setSuccess(42);
- return settable;
+ return settable.chain();
Review Comment:
[nitpick] Consider using settable directly since AsyncResult extends
AsyncChain functionality. The .chain() call creates an unnecessary wrapper.
```suggestion
return settable;
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
