Skip to content

Commit c8e615e

Browse files
fix(bigtable): Ensure that FallbackChannelPool locks doesnt leak to alien listeners (#13195)
1 parent c98e53d commit c8e615e

1 file changed

Lines changed: 29 additions & 19 deletions

File tree

  • java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/channels

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/channels/FallbackChannelPool.java

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
import javax.annotation.concurrent.GuardedBy;
3838

3939
public class FallbackChannelPool implements ChannelPool {
40-
private FallbackConfiguration config;
40+
private volatile FallbackConfiguration config;
4141
private final ChannelPool primary;
4242
private final ChannelPool secondary;
4343
private final PoolFallbackListener poolFallbackListener;
@@ -94,22 +94,29 @@ public void updateConfig(ChannelPoolConfiguration newConfig) {
9494
}
9595

9696
// For now only updates error rate, interval, and is enabled.
97-
synchronized void updateConfig(FallbackConfiguration newConfig) {
98-
config =
99-
config.toBuilder()
100-
.setEnabled(newConfig.isEnabled())
101-
.setCheckInterval(newConfig.getCheckInterval())
102-
.setErrorRate(newConfig.getErrorRate())
103-
.build();
104-
105-
if (!config.isEnabled()) {
106-
if (currentPool.compareAndSet(secondary, primary)) {
107-
poolFallbackListener.onFallback(
108-
config.getFallbackName(),
109-
config.getPrimaryName(),
110-
ChannelFallbackReason.FALLBACK_DISABLE);
97+
void updateConfig(FallbackConfiguration newConfig) {
98+
boolean triggerCallback = false;
99+
FallbackConfiguration localConfig;
100+
synchronized (this) {
101+
config =
102+
config.toBuilder()
103+
.setEnabled(newConfig.isEnabled())
104+
.setCheckInterval(newConfig.getCheckInterval())
105+
.setErrorRate(newConfig.getErrorRate())
106+
.build();
107+
localConfig = config;
108+
109+
if (!localConfig.isEnabled()) {
110+
triggerCallback = currentPool.compareAndSet(secondary, primary);
111111
}
112112
}
113+
114+
if (triggerCallback) {
115+
poolFallbackListener.onFallback(
116+
localConfig.getFallbackName(),
117+
localConfig.getPrimaryName(),
118+
ChannelFallbackReason.FALLBACK_DISABLE);
119+
}
113120
}
114121

115122
@Override
@@ -193,19 +200,22 @@ private void checkRatesAndReschedule() {
193200
scheduleCheckRates();
194201
}
195202

196-
private synchronized void checkRates() {
203+
private void checkRates() {
197204
int successes = successCount.getAndSet(0);
198205
int failures = failureCount.getAndSet(0);
199206
int total = successes + failures;
207+
FallbackConfiguration localConfig = config;
200208

201-
if (!config.isEnabled()) {
209+
if (!localConfig.isEnabled()) {
202210
return;
203211
}
204212

205-
if (total > 0 && ((double) failures) / total >= config.getErrorRate()) {
213+
if (total > 0 && ((double) failures) / total >= localConfig.getErrorRate()) {
206214
if (currentPool.compareAndSet(primary, secondary)) {
207215
poolFallbackListener.onFallback(
208-
config.getPrimaryName(), config.getFallbackName(), ChannelFallbackReason.ERROR_RATE);
216+
localConfig.getPrimaryName(),
217+
localConfig.getFallbackName(),
218+
ChannelFallbackReason.ERROR_RATE);
209219
}
210220
}
211221
}

0 commit comments

Comments
 (0)