Skip to content

refactor: [shard-distributor]Handle context.Cancelled errors #7493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

refactor: [shard-distributor]Handle context.Cancelled errors #7493

wants to merge 6 commits into from
+33 −0

Conversation

@gazi-yestemirova
Copy link
Contributor

@gazi-yestemirova gazi-yestemirova commented Nov 26, 2025 •
edited
Loading

What changed?

  • executorstore.GetState now returns ctx.Err() when etcd calls are aborted by context cancellation, preventing “get executor data” from being logged as an internal failure.
  • runRebalancingLoop, runShardStatsCleanupLoop, and rebalanceShardsImpl checks if the err is context.Cancelled or DeadlineExceeded

Why?
context cancellations are expected when leadership is changed or service is stopped, but they are treated as internal errors and it is polluting the logs. So we are ignoring cancellation errors while maintaining the genuine errors visibility.

How did you test it?
unit-tests

Potential risks

Release notes

Documentation Changes

@gazi-yestemirova gazi-yestemirova changed the title Store err logs refactor: [shard-distributor]Remove error logs from store level Nov 28, 2025
gazi-yestemirova and others added 5 commits November 28, 2025 10:44
@gazi-yestemirova
[shard-distributor]Remove error logs from store level
07c98ce 
Signed-off-by: Gaziza Yestemirova <[email protected]>
@gazi-yestemirova
update tests
6b28caa 
Signed-off-by: Gaziza Yestemirova <[email protected]>
@eleonoradgr @gazi-yestemirova
fix(shard-distributor): remove trimming of prefixes (cadence-workflow…
a68b74f 
…#7490)

<!-- Describe what has changed in this PR -->
**What changed?**
Reverting the trimprefix since we are using constants to compare the
values that include that

<!-- Tell your future self why have you made these changes -->
**Why?**
Constants that include the prefix are used to

<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Deployed in staging

<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
Corruption of db, which is already the case.

<!-- Is it notable for release? e.g. schema updates, configuration or
data migration required? If so, please mention it, and also update
CHANGELOG.md -->
**Release notes**

<!-- Is there any documentation updates should be made for config,
https://cadenceworkflow.io/docs/operation-guide/setup/ ? If so, please
open an PR in https://github.com/cadence-workflow/cadence-docs -->
**Documentation Changes**

---------

Signed-off-by: edigregorio <[email protected]>
Signed-off-by: Gaziza Yestemirova <[email protected]>
@jakobht @gazi-yestemirova
feat(shard-distributor): return executor metadata from spectator GetS…
b94764d 
…hardOwner (cadence-workflow#7476)

**What changed?**
Changed `GetShardOwner` to return an `ExecutorOwnership` struct
containing both executor ID and metadata map, instead of just the
executor ID string.
Also adds a Spectators group so we can easily pass around all
spectators.
**Why?**
Enables callers to access additional executor information like gRPC
address for peer routing, without requiring separate lookups. This is
needed for implementing canary peer chooser that routes requests to
executors based on their addresses.

**How did you test it?**
Updated all tests to verify metadata is included in responses. Verified
locally that ownership information includes metadata.

**Potential risks**
Low - this is an API enhancement that maintains backward compatibility
by returning the same executor ID, just with additional metadata.

**Release notes**

**Documentation Changes**
None

---------

Signed-off-by: Jakob Haahr Taankvist <[email protected]>
Signed-off-by: Gaziza Yestemirova <[email protected]>
@gazi-yestemirova
handle context cancelled
af3b6fa 
Signed-off-by: Gaziza Yestemirova <[email protected]>
@gazi-yestemirova gazi-yestemirova changed the title refactor: [shard-distributor]Remove error logs from store level refactor: [shard-distributor]Handle context.Cancelled errors Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Shaddoll Shaddoll Awaiting requested review from Shaddoll Shaddoll is a code owner

@neil-xie neil-xie Awaiting requested review from neil-xie neil-xie is a code owner

@davidporter-id-au davidporter-id-au Awaiting requested review from davidporter-id-au davidporter-id-au is a code owner

@shijiesheng shijiesheng Awaiting requested review from shijiesheng shijiesheng is a code owner

@jakobht jakobht Awaiting requested review from jakobht jakobht is a code owner

@3vilhamster 3vilhamster Awaiting requested review from 3vilhamster 3vilhamster is a code owner

@sankari165 sankari165 Awaiting requested review from sankari165 sankari165 is a code owner

@dkrotx dkrotx Awaiting requested review from dkrotx dkrotx is a code owner

@taylanisikdemir taylanisikdemir Awaiting requested review from taylanisikdemir taylanisikdemir is a code owner

@demirkayaender demirkayaender Awaiting requested review from demirkayaender demirkayaender is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gazi-yestemirova @eleonoradgr @jakobht