Skip to content

Commit fc9aafd

Browse files
chore: back OpExecutor with SerializingExecutor(userCallbackExecutor)
VOperationImpl now constructs OpExecutor over a per-call SequentialExecutor on the shared userCallbackExecutor, replacing the per-call SynchronizationContext. OpExecutor gains an UncaughtExceptionHandler ctor arg — the safety net that the removed RetryingVRpc-owned SyncContext provided. The 3-arg VRpcCallContext.create defaults to a no-op handler for tests; production callers go through VOperationImpl.
1 parent df61652 commit fc9aafd

4 files changed

Lines changed: 35 additions & 17 deletions

File tree

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/api/TableBase.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ public void readRow(
118118
new RetryingVRpc<>(() -> sessionPool.newCall(readRowDescriptor), timer);
119119
VRpcTracer tracer = metrics.newTableTracer(sessionPool.getInfo(), readRowDescriptor, deadline);
120120

121-
new VOperationImpl<>(retry, Context.current(), tracer, deadline, true).start(req, listener);
121+
new VOperationImpl<>(retry, Context.current(), userCallbackExecutor, tracer, deadline, true)
122+
.start(req, listener);
122123
}
123124

124125
public void mutateRow(
@@ -131,7 +132,8 @@ public void mutateRow(
131132
VRpcTracer tracer =
132133
metrics.newTableTracer(sessionPool.getInfo(), mutateRowDescriptor, deadline);
133134

134-
new VOperationImpl<>(retry, Context.current(), tracer, deadline, idempotent)
135+
new VOperationImpl<>(
136+
retry, Context.current(), userCallbackExecutor, tracer, deadline, idempotent)
135137
.start(req, listener);
136138
}
137139
}

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/middleware/OpExecutor.java

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,28 @@
1919
import java.util.concurrent.Executor;
2020

2121
/**
22-
* Per-op serializing executor. Wraps a delegate {@link Executor} and tracks which thread is
23-
* currently running a task, so callers can assert they are on the executor (analogous to {@code
22+
* Per-op serializing executor. Wraps a delegate {@link Executor} (typically a per-call {@code
23+
* SerializingExecutor} over the user-callback pool) and tracks which thread is currently running a
24+
* task, so callers can assert they are on the executor (analogous to {@code
2425
* SynchronizationContext#throwIfNotInThisSynchronizationContext}).
2526
*
26-
* <p>Backing executor evolves over the refactor — for now it is the per-call {@link
27-
* io.grpc.SynchronizationContext} that {@link VOperationImpl} constructs. Later commits swap it
28-
* for a {@code SerializingExecutor} over the user-callback pool, and eventually a tailored inline
29-
* queue.
27+
* <p>If a task throws, the registered {@link UncaughtExceptionHandler} is invoked — this is the
28+
* last-resort recovery point for the chain. Without it, a throw from a user-installed tracer or a
29+
* listener callback would silently drop and the caller's future would never complete.
3030
*/
3131
public final class OpExecutor implements Executor {
3232

33+
public interface UncaughtExceptionHandler {
34+
void uncaught(Throwable t);
35+
}
36+
3337
private final Executor backing;
38+
private final UncaughtExceptionHandler handler;
3439
private volatile Thread runningThread;
3540

36-
public OpExecutor(Executor backing) {
41+
public OpExecutor(Executor backing, UncaughtExceptionHandler handler) {
3742
this.backing = backing;
43+
this.handler = handler;
3844
}
3945

4046
@Override
@@ -45,6 +51,8 @@ public void execute(Runnable r) {
4551
runningThread = Thread.currentThread();
4652
try {
4753
r.run();
54+
} catch (Throwable t) {
55+
handler.uncaught(t);
4856
} finally {
4957
runningThread = prev;
5058
}

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/middleware/VOperationImpl.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
import com.google.common.util.concurrent.MoreExecutors;
2525
import io.grpc.Context;
2626
import io.grpc.Deadline;
27-
import io.grpc.SynchronizationContext;
2827
import java.util.Optional;
28+
import java.util.concurrent.Executor;
2929
import java.util.concurrent.TimeoutException;
3030
import javax.annotation.Nullable;
3131

@@ -39,6 +39,7 @@ public class VOperationImpl<ReqT, RespT> implements VOperation<ReqT, RespT> {
3939

4040
private final VRpc<ReqT, RespT> chain;
4141
private final Context grpcContext;
42+
private final Executor userCallbackExecutor;
4243
private final VRpcTracer tracer;
4344
private final Deadline deadline;
4445
private final boolean idempotent;
@@ -47,11 +48,13 @@ public class VOperationImpl<ReqT, RespT> implements VOperation<ReqT, RespT> {
4748
public VOperationImpl(
4849
VRpc<ReqT, RespT> chain,
4950
Context grpcContext,
51+
Executor userCallbackExecutor,
5052
VRpcTracer tracer,
5153
Deadline deadline,
5254
boolean idempotent) {
5355
this.chain = chain;
5456
this.grpcContext = grpcContext;
57+
this.userCallbackExecutor = userCallbackExecutor;
5558
this.tracer = tracer;
5659
this.deadline = deadline;
5760
this.idempotent = idempotent;
@@ -69,12 +72,14 @@ public VOperationImpl(
6972

7073
@Override
7174
public void start(ReqT req, VRpcListener<RespT> listener) {
72-
// Per-call SynchronizationContext serializes all middleware below this layer. Uncaught task
73-
// failures drive the chain to a terminal state so the caller's listener still gets onClose;
74-
// RetryingVRpc.cancel is idempotent so the resulting cascade collapses safely.
75-
SynchronizationContext syncContext =
76-
new SynchronizationContext((t, e) -> chain.cancel("Uncaught exception in op executor", e));
77-
OpExecutor exec = new OpExecutor(syncContext);
75+
// Per-call SerializingExecutor over the shared user-callback pool. The handler is the
76+
// last-resort recovery: if any op-executor task throws (typically a user-installed tracer or
77+
// a listener callback escape), drive the chain to a terminal state so the caller's listener
78+
// still receives an onClose. RetryingVRpc.cancel is idempotent so cascades collapse safely.
79+
OpExecutor exec =
80+
new OpExecutor(
81+
MoreExecutors.newSequentialExecutor(userCallbackExecutor),
82+
t -> chain.cancel("Uncaught exception in op executor task", t));
7883
VRpcCallContext ctx = VRpcCallContext.create(deadline, idempotent, tracer, exec);
7984
grpcContext.addListener(cancellationListener, MoreExecutors.directExecutor());
8085
chain.start(req, ctx, new CleanupListener<>(listener, grpcContext, cancellationListener));

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/middleware/VRpc.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,10 @@ abstract class VRpcCallContext {
111111
public static VRpcCallContext create(
112112
Deadline deadline, boolean isIdempotent, VRpcTracer tracer) {
113113
return create(
114-
deadline, isIdempotent, tracer, new OpExecutor(MoreExecutors.directExecutor()));
114+
deadline,
115+
isIdempotent,
116+
tracer,
117+
new OpExecutor(MoreExecutors.directExecutor(), t -> {}));
115118
}
116119

117120
public static VRpcCallContext create(

0 commit comments

Comments
 (0)