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

Consider adding Progress/Cancellation to reconnect loop #9560

Closed
markfields opened this issue Mar 22, 2022 · 3 comments
Closed

Consider adding Progress/Cancellation to reconnect loop #9560

markfields opened this issue Mar 22, 2022 · 3 comments
Labels
area: loader Loader related issues triage
Milestone

Comments

@markfields
Copy link
Member

I'm considering whether hosts would benefit from progress/cancelation in the connectCore loop, e.g. when thinking about this bug (server outage caused JoinSession to return 500 for a few hours).

We discussed this a bit in #8411 (comment), and it seems to me better to let the host define the policy, leveraging any driver-specific knowledge it has coupled with the error(s) being repeatedly hit. See #9558 for thoughts about formalizing what driver-specific info a host could pull from an ODSP driver error (that the Loader layer wouldn't be able to leverage).

@vladsud
Copy link
Contributor

vladsud commented Mar 23, 2022

I'd love to better understand programing model here. In other places (where there is clear request/response pattern in the form of API call) it's obvious how to provide cancelation and progress reporting.
With connection flow, most of the connection APIs (see https://github.com/microsoft/FluidFramework/pull/9439/files) are fire-and-forget. And even if they were not, there are implicit flows (connect on boot) where we do not return anything to consumers.
But consumers do want to have a way to know if 429 happens and connection is essentially not progressing.

500 cases seems like from slightly different bucket, but it has similar requirements - give ability to host to react to abnormal patterns (continuous reconnects, as an example). I think that's fine, but I'd probably not tight it together with progress / cancellation, we do not have to use same pattern here.

@markfields
Copy link
Member Author

markfields commented May 16, 2022

This issue applies to part of the overarching design discussed here: #9677

@vladsud
Copy link
Contributor

vladsud commented Jun 21, 2022

@markfields, feel free to port to ADO, but we are closing pretty much all github issues.
If you do port, please answer my question above :)

@vladsud vladsud closed this as completed Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: loader Loader related issues triage
Projects
None yet
Development

No branches or pull requests

2 participants