Skip to content

Commit de62fa1

Browse files
fix: lift shim loader throws to failed futures via SessionPoolMap
ReadRowShim and MutateRowShim cache per-target handles in a Guava LoadingCache whose loader calls Client.openTableAsync (and friends). After the Client-close hardening, that loader throws IllegalStateException post-close — and getUnchecked wraps it in UncheckedExecutionException, so callers see an unchecked exception thrown out of readRow / mutateRow instead of a failed CompletableFuture as the surface contract promises. Introduce SessionPoolMap, a small wrapper over the existing Util.createSessionMap cache that owns the conversion: get() unwraps UncheckedExecutionException to surface the original cause, and apply() converts a loader throw into a failed CompletableFuture for the async call paths. Replace the inline LoadingCache fields in both shim ops files. Covered by a new focused unit test.
1 parent 1a8ef60 commit de62fa1

4 files changed

Lines changed: 197 additions & 30 deletions

File tree

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/compat/ops/MutateRowShim.java

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,28 +22,26 @@
2222
import com.google.cloud.bigtable.data.v2.internal.api.Client;
2323
import com.google.cloud.bigtable.data.v2.internal.api.TableAsync;
2424
import com.google.cloud.bigtable.data.v2.internal.compat.ShimImpl;
25-
import com.google.cloud.bigtable.data.v2.internal.compat.Util;
2625
import com.google.cloud.bigtable.data.v2.internal.session.SessionPool;
2726
import com.google.cloud.bigtable.data.v2.models.AuthorizedViewId;
2827
import com.google.cloud.bigtable.data.v2.models.RowMutation;
2928
import com.google.cloud.bigtable.data.v2.models.TableId;
3029
import com.google.cloud.bigtable.data.v2.models.TargetId;
31-
import com.google.common.cache.LoadingCache;
3230
import io.grpc.Deadline;
3331
import java.io.IOException;
3432
import java.util.concurrent.CompletableFuture;
3533

3634
public class MutateRowShim implements UnaryShim<RowMutation, Void> {
3735

38-
private final LoadingCache<TableId, TableAsync> tables;
39-
private final LoadingCache<AuthorizedViewId, AuthorizedViewAsync> authViews;
36+
private final SessionPoolMap<TableId, TableAsync> tables;
37+
private final SessionPoolMap<AuthorizedViewId, AuthorizedViewAsync> authViews;
4038

4139
public MutateRowShim(Client client) {
4240
tables =
43-
Util.createSessionMap(
41+
new SessionPoolMap<>(
4442
k -> client.openTableAsync(k.getTableId(), Permission.PERMISSION_WRITE));
4543
authViews =
46-
Util.createSessionMap(
44+
new SessionPoolMap<>(
4745
k ->
4846
client.openAuthorizedViewAsync(
4947
k.getTableId(),
@@ -63,9 +61,9 @@ public boolean supports(RowMutation request) {
6361
SessionPool<?> pool;
6462
// TODO: avoid double lookup
6563
if (targetId instanceof TableId) {
66-
pool = tables.getUnchecked((TableId) targetId).getSessionPool();
64+
pool = tables.get((TableId) targetId).getSessionPool();
6765
} else if (targetId instanceof AuthorizedViewId) {
68-
pool = authViews.getUnchecked((AuthorizedViewId) targetId).getSessionPool();
66+
pool = authViews.get((AuthorizedViewId) targetId).getSessionPool();
6967
} else {
7068
return false;
7169
}
@@ -83,16 +81,12 @@ public CompletableFuture<Void> call(RowMutation request, Deadline deadline) {
8381
SessionMutateRowRequest innerReq = request.toSessionProto();
8482

8583
if (targetId instanceof TableId) {
86-
return tables
87-
.getUnchecked((TableId) targetId)
88-
.mutateRow(innerReq, deadline)
89-
.thenApply(r -> null);
84+
return tables.apply(
85+
(TableId) targetId, t -> t.mutateRow(innerReq, deadline).thenApply(r -> null));
9086
}
9187
if (targetId instanceof AuthorizedViewId) {
92-
return authViews
93-
.getUnchecked((AuthorizedViewId) targetId)
94-
.mutateRow(innerReq, deadline)
95-
.thenApply(r -> null);
88+
return authViews.apply(
89+
(AuthorizedViewId) targetId, v -> v.mutateRow(innerReq, deadline).thenApply(r -> null));
9690
}
9791

9892
CompletableFuture<Void> f = new CompletableFuture<>();

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/compat/ops/ReadRowShimInner.java

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,35 +25,33 @@
2525
import com.google.cloud.bigtable.data.v2.internal.api.MaterializedViewAsync;
2626
import com.google.cloud.bigtable.data.v2.internal.api.TableAsync;
2727
import com.google.cloud.bigtable.data.v2.internal.compat.ShimImpl;
28-
import com.google.cloud.bigtable.data.v2.internal.compat.Util;
2928
import com.google.cloud.bigtable.data.v2.internal.session.SessionPool;
3029
import com.google.cloud.bigtable.data.v2.models.AuthorizedViewId;
3130
import com.google.cloud.bigtable.data.v2.models.MaterializedViewId;
3231
import com.google.cloud.bigtable.data.v2.models.Query;
3332
import com.google.cloud.bigtable.data.v2.models.TableId;
3433
import com.google.cloud.bigtable.data.v2.models.TargetId;
35-
import com.google.common.cache.LoadingCache;
3634
import io.grpc.Deadline;
3735
import java.util.concurrent.CompletableFuture;
3836

3937
public class ReadRowShimInner implements UnaryShim<Query, SessionReadRowResponse> {
40-
private final LoadingCache<TableId, TableAsync> tables;
41-
private final LoadingCache<AuthorizedViewId, AuthorizedViewAsync> authViews;
42-
private final LoadingCache<MaterializedViewId, MaterializedViewAsync> matViews;
38+
private final SessionPoolMap<TableId, TableAsync> tables;
39+
private final SessionPoolMap<AuthorizedViewId, AuthorizedViewAsync> authViews;
40+
private final SessionPoolMap<MaterializedViewId, MaterializedViewAsync> matViews;
4341

4442
public ReadRowShimInner(Client client) {
4543
tables =
46-
Util.createSessionMap(
44+
new SessionPoolMap<>(
4745
k -> client.openTableAsync(k.getTableId(), Permission.PERMISSION_READ));
4846
authViews =
49-
Util.createSessionMap(
47+
new SessionPoolMap<>(
5048
k ->
5149
client.openAuthorizedViewAsync(
5250
k.getTableId(),
5351
k.getAuthorizedViewId(),
5452
OpenAuthorizedViewRequest.Permission.PERMISSION_READ));
5553
matViews =
56-
Util.createSessionMap(
54+
new SessionPoolMap<>(
5755
k ->
5856
client.openMaterializedViewAsync(
5957
k.getMaterializedViewId(),
@@ -73,11 +71,11 @@ public boolean supports(Query request) {
7371
SessionPool<?> pool;
7472
// TODO avoid double lookup
7573
if (targetId instanceof TableId) {
76-
pool = tables.getUnchecked((TableId) targetId).getSessionPool();
74+
pool = tables.get((TableId) targetId).getSessionPool();
7775
} else if (targetId instanceof AuthorizedViewId) {
78-
pool = authViews.getUnchecked((AuthorizedViewId) targetId).getSessionPool();
76+
pool = authViews.get((AuthorizedViewId) targetId).getSessionPool();
7977
} else if (targetId instanceof MaterializedViewId) {
80-
pool = matViews.getUnchecked((MaterializedViewId) targetId).getSessionPool();
78+
pool = matViews.get((MaterializedViewId) targetId).getSessionPool();
8179
} else {
8280
return false;
8381
}
@@ -95,13 +93,13 @@ public CompletableFuture<SessionReadRowResponse> call(Query query, Deadline dead
9593
SessionReadRowRequest innerReq = query.toSessionPointProto();
9694

9795
if (targetId instanceof TableId) {
98-
return tables.getUnchecked((TableId) targetId).readRow(innerReq, deadline);
96+
return tables.apply((TableId) targetId, t -> t.readRow(innerReq, deadline));
9997
}
10098
if (targetId instanceof AuthorizedViewId) {
101-
return authViews.getUnchecked((AuthorizedViewId) targetId).readRow(innerReq, deadline);
99+
return authViews.apply((AuthorizedViewId) targetId, v -> v.readRow(innerReq, deadline));
102100
}
103101
if (targetId instanceof MaterializedViewId) {
104-
return matViews.getUnchecked((MaterializedViewId) targetId).readRow(innerReq, deadline);
102+
return matViews.apply((MaterializedViewId) targetId, v -> v.readRow(innerReq, deadline));
105103
}
106104

107105
CompletableFuture<SessionReadRowResponse> f = new CompletableFuture<>();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
* Copyright 2026 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.cloud.bigtable.data.v2.internal.compat.ops;
17+
18+
import com.google.cloud.bigtable.data.v2.internal.compat.Util;
19+
import com.google.common.cache.LoadingCache;
20+
import com.google.common.util.concurrent.UncheckedExecutionException;
21+
import java.io.Closeable;
22+
import java.util.concurrent.CompletableFuture;
23+
import java.util.function.Function;
24+
25+
/**
26+
* Lazily-loaded map from target IDs to per-target handles (TableAsync / AuthorizedViewAsync /
27+
* MaterializedViewAsync, each holding a SessionPool), backed by a Guava {@link LoadingCache}.
28+
* Centralizes the conversion from Guava's {@link UncheckedExecutionException} (which wraps any
29+
* loader throw) into the appropriate shape for each call site: a raw {@link RuntimeException} for
30+
* sync inspections, or a failed {@link CompletableFuture} for async ops.
31+
*
32+
* <p>The most common loader failure is {@link IllegalStateException} from {@code
33+
* Client.openTableAsync} after {@code Client.close}; without unwrapping, callers see {@code
34+
* UncheckedExecutionException} instead of the documented exception type and async callers see a
35+
* thrown exception instead of a failed future.
36+
*/
37+
public class SessionPoolMap<K, V extends Closeable> {
38+
private final LoadingCache<K, V> cache;
39+
40+
public SessionPoolMap(Function<K, V> loader) {
41+
this.cache = Util.createSessionMap(loader);
42+
}
43+
44+
/** Returns the cached value, loading on miss. Loader throws surface as the original cause. */
45+
public V get(K key) {
46+
try {
47+
return cache.getUnchecked(key);
48+
} catch (UncheckedExecutionException e) {
49+
Throwable cause = e.getCause() != null ? e.getCause() : e;
50+
if (cause instanceof RuntimeException) throw (RuntimeException) cause;
51+
if (cause instanceof Error) throw (Error) cause;
52+
throw new RuntimeException(cause);
53+
}
54+
}
55+
56+
/**
57+
* Looks up the cached value and applies {@code op}. If the lookup throws (e.g. {@link
58+
* IllegalStateException} from a closed Client), the throw is converted to a failed future so
59+
* callers consistently observe failures through the future surface.
60+
*/
61+
public <R> CompletableFuture<R> apply(K key, Function<V, CompletableFuture<R>> op) {
62+
V v;
63+
try {
64+
v = get(key);
65+
} catch (Exception e) {
66+
CompletableFuture<R> f = new CompletableFuture<>();
67+
f.completeExceptionally(e);
68+
return f;
69+
}
70+
return op.apply(v);
71+
}
72+
73+
/** Evicts all entries, triggering Closeable.close on each via the cache's removal listener. */
74+
public void invalidateAll() {
75+
cache.invalidateAll();
76+
}
77+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
* Copyright 2026 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.cloud.bigtable.data.v2.internal.compat.ops;
17+
18+
import static com.google.common.truth.Truth.assertThat;
19+
import static org.junit.jupiter.api.Assertions.assertThrows;
20+
21+
import java.io.Closeable;
22+
import java.util.concurrent.CompletableFuture;
23+
import java.util.concurrent.ExecutionException;
24+
import java.util.concurrent.TimeUnit;
25+
import java.util.concurrent.atomic.AtomicInteger;
26+
import org.junit.jupiter.api.Test;
27+
28+
class SessionPoolMapTest {
29+
30+
// Minimal Closeable value type — tracks how many times close() was called so invalidateAll's
31+
// removal-listener wiring is observable from tests.
32+
private static final class CountingHandle implements Closeable {
33+
final AtomicInteger closeCount = new AtomicInteger();
34+
35+
@Override
36+
public void close() {
37+
closeCount.incrementAndGet();
38+
}
39+
}
40+
41+
@Test
42+
void get_unwrapsLoaderRuntimeException() {
43+
// Production trigger: Client.openTableAsync throws IllegalStateException after Client.close.
44+
// The Guava LoadingCache wraps it in UncheckedExecutionException; SessionPoolMap.get must
45+
// surface the original IllegalStateException so callers can pattern-match on it.
46+
SessionPoolMap<String, CountingHandle> map =
47+
new SessionPoolMap<>(
48+
key -> {
49+
throw new IllegalStateException("Client is closed");
50+
});
51+
52+
IllegalStateException thrown = assertThrows(IllegalStateException.class, () -> map.get("k"));
53+
assertThat(thrown).hasMessageThat().isEqualTo("Client is closed");
54+
}
55+
56+
@Test
57+
void apply_convertsLoaderThrowToFailedFuture() throws Exception {
58+
// The async surface must not throw — callers wire onFailure handlers on the returned future.
59+
SessionPoolMap<String, CountingHandle> map =
60+
new SessionPoolMap<>(
61+
key -> {
62+
throw new IllegalStateException("Client is closed");
63+
});
64+
65+
CompletableFuture<String> result =
66+
map.apply("k", v -> CompletableFuture.completedFuture("unreached"));
67+
68+
assertThat(result.isCompletedExceptionally()).isTrue();
69+
ExecutionException ee =
70+
assertThrows(ExecutionException.class, () -> result.get(1, TimeUnit.SECONDS));
71+
assertThat(ee).hasCauseThat().isInstanceOf(IllegalStateException.class);
72+
assertThat(ee).hasCauseThat().hasMessageThat().isEqualTo("Client is closed");
73+
}
74+
75+
@Test
76+
void apply_happyPathInvokesOp() throws Exception {
77+
CountingHandle handle = new CountingHandle();
78+
SessionPoolMap<String, CountingHandle> map = new SessionPoolMap<>(key -> handle);
79+
80+
CompletableFuture<String> result =
81+
map.apply("k", v -> CompletableFuture.completedFuture("ok"));
82+
83+
assertThat(result.get(1, TimeUnit.SECONDS)).isEqualTo("ok");
84+
}
85+
86+
@Test
87+
void invalidateAll_closesCachedValues() {
88+
CountingHandle handle = new CountingHandle();
89+
SessionPoolMap<String, CountingHandle> map = new SessionPoolMap<>(key -> handle);
90+
91+
// Populate the cache so invalidateAll has something to evict.
92+
map.get("k");
93+
assertThat(handle.closeCount.get()).isEqualTo(0);
94+
95+
map.invalidateAll();
96+
assertThat(handle.closeCount.get()).isEqualTo(1);
97+
}
98+
}

0 commit comments

Comments
 (0)