maytasm commented on code in PR #19787:
URL: https://github.com/apache/druid/pull/19787#discussion_r3670638568
##########
server/src/main/java/org/apache/druid/client/BrokerViewOfCoordinatorConfig.java:
##########
@@ -54,10 +53,11 @@ public class BrokerViewOfCoordinatorConfig extends
BaseBrokerViewOfConfig<Coordi
{
private final CoordinatorClient coordinatorClient;
- @GuardedBy("this")
- private Set<String> targetCloneServers;
- @GuardedBy("this")
- private Set<String> sourceCloneServers;
+ // volatile, not synchronized: getCurrentServersToIgnore() is called
per-segment during query planning.
+ // Under high concurrency, synchronized causes monitor convoy with 100x
throughput degradation.
+ // Each field is an immutable Set reference, so volatile provides sufficient
visibility.
+ private volatile Set<String> targetCloneServers = Set.of();
Review Comment:
I put this in the comment for the `getCurrentServersToIgnore` method, but I
think this is fine because in `getCurrentServersToIgnore` each branch reads
only one volatile field. Readers may see `targetCloneServers` and
`sourceCloneServers` from different `setDynamicConfig()` calls, but this is
acceptable since no `CloneQueryMode` needs both.
--
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]
