Respect SDK flags already present in history regardless of server capability detection.#2936
Merged
Conversation
Sushisource
reviewed
Jul 1, 2026
Sushisource
left a comment
Sushisource
left a comment
Member
There was a problem hiding this comment.
Overall semantics make sense to me but will defer to @maciejdudko for more detailed review
|
|
||
| final class GrpcCompressionInterceptor implements ClientInterceptor { | ||
| private final GrpcCompression compression; | ||
| private final Set<String> compressionUnsupportedMethods = |
Member
There was a problem hiding this comment.
Needs to be protected for thread safety
maciejdudko
approved these changes
Jul 1, 2026
Comment on lines
24
to
26
| * @return True, as long as the server supports SDK flags | ||
| * @return True. | ||
| */ | ||
| public boolean setSdkFlag(SdkFlag flag) { |
Contributor
There was a problem hiding this comment.
Nit: arguably this method should return void. (Return value is unused anyway.)
Comment on lines
47
to
49
Contributor
There was a problem hiding this comment.
Nit: this will always return false now.
| } | ||
| } | ||
|
|
||
| static boolean isGetSystemInfoUnknownMethod(Status status) { |
Contributor
There was a problem hiding this comment.
Nit: add @VisibleForTesting annotation
Add a replay regression test that routes WorkflowService calls through a small proxy which returns UNIMPLEMENTED for GetSystemInfo and forwards all other RPCs. The test injects VERSION_WAIT_FOR_MARKER into a known interleaved getVersion history, verifies the history replays without the proxy, then expects the same history to replay through the proxy. On current buggy code this fails with TMPRL1100 because default capabilities disable SDK metadata support and mask the recorded SDK flag.
not found, rather than compression
Retry gzip-compressed unary RPCs once without request compression when the server returns UNIMPLEMENTED with compression-related error text. Cache the affected gRPC method so subsequent calls to that method use identity compression, while unrelated RPCs continue using gzip. This mirrors sdk-go's per-RPC compression negotiation and avoids treating proxy/server gzip support failures as missing GetSystemInfo capability data. Keep generic UNIMPLEMENTED errors unchanged. Add service-client tests covering downgrade, method-level caching, opt-out behavior when compression is NONE, and generic UNIMPLEMENTED errors that must not trigger downgrade.
eamsden
force-pushed
the
ea/respect-the-flag
branch
from
f7e7a11 to
cfdadd3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
Respect SDK flags already present in history regardless of server capability detection.
Only default
GetSystemInfocapabilities for true unknown-methodUNIMPLEMENTEDerrors.Retry gzip-compressed unary RPCs once without compression on compression-related
UNIMPLEMENTEDerrors, then cache that downgrade per RPC method.Why?
A gzip-incompatible gRPC endpoint or proxy can return
UNIMPLEMENTEDforGetSystemInfo. That should not be confused with an old server missing the RPC, and default capabilities should not mask SDK flags already recorded in history.Checklist
Closes N/A.
How was this tested:
Added service-client coverage for per-RPC gzip downgrade, method-level downgrade caching,
GrpcCompression.NONE, and genericUNIMPLEMENTEDerrors that must not downgrade.Updated capability tests to distinguish unknown-method
GetSystemInfofailures from gzip/decompressionUNIMPLEMENTEDfailures.Added replay regression coverage for histories whose SDK flags must still be respected when
GetSystemInfofalls back to default capabilities.No external docs updates. Javadocs for gRPC compression were updated.