murblanc commented on a change in pull request #2291:
URL: https://github.com/apache/lucene-solr/pull/2291#discussion_r569754269
##########
File path:
solr/core/src/java/org/apache/solr/cluster/events/impl/DelegatingClusterEventProducer.java
##########
@@ -90,7 +101,11 @@ public void setDelegate(ClusterEventProducer newDelegate) {
log.debug("--- delegate {} already in state {}", delegate,
delegate.getState());
}
}
- this.version++;
+ Phaser localPhaser = phaser; // volatile read
+ if (localPhaser != null) {
+ assert localPhaser.getRegisteredParties() == 1;
+ localPhaser.arrive(); // we should be the only ones registered, so this
will advance phase each time
Review comment:
Why make the assumption this class is the only one used by the test?
Maybe the test uses the `Phaser` to synchronize multiple classes? That just
means remove the assert and the comment.
But here's the issue I alluded to in the Jira: by exposing directly `Phaser`
and not a test provided implementation of an interface method to call, we
restrict the usage of such synchronization to only allow the test code to know
when something took place in the production code.
If we expose an interface, it allows the test to stall the production code
at this stage, maybe to make it wait for another condition to happen (for
example let the test verify the current state produced by the production code)
and then the test can decide to resume the production code. This is useful when
verifying state machines for example, as otherwise the state transitions can be
so fast that it's impossible to verify reliably (by the time the test is
notified of the first transition and tries to verify it, the state machine has
made 5 more transitions...).
I'd really prefer to expose an interface as in the previous version of the
code. Then have the test implement it by basically delegating to `Phaser`
(indeed simpler than implementing our own).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
