search

uber / cadence

Cadence is a distributed, scalable, durable, and highly available orchestration engine to execute asynchronous long-running business logic in a scalable and resilient way.
https://cadenceworkflow.io
MIT License
8.22k stars 792 forks source link

Added a defensive check in the getTaskListManager function #6199

Closed jakobht closed 2 months ago

jakobht commented 2 months ago

What changed? Added a check whether or not we should own a shard before we steal it.

Why? This makes sure we do not steal tasklists we do not own according to the hashring.

How did you test it? Unit tests, local runs, and deployment in dev environments. I didn't see any ill effects from the change.

Potential risks This code path is quite busy, so introducing a check might have performance impacts.

There is a risk that correct requests are rejected because the matching host has a wrong view of the hash ring. I expect this to be transient, and everything should be retried.

We introduce a new error, I have checked the callers and they should all handle unknown errors.

Release notes

Documentation Changes

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 72.90%. Comparing base (9a7a8a4) to head (b659dda). Report is 9 commits behind head on master.

Additional details and impacted files | [Files](https://app.codecov.io/gh/uber/cadence/pull/6199?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber) | Coverage Δ | | |---|---|---| | [service/matching/handler/engine.go](https://app.codecov.io/gh/uber/cadence/pull/6199?src=pr&el=tree&filepath=service%2Fmatching%2Fhandler%2Fengine.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber#diff-c2VydmljZS9tYXRjaGluZy9oYW5kbGVyL2VuZ2luZS5nbw==) | `77.99% <100.00%> (+0.84%)` | :arrow_up: | ... and [26 files with indirect coverage changes](https://app.codecov.io/gh/uber/cadence/pull/6199/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/uber/cadence/pull/6199?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/uber/cadence/pull/6199?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber). Last update [9a7a8a4...b659dda](https://app.codecov.io/gh/uber/cadence/pull/6199?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber).