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.

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

Rewrite networking to have proper round-robin across instances and retries #218

Merged
merged 6 commits into from
Sep 16, 2024
Merged

Rewrite networking to have proper round-robin across instances and retries #218

merged 6 commits into from
Sep 16, 2024

Conversation

sosthene-nitrokey
Copy link
Contributor

@sosthene-nitrokey sosthene-nitrokey commented Sep 5, 2024 •
edited
Loading

This PR:

  • Adds state for each instance in a slot. This state stores information on whether the instance is is a "failed" state or not
  • On failure (IO or 5xx or 412), mark the instance as failed
  • When an instance is marked as failed and threads are allowed, spawn a background thread to retry it with some backoff.
  • Don't use failed instances
  • If all instances are failed, try failed instances anyway (this will especially happen in the case where spawning threads is not allowed).
  • On IO failure, remove all the idle connections

Depends on:

@sosthene-nitrokey sosthene-nitrokey force-pushed the rewrite-networking branch 3 times, most recently from ab93224 to 334812d Compare September 10, 2024 09:03
@sosthene-nitrokey sosthene-nitrokey marked this pull request as ready for review September 13, 2024 09:37
@sosthene-nitrokey
LoginCtx: Store reference to the entire slot
e8e5774 
This will allow keeping track of the "bad" status in each instance
This also makes the round-robin real for the slots, not just for the session
This also rejects slots with no instances
@sosthene-nitrokey
Remove unrequired LoginCtx clones
014bdaa
@sosthene-nitrokey
Store metadata with the instance configuration
f10604d
@sosthene-nitrokey
Run regular retries for new failed instances
366c3f9
@sosthene-nitrokey
Fix retry count
94b8838 
Retry count should only count retries, not the number of attempts.
A retry count of 0  should still mean 1 connection attempt.

A retry count of N should mean 1 attempt + N retry attempt
@sosthene-nitrokey
Clear all connections pool on IO errors
69abbac
@sosthene-nitrokey
Copy link
Contributor Author

In the single threaded case it would probably be nice to perform a health-check request with a very short timeout (1s) regularly even if it means slowing down the "real" request.

@sosthene-nitrokey sosthene-nitrokey merged commit ff3b7de into main Sep 16, 2024
6 checks passed
@sosthene-nitrokey sosthene-nitrokey deleted the rewrite-networking branch September 16, 2024 09:37
This was referenced Sep 16, 2024
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daringer daringer daringer approved these changes

@ansiwen ansiwen Awaiting requested review from ansiwen

@robin-nitrokey robin-nitrokey Awaiting requested review from robin-nitrokey

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sosthene-nitrokey @daringer