belliottsmith commented on code in PR #136:
URL: https://github.com/apache/cassandra-accord/pull/136#discussion_r1837202348
##########
accord-core/src/main/java/accord/impl/AbstractConfigurationService.java:
##########
@@ -252,26 +294,16 @@ public synchronized void fetchTopologyForEpoch(long epoch)
@Override
public void acknowledgeEpoch(EpochReady ready, boolean startSync)
{
- ready.metadata.addCallback(() -> {
- synchronized (AbstractConfigurationService.this)
- {
- epochs.acknowledge(ready);
- }
- });
- ready.fastPath.addCallback(() -> {
- synchronized (AbstractConfigurationService.this)
- {
- localSyncComplete(epochs.getOrCreate(ready.epoch).topology,
startSync);
- }
- });
+ ready.metadata.addCallback(() -> epochs.acknowledge(ready));
+ ready.fastPath.addCallback(() ->
localSyncComplete(epochs.getOrCreate(ready.epoch).topology, startSync));
}
protected void topologyUpdatePreListenerNotify(Topology topology) {}
protected void topologyUpdatePostListenerNotify(Topology topology) {}
- public synchronized void reportTopology(Topology topology, boolean isLoad,
boolean startSync)
+ public void reportTopology(Topology topology, boolean isLoad, boolean
startSync)
Review Comment:
it slightly bothers me that we can have the same topology registered
multiple times.
But, this appears to already be a problem, although a new category of race
conditions open that permit it.
I don't see any obvious way this causes problems besides leaking memory, so
perhaps nothing to worry about here.
--
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]
