Skip to content

Commit 3d6df06

Browse files
committed
xds: Implement fail-open queue draining and optimize locks in ext_proc interceptor
- Implement fail-open queue draining for both inbound and outbound directions. - Defer transport half-close during fail-open until all outbound queues are drained. - Refactor DataPlaneClientCall.onReady to release streamLock before calling drain/update methods, avoiding deadlocks. - Consolidate and batch outbound draining in drainPendingDrainingMessages.
1 parent b23b94d commit 3d6df06

2 files changed

Lines changed: 710 additions & 53 deletions

File tree

xds/src/main/java/io/grpc/xds/ExternalProcessorClientInterceptor.java

Lines changed: 118 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,8 @@ private void onExtProcStreamReady() {
792792
void drainPendingRequests() {
793793
synchronized (streamLock) {
794794
if (config.getObservabilityMode()
795-
|| currentProcessingMode.getResponseBodyMode() != ProcessingMode.BodySendMode.GRPC) {
795+
|| currentProcessingMode.getResponseBodyMode() != ProcessingMode.BodySendMode.GRPC
796+
|| extProcStreamState.get().isCompleted()) {
796797
int toRequest = pendingRequests.getAndSet(0);
797798
if (toRequest > 0) {
798799
super.request(toRequest);
@@ -853,6 +854,37 @@ private void onReadyNotify() {
853854
wrappedListener.onReadyNotify();
854855
}
855856

857+
void onReady() {
858+
boolean isPassThrough;
859+
boolean isCompleted;
860+
boolean isDraining;
861+
862+
synchronized (streamLock) {
863+
isPassThrough = passThroughMode.get();
864+
ExtProcStreamState state = extProcStreamState.get();
865+
isCompleted = state.isCompleted();
866+
isDraining = state.isDraining();
867+
}
868+
869+
if (isPassThrough) {
870+
onReadyNotify();
871+
return;
872+
}
873+
874+
if (isCompleted) {
875+
drainPendingDrainingMessages();
876+
return;
877+
}
878+
879+
// Normal or Draining operation
880+
drainPendingUpstreamBodyMessages();
881+
if (!isDraining) {
882+
trySendAccumulatedWindowUpdates();
883+
}
884+
drainPendingRequests();
885+
onReadyNotify();
886+
}
887+
856888
boolean isSidecarReady() {
857889
ExtProcStreamState state = extProcStreamState.get();
858890
if (state.isCompleted()) {
@@ -1109,7 +1141,7 @@ private void handleRequestBodyResponse(BodyResponse bodyResponse) {
11091141
boolean sendImmediately = false;
11101142
synchronized (streamLock) {
11111143
sidestreamToUpstreamWindow -= body.size();
1112-
if (super.isReady() && pendingUpstreamBodyMessages.isEmpty()) {
1144+
if (pendingUpstreamBodyMessages.isEmpty() && super.isReady()) {
11131145
sendImmediately = true;
11141146
accumulatedWindowUpdateSidestreamToUpstream += body.size();
11151147
} else {
@@ -1202,29 +1234,44 @@ private void drainPendingMutatedResponseBodies() {
12021234
}
12031235
}
12041236

1237+
void drainPendingMutatedResponseBodiesDirect(DataPlaneListener listener) {
1238+
List<ByteString> toDeliver = new ArrayList<>();
1239+
synchronized (streamLock) {
1240+
ByteString body;
1241+
while ((body = pendingMutatedResponseBodies.poll()) != null) {
1242+
toDeliver.add(body);
1243+
}
1244+
}
1245+
for (ByteString body : toDeliver) {
1246+
listener.onExternalBody(body);
1247+
}
1248+
}
1249+
12051250
void drainPendingUpstreamBodyMessages() {
1206-
while (true) {
1207-
ByteString body = null;
1208-
boolean triggerHalfClose = false;
1209-
synchronized (streamLock) {
1210-
if (super.isReady() && !pendingUpstreamBodyMessages.isEmpty()) {
1211-
body = pendingUpstreamBodyMessages.poll();
1251+
List<ByteString> toSend = new ArrayList<>();
1252+
boolean triggerHalfClose = false;
1253+
synchronized (streamLock) {
1254+
if (super.isReady()) {
1255+
ByteString body;
1256+
while ((body = pendingUpstreamBodyMessages.poll()) != null) {
1257+
toSend.add(body);
12121258
accumulatedWindowUpdateSidestreamToUpstream += body.size();
1213-
if (pendingUpstreamBodyMessages.isEmpty()
1214-
&& pendingUpstreamHalfClose.compareAndSet(true, false)) {
1215-
triggerHalfClose = true;
1216-
}
1259+
}
1260+
if (pendingUpstreamBodyMessages.isEmpty()
1261+
&& pendingUpstreamHalfClose.compareAndSet(true, false)) {
1262+
triggerHalfClose = true;
12171263
}
12181264
}
1219-
if (body == null) {
1220-
break;
1221-
}
1265+
}
1266+
for (ByteString body : toSend) {
12221267
super.sendMessage(new KnownLengthInputStream(body));
1268+
}
1269+
if (!toSend.isEmpty()) {
12231270
trySendAccumulatedWindowUpdates();
1224-
if (triggerHalfClose) {
1225-
if (requestSideClosed.compareAndSet(false, true)) {
1226-
proceedWithHalfClose();
1227-
}
1271+
}
1272+
if (triggerHalfClose) {
1273+
if (requestSideClosed.compareAndSet(false, true)) {
1274+
proceedWithHalfClose();
12281275
}
12291276
}
12301277
}
@@ -1258,18 +1305,58 @@ private void handleImmediateResponse(ImmediateResponse immediate, DataPlaneListe
12581305
}
12591306

12601307
private void drainPendingDrainingMessages() {
1308+
List<ByteString> mutatedToSend = new ArrayList<>();
1309+
List<ByteString> blockedToSend = new ArrayList<>();
1310+
List<InputStream> drainingToSend = new ArrayList<>();
1311+
boolean fullyDrained = false;
1312+
boolean triggerHalfClose = false;
1313+
12611314
synchronized (streamLock) {
1262-
InputStream msg;
1263-
while ((msg = pendingDrainingMessages.poll()) != null) {
1264-
super.sendMessage(msg);
1315+
fullyDrained = pendingUpstreamBodyMessages.isEmpty()
1316+
&& pendingRequestBodyMessages.isEmpty()
1317+
&& pendingDrainingMessages.isEmpty();
1318+
1319+
if (!fullyDrained && super.isReady()) {
1320+
ByteString body;
1321+
while ((body = pendingUpstreamBodyMessages.poll()) != null) {
1322+
mutatedToSend.add(body);
1323+
}
1324+
while ((body = pendingRequestBodyMessages.poll()) != null) {
1325+
blockedToSend.add(body);
1326+
}
1327+
InputStream msg;
1328+
while ((msg = pendingDrainingMessages.poll()) != null) {
1329+
drainingToSend.add(msg);
1330+
}
1331+
fullyDrained = pendingUpstreamBodyMessages.isEmpty()
1332+
&& pendingRequestBodyMessages.isEmpty()
1333+
&& pendingDrainingMessages.isEmpty();
12651334
}
1266-
passThroughMode.set(true);
1267-
if (pendingHalfClose.get()) {
1268-
if (requestSideClosed.compareAndSet(false, true)) {
1269-
proceedWithHalfClose();
1335+
1336+
if (fullyDrained) {
1337+
passThroughMode.set(true);
1338+
if (pendingHalfClose.get()) {
1339+
triggerHalfClose = true;
12701340
}
12711341
}
12721342
}
1343+
1344+
// Send messages outside the streamLock to prevent potential deadlocks
1345+
for (ByteString body : mutatedToSend) {
1346+
super.sendMessage(new KnownLengthInputStream(body));
1347+
}
1348+
for (ByteString body : blockedToSend) {
1349+
super.sendMessage(new KnownLengthInputStream(body));
1350+
}
1351+
for (InputStream msg : drainingToSend) {
1352+
super.sendMessage(msg);
1353+
}
1354+
1355+
if (triggerHalfClose) {
1356+
if (requestSideClosed.compareAndSet(false, true)) {
1357+
proceedWithHalfClose();
1358+
}
1359+
}
12731360
}
12741361

12751362
private void handleFailOpen(DataPlaneListener listener) {
@@ -1380,10 +1467,7 @@ void setImmediateResponse(Status status, Metadata trailers) {
13801467

13811468
@Override
13821469
public void onReady() {
1383-
dataPlaneClientCall.drainPendingUpstreamBodyMessages();
1384-
dataPlaneClientCall.trySendAccumulatedWindowUpdates();
1385-
dataPlaneClientCall.drainPendingRequests();
1386-
onReadyNotify();
1470+
dataPlaneClientCall.onReady();
13871471
}
13881472

13891473
@Override
@@ -1608,7 +1692,11 @@ void onExternalBody(ByteString body) {
16081692

16091693
void unblockAfterStreamComplete() {
16101694
proceedWithHeaders();
1695+
// 1. Drain mutated responses first
1696+
dataPlaneClientCall.drainPendingMutatedResponseBodiesDirect(this);
1697+
// 2. Drain raw responses
16111698
proceedWithSavedMessages();
1699+
// 3. Drain outbound requests
16121700
dataPlaneClientCall.drainPendingDrainingMessages();
16131701
proceedWithClose();
16141702
}

0 commit comments

Comments
 (0)