chore(shard-distirbutor): reuse the state for assignephemeral shards#8020
Open
eleonoradgr wants to merge 1 commit intocadence-workflow:masterfrom
Open
chore(shard-distirbutor): reuse the state for assignephemeral shards#8020eleonoradgr wants to merge 1 commit intocadence-workflow:masterfrom
eleonoradgr wants to merge 1 commit intocadence-workflow:masterfrom
Conversation
Signed-off-by: edigregorio <edigregorio@uber.com>
CI failed: Integration tests are failing with fatal memory-related panics (WASM out of bounds and stack overflow) in the SQLite driver, likely due to resource exhaustion under test concurrency.OverviewIntegration tests for SQLite are consistently failing due to severe memory and stack exhaustion issues within the FailuresSQLite Integration Test Panics (confidence: medium)
Summary
Code Review ✅ ApprovedRefactors the shard distributor to reuse state for assigned ephemeral shards. No issues found. Rules
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
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
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 changed?
The fetchExecutorMetadata method (which made one GetExecutor storage call per unique assigned executor after writing new shard assignments) was replaced with a pure in-memory function
buildExecutorOwners. Instead of going back to storage, it now derives the executor metadata directly from the HeartbeatState already loaded by the preceding GetState call.
part of #6862
Why?
The GetState call already fetches the full executor heartbeat state, including the metadata needed for the response. The previous code discarded that data and then re-fetched it from storage
once per executor — wasting N extra round-trips per assignEphemeralBatch invocation. This change eliminates those redundant reads: fewer storage calls means lower latency, less load on etcd.
How did you test it?
go test -race ./service/sharddistributor/handler/...
Potential risks
tested in local development, do not expect any issue
Release notes
N/A
Documentation Changes
N/A