Skip to content

Commit c70f7ff

Browse files
committed
rearchitect so that channelpoolhealthchecker doesn't keep a separate datastructure of entries
Change-Id: I0da4b9bad6470c0cc6b5ae168e318a1ef0620ab4
1 parent 4229883 commit c70f7ff

3 files changed

Lines changed: 38 additions & 18 deletions

File tree

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelPool.java

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import java.util.logging.Level;
4343
import java.util.logging.Logger;
4444
import javax.annotation.Nullable;
45+
import org.checkerframework.checker.units.qual.A;
4546

4647
/**
4748
* A {@link ManagedChannel} that will send requests round-robin via a set of channels.
@@ -61,6 +62,7 @@ public class BigtableChannelPool extends ManagedChannel {
6162
private final BigtableChannelPoolSettings settings;
6263
private final ChannelFactory channelFactory;
6364
private final ScheduledExecutorService executor;
65+
private final ScheduledExecutorService channelHealthProbingExecutor = Executors.newSingleThreadScheduledExecutor();
6466

6567
private final Object entryWriteLock = new Object();
6668
@VisibleForTesting final AtomicReference<ImmutableList<Entry>> entries = new AtomicReference<>();
@@ -94,7 +96,7 @@ public static BigtableChannelPool create(
9496
ImmutableList.Builder<Entry> initialListBuilder = ImmutableList.builder();
9597

9698
for (int i = 0; i < settings.getInitialChannelCount(); i++) {
97-
initialListBuilder.add(new Entry(channelFactory.createSingleChannel()));
99+
initialListBuilder.add(new Entry(channelFactory.createSingleChannel(), channelHealthProbingExecutor));
98100
}
99101

100102
entries.set(initialListBuilder.build());
@@ -152,6 +154,9 @@ public ManagedChannel shutdown() {
152154
// shutdownNow will cancel scheduled tasks
153155
executor.shutdownNow();
154156
}
157+
if (channelHealthProbingExecutor != null){
158+
channelHealthProbingExecutor.shutdownNow();
159+
}
155160
return this;
156161
}
157162

@@ -192,6 +197,9 @@ public ManagedChannel shutdownNow() {
192197
if (executor != null) {
193198
executor.shutdownNow();
194199
}
200+
if (channelHealthProbingExecutor != null) {
201+
channelHealthProbingExecutor.shutdownNow();
202+
}
195203
return this;
196204
}
197205

@@ -318,7 +326,7 @@ private void expand(int desiredSize) {
318326

319327
for (int i = 0; i < desiredSize - localEntries.size(); i++) {
320328
try {
321-
newEntries.add(new Entry(channelFactory.createSingleChannel()));
329+
newEntries.add(new Entry(channelFactory.createSingleChannel(), channelHealthProbingExecutor));
322330
} catch (IOException e) {
323331
LOG.log(Level.WARNING, "Failed to add channel", e);
324332
}
@@ -356,7 +364,7 @@ void refresh() {
356364

357365
for (int i = 0; i < newEntries.size(); i++) {
358366
try {
359-
newEntries.set(i, new Entry(channelFactory.createSingleChannel()));
367+
newEntries.set(i, new Entry(channelFactory.createSingleChannel(), channelHealthProbingExecutor));
360368
} catch (IOException e) {
361369
LOG.log(Level.WARNING, "Failed to refresh channel, leaving old channel", e);
362370
}
@@ -433,19 +441,40 @@ static class Entry {
433441

434442
private final AtomicInteger maxOutstanding = new AtomicInteger();
435443

444+
/** Number of probes in flight plus number of probe results. (No-op stub) */
445+
AtomicInteger recentProbesSent() {
446+
return new AtomicInteger(0);
447+
}
448+
449+
/** Number of recently failed probes. (No-op stub) */
450+
AtomicInteger recentlyFailedProbes() {
451+
return new AtomicInteger(0);
452+
}
453+
454+
private final AtomicInteger probesInFlight = new AtomicInteger(0);
455+
456+
436457
// Flag that the channel should be closed once all of the outstanding RPC complete.
437458
private final AtomicBoolean shutdownRequested = new AtomicBoolean();
438459
// Flag that the channel has been closed.
439460
private final AtomicBoolean shutdownInitiated = new AtomicBoolean();
461+
private final ChannelHealthChecker healthChecker;
440462

441-
private Entry(ManagedChannel channel) {
463+
private Entry(ManagedChannel channel, ScheduledExecutorService executor) {
442464
this.channel = channel;
465+
this.healthChecker = new ChannelHealthChecker(this, executor);
443466
}
444467

445468
ManagedChannel getManagedChannel() {
446469
return this.channel;
447470
}
448471

472+
// Add a getter for the healthChecker
473+
public ChannelHealthChecker getHealthChecker() {
474+
return healthChecker;
475+
}
476+
477+
449478
int getAndResetMaxOutstanding() {
450479
return maxOutstanding.getAndSet(outstandingRpcs.get());
451480
}
@@ -460,7 +489,7 @@ private boolean retain() {
460489
// register desire to start RPC
461490
int currentOutstanding = outstandingRpcs.incrementAndGet();
462491

463-
// Rough book keeping
492+
// Rough bookkeeping
464493
int prevMax = maxOutstanding.get();
465494
if (currentOutstanding > prevMax) {
466495
maxOutstanding.incrementAndGet();
@@ -497,6 +526,9 @@ private void release() {
497526
*/
498527
private void requestShutdown() {
499528
shutdownRequested.set(true);
529+
if (healthChecker != null) {
530+
healthChecker.stop();
531+
}
500532
if (outstandingRpcs.get() == 0) {
501533
shutdown();
502534
}

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/ChannelHealthChecker.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public class ChannelHealthChecker {
4343
private volatile ScheduledFuture<?> scheduledProbeFuture;
4444
private final ReadWriteLock probeResultsLock = new ReentrantReadWriteLock();
4545
private final EvictingQueue<ProbeResult> probeResults;
46-
private final AtomicInteger probesInFlight = new AtomicInteger(0);
46+
// private final AtomicInteger probesInFlight = new AtomicInteger(0);
4747

4848
/** Inner class to represent the result of a single probe. */
4949
class ProbeResult {
@@ -84,16 +84,6 @@ void probeFinished(Instant startTime, boolean success) {
8484
// Method stub, no operation.
8585
}
8686

87-
/** Number of probes in flight plus number of probe results. (No-op stub) */
88-
private int recentProbesSent() {
89-
return 0;
90-
}
91-
92-
/** Number of recently failed probes. (No-op stub) */
93-
public int recentlyFailedProbes() {
94-
return 0;
95-
}
96-
9787
/**
9888
* Determines if the channel is healthy. (No-op stub)
9989
*

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/ChannelPoolHealthChecker.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,11 @@ public class ChannelPoolHealthChecker {
3131

3232
// Class fields
3333
private final Supplier<ImmutableList<Entry>> entrySupplier;
34-
private Map<Entry, ChannelHealthChecker> healthCheckers;
3534
private Instant lastEviction;
3635
private ScheduledExecutorService executor;
3736

3837
/** Constructor for the pool health checker. */
3938
public ChannelPoolHealthChecker(Supplier<ImmutableList<Entry>> entrySupplier) {
40-
this.healthCheckers = Collections.synchronizedMap(new WeakHashMap<>());
4139
this.entrySupplier = entrySupplier;
4240
this.lastEviction = Instant.MIN;
4341
this.executor = Executors.newSingleThreadScheduledExecutor();

0 commit comments

Comments
 (0)