You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fix backward compatibility issue with interrupt shuffle by checking if the reason is nonEmpty.
Why are the changes needed?
If someone uses a new client with old server which is not sending CheckQuotaResponse in HeartbeatFromApplicationResponse then proto uses default value to build CheckQuotaResponse with isAvailable=false and reason="". In this case the job will always fails without breaching the quota, we should not fail the job if the reason is empty to make it backward compatible.
Using reason.nonEmpty as a proxy for "server supports quota check" is fragile — reason is a business description field, not a compatibility signal. If a legitimate quota-exceeded response happens to have an empty reason, it would be silently ignored; conversely, any future change to the reason format could break this logic.
Since quotaInterruptShuffleEnabled defaults to false, users must explicitly opt in. In a new-client/old-server scenario, users should simply not enable this config. This is an explicit and semantically clear approach, rather than relying on an implicit convention around the reason field.
If we do need robust backward compatibility, a better approach would be to add an explicit capability field (e.g., quotaCheckSupported) to the heartbeat response. But that's a larger change and can be addressed separately.
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
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 changes were proposed in this pull request?
Fix backward compatibility issue with interrupt shuffle by checking if the reason is nonEmpty.
Why are the changes needed?
If someone uses a new client with old server which is not sending
CheckQuotaResponseinHeartbeatFromApplicationResponsethen proto uses default value to build CheckQuotaResponse with isAvailable=false and reason="". In this case the job will always fails without breaching the quota, we should not fail the job if the reason is empty to make it backward compatible.Does this PR resolve a correctness bug?
No
Does this PR introduce any user-facing change?
No
How was this patch tested?
Tested in local setup.