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
7.96k stars 772 forks source link

Switch quotas.Collection to clock.Ratelimiter, minor fixes along the way #6126

Closed Groxx closed 2 weeks ago

Groxx commented 2 weeks ago

Partly this is worth doing at some point regardless, and partly this is a bit of a blocker for the global ratelimiter, as it needs to expose some Reservation-using APIs to fit into existing ratelimiter code.

This has a larger blast-radius than I'd like, as it's not just frontend, but it should be safe - the clock.Ratelimiter is pretty thoroughly tested, and the small changes in behavior seem in line with what the code around it expects to happen, rather than what actually happens.

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.

Project coverage is 72.21%. Comparing base (080dec9) to head (d2f6889). Report is 5 commits behind head on master.

Additional details and impacted files | [Files](https://app.codecov.io/gh/uber/cadence/pull/6126?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber) | Coverage Δ | | |---|---|---| | [common/clock/ratelimiter.go](https://app.codecov.io/gh/uber/cadence/pull/6126?src=pr&el=tree&filepath=common%2Fclock%2Fratelimiter.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber#diff-Y29tbW9uL2Nsb2NrL3JhdGVsaW1pdGVyLmdv) | `100.00% <100.00%> (ø)` | | | [common/quotas/dynamicratelimiter.go](https://app.codecov.io/gh/uber/cadence/pull/6126?src=pr&el=tree&filepath=common%2Fquotas%2Fdynamicratelimiter.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber#diff-Y29tbW9uL3F1b3Rhcy9keW5hbWljcmF0ZWxpbWl0ZXIuZ28=) | `75.00% <100.00%> (ø)` | | | [common/quotas/dynamicratelimiterfactory.go](https://app.codecov.io/gh/uber/cadence/pull/6126?src=pr&el=tree&filepath=common%2Fquotas%2Fdynamicratelimiterfactory.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber#diff-Y29tbW9uL3F1b3Rhcy9keW5hbWljcmF0ZWxpbWl0ZXJmYWN0b3J5Lmdv) | `100.00% <ø> (ø)` | | | [common/quotas/multistageratelimiter.go](https://app.codecov.io/gh/uber/cadence/pull/6126?src=pr&el=tree&filepath=common%2Fquotas%2Fmultistageratelimiter.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber#diff-Y29tbW9uL3F1b3Rhcy9tdWx0aXN0YWdlcmF0ZWxpbWl0ZXIuZ28=) | `88.23% <100.00%> (+10.45%)` | :arrow_up: | | [common/quotas/ratelimiter.go](https://app.codecov.io/gh/uber/cadence/pull/6126?src=pr&el=tree&filepath=common%2Fquotas%2Fratelimiter.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber#diff-Y29tbW9uL3F1b3Rhcy9yYXRlbGltaXRlci5nbw==) | `52.94% <80.00%> (ø)` | | | [service/matching/tasklist/matcher.go](https://app.codecov.io/gh/uber/cadence/pull/6126?src=pr&el=tree&filepath=service%2Fmatching%2Ftasklist%2Fmatcher.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber#diff-c2VydmljZS9tYXRjaGluZy90YXNrbGlzdC9tYXRjaGVyLmdv) | `81.16% <87.50%> (+1.24%)` | :arrow_up: | ... and [14 files with indirect coverage changes](https://app.codecov.io/gh/uber/cadence/pull/6126/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/6126?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/6126?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber). Last update [080dec9...d2f6889](https://app.codecov.io/gh/uber/cadence/pull/6126?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).
coveralls commented 2 weeks ago

Pull Request Test Coverage Report for Build 0190054b-dbbb-47f0-a6b6-f76fdd71f23d

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
service/matching/tasklist/matcher.go 6 12 50.0%
<!-- Total: 17 23 73.91% -->
Files with Coverage Reduction New Missed Lines %
common/task/weighted_round_robin_task_scheduler.go 2 89.05%
service/history/task/fetcher.go 2 85.57%
service/history/task/transfer_standby_task_executor.go 3 86.33%
service/history/task/task.go 3 84.81%
common/task/fifo_task_scheduler.go 3 82.47%
service/history/queue/timer_queue_processor_base.go 4 77.66%
service/matching/tasklist/matcher.go 4 88.04%
service/matching/tasklist/task_list_manager.go 5 76.05%
service/history/execution/cache.go 6 74.61%
service/history/task/cross_cluster_task_processor.go 8 80.79%
<!-- Total: 40 -->
Totals Coverage Status
Change from base Build 018ff521-a453-4f25-a2e8-6d1d59e59c06: -0.01%
Covered Lines: 106456
Relevant Lines: 149076

💛 - Coveralls
coveralls commented 2 weeks ago

Pull Request Test Coverage Report for Build 01900dbd-fce7-4eba-9b1d-30b9f434ee15

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
service/matching/tasklist/matcher.go 7 13 53.85%
<!-- Total: 18 24 75.0% -->
Files with Coverage Reduction New Missed Lines %
common/task/weighted_round_robin_task_scheduler.go 2 89.05%
service/matching/tasklist/task_reader.go 2 77.72%
service/history/queue/timer_queue_processor_base.go 3 77.87%
service/matching/tasklist/task_list_manager.go 5 76.45%
service/matching/tasklist/matcher.go 6 87.27%
<!-- Total: 18 -->
Totals Coverage Status
Change from base Build 018ff521-a453-4f25-a2e8-6d1d59e59c06: 0.02%
Covered Lines: 106500
Relevant Lines: 149075

💛 - Coveralls
coveralls commented 2 weeks ago

Pull Request Test Coverage Report for Build 01900e68-dd59-4ac5-9ae7-57954b7453dd

Details


Files with Coverage Reduction New Missed Lines %
common/task/weighted_round_robin_task_scheduler.go 2 89.05%
common/peerprovider/ringpopprovider/config.go 2 81.58%
tools/cli/admin_db_decode_thrift.go 2 70.51%
service/frontend/api/handler.go 2 75.6%
service/history/queue/timer_queue_processor_base.go 3 77.87%
common/archiver/filestore/historyArchiver.go 4 80.95%
service/matching/tasklist/task_list_manager.go 5 76.05%
<!-- Total: 20 -->
Totals Coverage Status
Change from base Build 018ff521-a453-4f25-a2e8-6d1d59e59c06: 0.01%
Covered Lines: 106500
Relevant Lines: 149081

💛 - Coveralls