Skip to content

Extend the signaling setting API to allow somewhat clustering#3292

Merged
nickvergessen merged 24 commits into
masterfrom
feature/noid/hpb-clustering-per-conversation
May 12, 2020
Merged

Extend the signaling setting API to allow somewhat clustering#3292
nickvergessen merged 24 commits into
masterfrom
feature/noid/hpb-clustering-per-conversation

Conversation

@nickvergessen

@nickvergessen nickvergessen commented Apr 7, 2020

Copy link
Copy Markdown
Member
  • Improve "random empty default picking"
  • Add token when requesting signaling information
  • Make sure the UI uses the correct signaling server per conversation

Fix #2591
Fix #3463

@nickvergessen nickvergessen added 2. developing enhancement feature: api 🛠️ OCS API for conversations, chats and participants feature: signaling 📶 Internal and external signaling backends technical debt feature: frontend 🖌️ "Web UI" client labels Apr 7, 2020
@nickvergessen nickvergessen added this to the 💚 Next Major (19) milestone Apr 7, 2020
@nickvergessen nickvergessen force-pushed the feature/noid/hpb-clustering-per-conversation branch 3 times, most recently from 18d030a to 2be5e72 Compare April 14, 2020 11:29
@nickvergessen nickvergessen force-pushed the feature/noid/hpb-clustering-per-conversation branch from 2be5e72 to 30fee5c Compare April 15, 2020 15:45
@nickvergessen nickvergessen marked this pull request as ready for review April 15, 2020 15:46
@nickvergessen nickvergessen force-pushed the feature/noid/hpb-clustering-per-conversation branch from 30fee5c to d4d8cca Compare April 15, 2020 15:50
@nickvergessen

Copy link
Copy Markdown
Member Author

Should be functional now.

We still pick a random one for the first participant, but that is hard to make right without a real usecase in hand. But adjusting that should be rather easy in the end, so testing it for now should be fine.

Most effects should be noticable without the clustering.
But real clustering is skipped unless there is more then one signaling server defined and the config is set. Can be hacked in Config.php by adding return self::SIGNALING_CLUSTER_CONVERSATION; in getSignalingMode().

By replacing: currentSignalingServer = settings.url with currentSignalingServer = settings.url + 'different' iin src/utils/webrtc/index.js you can also fake a different signalign server being used all the time, making you close and open the connection on every conversation change.

@nickvergessen nickvergessen requested a review from danxuliu April 15, 2020 15:52
@nickvergessen

Copy link
Copy Markdown
Member Author

Would be nice if you can have a quick look and spill your opinion @danxuliu

@nickvergessen

Copy link
Copy Markdown
Member Author

Added a commit to check the config.php for a hardcoded list to allow overwriting the random for some known big conversations which can then be assigned specifically to always end up on the set machines

Comment thread lib/TInitialState.php Outdated
@danxuliu danxuliu force-pushed the feature/noid/hpb-clustering-per-conversation branch 2 times, most recently from f73b07f to 154502c Compare May 5, 2020 18:13
@nickvergessen nickvergessen requested a review from danxuliu May 6, 2020 07:39
nickvergessen and others added 15 commits May 12, 2020 10:40
So we can use it to decide which HPB to talk to

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Now a single signaling connection is kept open no matter how many
signaling servers are available. When joining a conversation the
signaling settings for that conversation are always got, as the
signaling server could have changed since the last time it was joined.
If the conversation uses a different signaling server than the one
currently open that connection is closed and a connection to the new
signaling server is started.

If a different conversation is joined while waiting for joining to a
previous one the previous one is cancelled and only the latest one is
taken into account.

All the other signaling operations (joining a call, leaving a call or
leaving a conversation) are taken into account only if they act on a
conversation that belongs to the current signaling server. Otherwise
they are ignored.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the feature/noid/hpb-clustering-per-conversation branch from 8c9e598 to 40fa7c6 Compare May 12, 2020 08:41

@danxuliu danxuliu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the last changes there are no ghost sessions and forced reconnections work. However, during a forced reconnection now the UI goes back to the chat view and then shows again the call view (the same that happened in Talk 7).

Other than that and the unit tests everything seems to work now.

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen

Copy link
Copy Markdown
Member Author

Ran unit tests locally, they pass and random integration tests also still worked, so merging

@morph027

morph027 commented Jun 6, 2020

Copy link
Copy Markdown

Jumping in here...Running latest version of NC and Talk (19 + 9) and Redis but i can't find a possibilty to add more than one signaling server in talk settings. Do i miss something?

@nickvergessen

Copy link
Copy Markdown
Member Author

The HPB can handle a few hundred users by default without any bigger problems. If you need the clustering you should get in contact with our sales team for a Nextcloud Talk Enterprise Subscription:

https://nextcloud.com/talk#pricing

@morph027

morph027 commented Jun 6, 2020

Copy link
Copy Markdown

Ah, well, not really necessary, just stumbled upon this in the release notes and wanted to try to satisfy my curiosity and improve knowledge about everything ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement feature: api 🛠️ OCS API for conversations, chats and participants feature: frontend 🖌️ "Web UI" client feature: signaling 📶 Internal and external signaling backends technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

As guest or un-invited user, the room load all the messages Switching to another conversation is sluggish

3 participants