Skip to content

Don't use BackoffThrottler in multithread contexts#1467

Merged
mjameswh merged 4 commits intotemporalio:masterfrom
mjameswh:grpc-retry-refactor
Oct 13, 2022
Merged

Don't use BackoffThrottler in multithread contexts#1467
mjameswh merged 4 commits intotemporalio:masterfrom
mjameswh:grpc-retry-refactor

Conversation

@mjameswh
Copy link
Copy Markdown
Contributor

@mjameswh mjameswh commented Sep 30, 2022

What was changed

  • Avoid using BackoffThrottler instances in multi-thread contexts (Poller)
  • Refactor GrpcAsyncRetryer to carry its state in fields instead of passing them as arguments
  • Pass the appropriate ScheduledExecutor to GrpcAsyncRetryer, instead of having a global one
  • Modify Grpc*RetryerTest to more strongly validate either the test is expected to retry, and how long these retries should take (checking for either max or min possible time)

Why?

  • BackoffThrottler was not designed to provide an external thread safe semantic; consequently, a race condition between several threads could result in skipping throttling or in throttling more than expected.
  • These changes will make it much easier to implement longer backoff time on RESOURCE_EXHAUSTION failures.

@mjameswh mjameswh marked this pull request as ready for review September 30, 2022 20:09
@mjameswh mjameswh requested a review from Spikhalskiy October 12, 2022 20:43
@mjameswh mjameswh merged commit cea3e73 into temporalio:master Oct 13, 2022
@mjameswh mjameswh deleted the grpc-retry-refactor branch October 13, 2022 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants