MCS connection scaling interop tests for Java#12651
Conversation
| return; | ||
| } | ||
| if (new String(request.getPayload().getBody().toByteArray(), UTF_8) | ||
| .equals(MCS_CS.description())) { |
There was a problem hiding this comment.
Please no. There's two regular approaches we use for tweaking the server's behavior:
- A field in the request proto (e.g.,
fill_username) - Header metadata. I don't think we actually use this approach in the vanilla interop today, but we do do it for psm interop
(fill_username and the like are because of mistakes in the past where grpc-java verified the result message exactly. That means you can't add new fields to have them be ignored. We did want to verify the result was what we expected, but it would have probably been better to just verify the payload. But the behavior hasn't been changed, so we still need these knobs that enable result fields.)
There was a problem hiding this comment.
Added new fields in request and response for setting the client socket address.
…ting." This reverts commit 3719011.
|
I realized I forgot to push the commits addressing review comments last week. |
| .asRuntimeException()); | ||
| return; | ||
| } | ||
| if (whetherSendClientSocketAddressInResponse(request)) { |
There was a problem hiding this comment.
So we just ignore most of the request? It looks like this should go through toChunkQueue() to actually send the response.
There was a problem hiding this comment.
The other parts of the request are about chunking behavior which doesn't apply for this test. I don't need to use toChunkQueue which is used to keep track of what response sizes to send in successive chunks, nor dispatcher.enqueue which handles scheduled sending of the chunks.
There was a problem hiding this comment.
There is no "this test" on the server-side. All features are supported simultaneously. We should plumb it through the Chunk/toResponse().
There was a problem hiding this comment.
I might as well have said: The other parts of the request are about chunking behavior which doesn't apply when the request message indicates that client socket address needs to be sent in the response.
It is just service code branching based on how the particular request needs to be handled.
There was a problem hiding this comment.
I realized my reply didn't address the all features simultaneously part. I have plumbed it via chunked response handling now.
| @SuppressWarnings("AddressSelection") | ||
| @VisibleForTesting | ||
| void start() throws Exception { | ||
| System.out.println("TestServiceServer.start called with addressType " + addressType); |
There was a problem hiding this comment.
Revert "Add temp debug stmts for server start."?
| .asRuntimeException()); | ||
| return; | ||
| } | ||
| if (whetherSendClientSocketAddressInResponse(request)) { |
There was a problem hiding this comment.
There is no "this test" on the server-side. All features are supported simultaneously. We should plumb it through the Chunk/toResponse().
…dropping intermediary commit with debug statements). # Conflicts: # interop-testing/src/main/java/io/grpc/testing/integration/TestServiceServer.java
kannanjgithub
force-pushed
the
mcs-interop-tests
branch
from
809ae0f to
12f35cf
Compare
# Conflicts: # interop-testing/src/main/java/io/grpc/testing/integration/TestServiceImpl.java
…Impl
Refactor `fullDuplexCall` in `TestServiceImpl` to route peer socket address
requests through the `toChunkQueue()` pipeline and the `ResponseDispatcher`.
This replaces the previous approach of manually branching and bypassing
the standard chunk streaming flow.
Specifically:
- Retrieve the peer's socket address on the request-handling thread inside
`toChunkQueue()` to prevent context loss on the background scheduling thread.
- Overload the `Chunk` class constructor to accept a nullable `peerSocketAddress`
while keeping the default constructor for backward compatibility.
- Ensure `Chunk.toResponse()` only populates the response `payload` field if
`length > 0`, preserving the exact wire-format for empty/address-only requests.
- Remove the manual branch on `whetherSendClientSocketAddressInResponse` and its
associated helper method, simplifying `fullDuplexCall`.
kannanjgithub
force-pushed
the
mcs-interop-tests
branch
from
3a232aa to
6a7bf67
Compare
Corresponding changes in (common) Java server for interop test: grpc/grpc-java#12651 Closes #41664 COPYBARA_INTEGRATE_REVIEW=#41664 from kannanjgithub:mcs 64293f6 PiperOrigin-RevId: 930078263
2 participants
No description provided.