Skip to content

Commit e8bc94a

Browse files
committed
address comments
1 parent 944d965 commit e8bc94a

3 files changed

Lines changed: 14 additions & 24 deletions

File tree

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyAwareChannel.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -413,11 +413,6 @@ private static boolean shouldExcludeEndpointOnRetry(io.grpc.Status.Code statusCo
413413
|| statusCode == io.grpc.Status.Code.UNAVAILABLE;
414414
}
415415

416-
private Predicate<String> consumeExcludedEndpointsForCurrentCall(
417-
@Nullable String logicalRequestKey) {
418-
return endpointOverloadCooldowns::isCoolingDown;
419-
}
420-
421416
@VisibleForTesting
422417
boolean isCoolingDown(String address) {
423418
return endpointOverloadCooldowns.isCoolingDown(address);
@@ -844,7 +839,7 @@ private void maybeTrackReadOnlyBegin(TransactionSelector selector) {
844839

845840
private Predicate<String> excludedEndpoints() {
846841
if (excludedEndpoints == null) {
847-
excludedEndpoints = parentChannel.consumeExcludedEndpointsForCurrentCall(logicalRequestKey);
842+
excludedEndpoints = parentChannel.endpointOverloadCooldowns::isCoolingDown;
848843
}
849844
return excludedEndpoints;
850845
}

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyRangeCache.java

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.google.api.core.InternalApi;
2020
import com.google.common.annotations.VisibleForTesting;
21+
import com.google.common.collect.Lists;
2122
import com.google.common.hash.Hashing;
2223
import com.google.protobuf.ByteString;
2324
import com.google.spanner.v1.CacheUpdate;
@@ -27,7 +28,6 @@
2728
import com.google.spanner.v1.RoutingHint;
2829
import com.google.spanner.v1.Tablet;
2930
import java.time.Duration;
30-
import java.util.AbstractList;
3131
import java.util.ArrayList;
3232
import java.util.Arrays;
3333
import java.util.Collections;
@@ -740,7 +740,7 @@ RouteLookupResult lookupRoutingHint(
740740
directedReadOptions.getReplicasCase()
741741
!= DirectedReadOptions.ReplicasCase.REPLICAS_NOT_SET;
742742
Map<String, ChannelEndpoint> resolvedEndpoints = new HashMap<>();
743-
SelectionStats selectionStats = new SelectionStats();
743+
SelectionState selectionStats = new SelectionState();
744744

745745
TabletSnapshot selected =
746746
selectTablet(
@@ -812,7 +812,7 @@ private TabletSnapshot selectTablet(
812812
Set<Long> skippedTabletUids,
813813
List<SkippedTabletDetail> skippedTabletDetails,
814814
Map<String, ChannelEndpoint> resolvedEndpoints,
815-
SelectionStats selectionStats) {
815+
SelectionState selectionStats) {
816816
if (!preferLeader || hintBuilder.getOperationUid() > 0L) {
817817
TabletSnapshot preferredLeader =
818818
preferLeader ? localLeaderForScoreBias(snapshot, hasDirectedReadOptions) : null;
@@ -893,7 +893,7 @@ private TabletSnapshot selectScoreAwareTablet(
893893
Set<Long> skippedTabletUids,
894894
List<SkippedTabletDetail> skippedTabletDetails,
895895
Map<String, ChannelEndpoint> resolvedEndpoints,
896-
SelectionStats selectionStats,
896+
SelectionState selectionStats,
897897
@javax.annotation.Nullable TabletSnapshot preferredLeader) {
898898
long operationUid = hintBuilder.getOperationUid();
899899
List<EligibleReplica> eligibleReplicas =
@@ -924,7 +924,7 @@ private List<EligibleReplica> collectEligibleReplicas(
924924
Set<Long> skippedTabletUids,
925925
List<SkippedTabletDetail> skippedTabletDetails,
926926
Map<String, ChannelEndpoint> resolvedEndpoints,
927-
SelectionStats selectionStats,
927+
SelectionState selectionStats,
928928
long operationUid,
929929
boolean preferLeader,
930930
@javax.annotation.Nullable TabletSnapshot preferredLeader) {
@@ -991,17 +991,7 @@ private EligibleReplica lowestCostReplica(List<EligibleReplica> eligibleReplicas
991991
}
992992

993993
private List<ChannelEndpoint> endpointView(List<EligibleReplica> eligibleReplicas) {
994-
return new AbstractList<ChannelEndpoint>() {
995-
@Override
996-
public ChannelEndpoint get(int index) {
997-
return eligibleReplicas.get(index).endpoint;
998-
}
999-
1000-
@Override
1001-
public int size() {
1002-
return eligibleReplicas.size();
1003-
}
1004-
};
994+
return Lists.transform(eligibleReplicas, candidate -> candidate.endpoint);
1005995
}
1006996

1007997
private double selectionCostForEndpoint(
@@ -1109,7 +1099,7 @@ private boolean shouldSkip(
11091099
Set<Long> skippedTabletUids,
11101100
List<SkippedTabletDetail> skippedTabletDetails,
11111101
Map<String, ChannelEndpoint> resolvedEndpoints,
1112-
SelectionStats selectionStats) {
1102+
SelectionState selectionStats) {
11131103
String targetEndpointLabel = endpointLabel(snapshot, tablet);
11141104
if (tablet.skip) {
11151105
selectionStats.sawNonExcludedReplica = true;
@@ -1179,7 +1169,7 @@ private boolean shouldSkip(
11791169
return true;
11801170
}
11811171

1182-
private final class SelectionStats {
1172+
private final class SelectionState {
11831173
private boolean sawMatchingReplica;
11841174
private boolean sawExcludedReplica;
11851175
private boolean sawNonExcludedReplica;

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/RequestIdTargetTracker.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,13 @@
2626
final class RequestIdTargetTracker {
2727
@VisibleForTesting static final long MAX_TRACKED_TARGETS = 1_000_000L;
2828

29+
@VisibleForTesting
30+
static final int TARGET_CACHE_CONCURRENCY =
31+
Math.max(4, Runtime.getRuntime().availableProcessors());
32+
2933
private static final Cache<String, RoutingTarget> TARGETS =
3034
CacheBuilder.newBuilder()
35+
.concurrencyLevel(TARGET_CACHE_CONCURRENCY)
3136
.maximumSize(MAX_TRACKED_TARGETS)
3237
.expireAfterWrite(10, TimeUnit.MINUTES)
3338
.build();

0 commit comments

Comments
 (0)