Skip to content
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.

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

feat: make atServer address lookup resilient to transient failures to reach atDirectory #401

Merged
merged 5 commits into from
Sep 3, 2023

Conversation

gkc
Copy link
Contributor

@gkc gkc commented Sep 3, 2023

closes #368

- What I did

  • feat: Added retry behaviour to SecondaryUrlFinder. If root lookup fails with an exception, the lookup will be retried up to four times, with default delays of 50, 100, 150 and 200 after each failure.
  • feat: made retryDelaysMillis a public static variable in SecondaryUrlFinder; this allows clients to control (1) how many retries are done and (2) the delay after each subsequent retry

- How I did it
Note to reviewers: the 'real' changes are in cacheable_secondary_address_finder.dart and secondary_address_cache_test.dart. Changes to other test files are just the results of refactoring of some shared test code.

  • modified SecondaryUrlFinder so that it will retry up to 4 (number of retries is configurable) times with delays of [50, 100, 150, 200] milliseconds respectively (these delays are configurable) in the event of some exception while creating a connection to the atDirectory
  • modified SecondaryUrlFinder to allow injection of a socket factory, so we can test in unit tests
  • add unit tests for atDirectory retry behaviour
    • full lookup on a non-existent atDirectory
    • lookup on a mocked atDirectory with 0, 1, 2, 3, 4 or 5 simulated failures
  • docs: added some docs for SecondaryUrlFinder
  • feat: made retryDelaysMillis a public static varaiable in SecondaryUrlFinder; this allows clients to control (1) how many retries are done and (2) the delay after each subsequent retry
  • refactor unit tests
    • extract shared mock definitions into new file at_lookup_test_utils.dart
    • remove duplicates of the createMockSecureSocket function and moved into at_lookup_test_utils.dart
  • refactor: more refactoring and tidying of shared test code

- How to verify it

  • Existing and new at_lookup tests pass
  • at_client_sdk and at_server tests pass when using this at_lookup branch

gkc added 2 commits September 3, 2023 12:47
- refactor CacheableSecondaryAddressFinder to allow injection of an AtLookupSecureSocketFactory
- extract shared mock definitions into new file at_lookup_test_utils.dart
- remove duplicates of the `createMockSecureSocket` function and moved into at_lookup_test_utils.dart
@gkc gkc changed the title feat: resilience to transient failures to reach atDirectory feat: make atServer address lookup resilient to transient failures to reach atDirectory Sep 3, 2023
gkc added 2 commits September 3, 2023 15:25
docs: added some docs for SecondaryUrlFinder
refactor: reverted injection of secureSocketFactory to CacheableSecondaryAddressFinder because can already inject a SecondaryUrlFinder which can be injected with a secureSocketFactory
feat: made `retryDelaysMillis` a public static varaiable in SecondaryUrlFinder; this allows clients to control (1) how many retries are done and (2) the delay after each subsequent retry
refactor: more refactoring and tidying of shared test code
@gkc gkc marked this pull request as ready for review September 3, 2023 14:30
…, modified its constructor so its new `socketFactory` argument is named and optional
Copy link
Member

@cconstab cconstab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great...TY

@gkc gkc merged commit a1ffec7 into trunk Sep 3, 2023
9 checks passed
@gkc gkc deleted the feat/gkc-add-root-lookup-retry branch September 3, 2023 16:13
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.

address lookup requests to atDirectory (root) service should retry at least once upon failure
2 participants