view-only mode should issue "connected" event on current clientID being in audience

[vladsud]

We should also double-check that "write" mode also follows same requirements.
Once done, we should follow up and remove code on Bohemia side working around this issue.

More background:

From: Tony Murphy Anthony.Murphy@microsoft.com
Sent: Tuesday, April 7, 2020 10:09 AM
To: Vlad Sudzilouski vladsud@exchange.microsoft.com; Tanvir Aumi mdaumi@microsoft.com
Subject: RE: Readonly and Audience

The change made to work around is pretty complicated for something I think should be trivial. I really don’t want all our partners to build similar logic just to find the current user:
https://office.visualstudio.com/OC/_git/74031860-e0cd-45a1-913f-10bbf3f82555/pullrequest/446248?_a=overview

From: Tony Murphy
Sent: Tuesday, April 7, 2020 10:03 AM
To: Vlad Sudzilouski vladsud@exchange.microsoft.com; Tanvir Aumi mdaumi@microsoft.com
Subject: RE: Readonly and Audience

That would be great too. I’m not really proposing a specific solution, just that I think we need on here. Another idea I have was a “connected”-like event on the audience that fires when your client show up in the audience.

-Tony

From: Vlad Sudzilouski vladsud@exchange.microsoft.com
Sent: Tuesday, April 7, 2020 9:26 AM
To: Tony Murphy Anthony.Murphy@microsoft.com; Tanvir Aumi mdaumi@microsoft.com
Subject: RE: Readonly and Audience

We can. But I think it’s even better if it’s handled as first class by server (see attached)

From: Tony Murphy Anthony.Murphy@microsoft.com
Sent: Tuesday, April 7, 2020 9:11 AM
To: Vlad Sudzilouski vladsud@exchange.microsoft.com; Tanvir Aumi mdaumi@microsoft.com
Subject: Readonly and Audience

Hey Guys,

I was thinking about this, and I really don’t like the proposed solution for getting the current user for the audience. I think it requires some pretty nasty code to handle all cases. What is the worry about delaying connected until the user is in it’s own audience?

-Tony

[curtisman]

I am wondering whether there should be two separate event for actual websocket connection versus quorum/audience connection.

[vladsud]

@tanviraumi has similar point in email (regarding latency of being connected). Adding to his point - "connected" is used to start sending ops (in "write" mode), so delaying it is not ideal.

If I understand correctly, they are doing that get trusted user metadata such as PUID. The Framework or runtime can hide this easily and add a method like getUserInfo(). Or even better if we add a User class. The User class will use Quorum and Audience to get info. It can also deduplicate quorum and audience ops and emits one "add/removeMember" op to the App.

[vladsud]

Summarizing, based on a bunch of earlier discussions and re-reading threads:

1. It would be great to raise connected only when own clientId is in
   • audience (for ALL types of connections)
   • AND in quorum for r/w connection only.

Note that side-effect of this may be slower time to raise "connected" event which might delay sending of local ops. We might want to measure that delay to make sure we are not regressing it badly.

Maybe better solution for Audience would be to synthetize self in audience on connection, as I believe we have all the data to do so, and ignore addMember that will happen next. Worth testing before making it more complex that indeed self shows up in audience later, i.e. this code (on "connected") does not bring self:

        // Back-compat for new client and old server.
        this._audience.clear();

        for (const priorClient of details.initialClients ?? []) {
            this._audience.addMember(priorClient.clientId, priorClient.client);
        }

2. getUserInfo() might be useful API to add that exposes user info, and caches old data while disconnected (to mimic other places where we do like that).

3. I do not like the fact that quorum & audience are not synchronized in any way. For example, we do not drop users from either on disconnect (which might be fine). But as we process messages (which can be coming from storage), we will update quorum based on those messages (users joing & leaving, including self), but Audience would stay stagnant. I believe we need to fix it:

   • all quorum members should be "added" to audience at all times.
   • all "write" members in audience that are not in quorum should be removed from there.
     Basically we need some kind of synthetic on top of those two data structures, not expose raw audience.

Might be worth splitting it into many issues.

[vladsud]

Related: #5251, #1839

[ghost]

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!

[markfields]

I'm closing this one as a dupe of "fresher" issue #7275. But this is super helpful context/back-story to refer to.