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

Try to assert that audience is superset of quorum #5251

Closed
vladsud opened this issue Feb 24, 2021 · 5 comments
Closed

Try to assert that audience is superset of quorum #5251

vladsud opened this issue Feb 24, 2021 · 5 comments
Assignees
Labels
design-required This issue requires design thought status: stale
Milestone

Comments

@vladsud
Copy link
Contributor

vladsud commented Feb 24, 2021

We run into a case where there was a client in quorum that never left (i.e. there was no leave op for this client even though there was no such client connected via websocket). The issue is fixed on server side, but it would be great to have client side telemetry to find these cases.

It was easy to spot it - that same client was missing from Audience.
At the same it was hard to say what's wrong, as summaries were not going at all, but presence well was not showing any users, so it felt like client summarizing logic was broken. Overall, it was totally broken from user POV (summaries not going).

Note that Audience is superset of quorum, but due to async nature of populating audience, we can't easily assert that. But if we can track users over long period of time, then we should be able to assert users that are not showing up in audience for very long time.
In addition, it might be great to merge these two data structures in terms of presentation layer to consumers. It's very confusing for people to have two, have async population, have self missing from audience on raising connected event, etc.

@vladsud vladsud added the bug Something isn't working label Feb 24, 2021
@vladsud vladsud added this to the March 2021 milestone Feb 24, 2021
@vladsud vladsud self-assigned this Feb 24, 2021
@vladsud vladsud modified the milestones: March 2021, April 2021 Feb 26, 2021
@vladsud vladsud modified the milestones: April 2021, May 2021 Mar 22, 2021
@markfields
Copy link
Member

I'll add this back to Reliability I guess. This issue needs a lot more clarity... but it sounds like the point is to add better detection of an invalid state to fail faster, which fits in the Reliability Workstream.

@markfields
Copy link
Member

@vladsud Can we focus this one on the assert you mention in the title, and track the bigger design question around quorum/audience in a separate place? Trying to make sure stuff in the Reliability Workstream is crystal clear.

@vladsud
Copy link
Contributor Author

vladsud commented Mar 31, 2021

I do not think it will be possible, simply because Audience & Quorum represent different points in time.
So partially that's by design. However this design is super confusing to consumers and they do not have good way of dealing with it today, especially with async nature to Audience relative to other things (like connected events, population of self).
So I think we need to clearly articulate where we want to get to in long run, and then figure out what we should do in short terms.

@markfields
Copy link
Member

Ok, yes that clear articulation is what I'm asking for, and if we don't have it then I'm marking this as design-required and removing the "bug" label.

@markfields markfields added design-required This issue requires design thought and removed bug Something isn't working labels Mar 31, 2021
@markfields markfields changed the title Assert that audience is superset of quorum Try to assert that audience is superset of quorum Mar 31, 2021
@vladsud vladsud modified the milestones: May 2021, June 2021 Apr 22, 2021
@vladsud vladsud modified the milestones: June 2021, Next May 20, 2021
@ghost ghost added the status: stale label Nov 16, 2021
@ghost
Copy link

ghost commented Nov 16, 2021

This issue has been automatically marked as stale because it has had no activity for 180 days. It will be closed if no further activity occurs within 8 days of this comment. Thank you for your contributions to Fluid Framework!

@ghost ghost closed this as completed Nov 24, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-required This issue requires design thought status: stale
Projects
None yet
Development

No branches or pull requests

2 participants