interop-testing: fix wrong semantics for RPC failure stats in xDS test client (#7618)
The proto field is named as num_failures but its comment is saying it is for number of RPCs that failed to record a remote peer. RPC failed == RPC failed to record a remote peer was true previously (so no existing tests should be affected by this changed) as server completed RPCs immediately. It is no longer true with server capability to keep the call open/delayed.
This change clarifies the proto definition for stats RPC. rpcs_by_peer is for recording RPCs succeeded and num_failures is for RPCs failed. RPCs in the flight when the stats call times out are not counted towards any of the stats.
diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/XdsTestClient.java b/interop-testing/src/main/java/io/grpc/testing/integration/XdsTestClient.java
index 47cde98..1cb3b3e 100644
--- a/interop-testing/src/main/java/io/grpc/testing/integration/XdsTestClient.java
+++ b/interop-testing/src/main/java/io/grpc/testing/integration/XdsTestClient.java
@@ -326,7 +326,7 @@
@Override
public void onError(Throwable t) {
- handleRpcError(requestId, rpcType, hostnameRef.get(), savedWatchers);
+ handleRpcError(requestId, rpcType, savedWatchers);
}
@Override
@@ -347,7 +347,7 @@
if (printResponse) {
logger.log(Level.WARNING, "Rpc failed: {0}", t);
}
- handleRpcError(requestId, rpcType, hostnameRef.get(), savedWatchers);
+ handleRpcError(requestId, rpcType, savedWatchers);
}
@Override
@@ -395,8 +395,7 @@
notifyWatchers(watchers, rpcType, requestId, hostname);
}
- private void handleRpcError(long requestId, RpcType rpcType, String hostname,
- Set<XdsStatsWatcher> watchers) {
+ private void handleRpcError(long requestId, RpcType rpcType, Set<XdsStatsWatcher> watchers) {
synchronized (lock) {
Integer failedBase = rpcsFailedByMethod.get(rpcType.name());
if (failedBase == null) {
@@ -404,7 +403,7 @@
}
rpcsFailedByMethod.put(rpcType.name(), failedBase + 1);
}
- notifyWatchers(watchers, rpcType, requestId, hostname);
+ notifyWatchers(watchers, rpcType, requestId, null);
}
}
@@ -508,7 +507,7 @@
private final EnumMap<RpcType, Map<String, Integer>> rpcsByTypeAndPeer =
new EnumMap<>(RpcType.class);
private final Object lock = new Object();
- private int noRemotePeer;
+ private int rpcsFailed;
private XdsStatsWatcher(long startId, long endId) {
latch = new CountDownLatch(Ints.checkedCast(endId - startId));
@@ -539,7 +538,7 @@
rpcsByTypeAndPeer.put(rpcType, rpcMap);
}
} else {
- noRemotePeer += 1;
+ rpcsFailed += 1;
}
latch.countDown();
}
@@ -565,7 +564,7 @@
rpcs.putAllRpcsByPeer(entry.getValue());
builder.putRpcsByMethod(getRpcTypeString(entry.getKey()), rpcs.build());
}
- builder.setNumFailures(noRemotePeer + (int) latch.getCount());
+ builder.setNumFailures(rpcsFailed);
}
return builder.build();
}
diff --git a/interop-testing/src/main/proto/grpc/testing/messages.proto b/interop-testing/src/main/proto/grpc/testing/messages.proto
index 797adb7..81724a5 100644
--- a/interop-testing/src/main/proto/grpc/testing/messages.proto
+++ b/interop-testing/src/main/proto/grpc/testing/messages.proto
@@ -191,15 +191,15 @@
}
message LoadBalancerStatsResponse {
- // The number of completed RPCs for each peer.
+ // The number of RPCs succeeded for each peer.
map<string, int32> rpcs_by_peer = 1;
- // The number of RPCs that failed to record a remote peer.
+ // The number of RPCs that failed.
int32 num_failures = 2;
message RpcsByPeer {
- // The number of completed RPCs for each peer.
+ // The number of RPCs succeeded for each peer.
map<string, int32> rpcs_by_peer = 1;
}
- // The number of completed RPCs for each type (UnaryCall or EmptyCall).
+ // The number of RPCs succeeded for each type (UnaryCall or EmptyCall).
map<string, RpcsByPeer> rpcs_by_method = 3;
}
