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

[feature/performance] Fail fast when doing remote transport calls inside incoming request contexts #1119

Merged
merged 5 commits into from
Nov 23, 2022

Conversation

tsmethurst
Copy link
Contributor

@tsmethurst tsmethurst commented Nov 22, 2022

Add a Fastfail context that can be passed in to external calls made by our httpclient transport.

The idea here is that sometimes we do outbound http requests inside an incoming request context (say when a user is doing a search for an account our instance doesn't know yet). In these cases, the caller should wrap the request passed to the dereferencing functions in a Fastfail context, which indicates to the transport logic that it should fail after the first attempt.

This PR prevents situations where the search fails but the transport keeps retrying, which makes the search request feel very slow and buggy. Now, fails after the first attempt in such situations will return immediately, making things feel a lot snappier for the client (even when the request fails -- at least they can try again immediately if they want).

@tsmethurst tsmethurst force-pushed the allow_http_transport_to_fail_fast branch from ee48b67 to 3012b1b Compare November 23, 2022 12:03
tsmethurst and others added 3 commits November 23, 2022 13:12
…ail instead of extra log entry

Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
@NyaaaWhatsUpDoc
Copy link
Member

Have made some small tweaks to make the context key checking in-line with intended use, and updated to wrap fast-fail errors instead of adding another line of log entry :). Aside from these very small tweaks all was looking good! So once tests are passing will merge this

@NyaaaWhatsUpDoc NyaaaWhatsUpDoc merged commit c9d893f into main Nov 23, 2022
@NyaaaWhatsUpDoc NyaaaWhatsUpDoc deleted the allow_http_transport_to_fail_fast branch November 23, 2022 21:40
@tsmethurst
Copy link
Contributor Author

Sweet!

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.

None yet

2 participants