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.

Sign up for GitHub

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

Jump to bottom

Chat should not poll the server but use events with the standalone signaling server #624

Open
fancycode opened this issue Feb 5, 2018 · 8 comments
Open

Chat should not poll the server but use events with the standalone signaling server #624

fancycode opened this issue Feb 5, 2018 · 8 comments
Assignees
@fancycode
Labels
1. to develop enhancement feature: chat 💬 Chat and system messages
Milestone
💔 Backlog

Comments

@fancycode
Copy link
Member

The chat currently polls for new messages. When using the standalone signaling server, this should be changed to be event based to reduce the server/database load and improve latency.
@fancycode fancycode added the feature: chat 💬 Chat and system messages label Feb 5, 2018
@fancycode fancycode self-assigned this Feb 5, 2018
@nickvergessen
Copy link
Member

Hmm the chat backend is not settled yet as far as I understood.

@nickvergessen nickvergessen added this to the 4.0 (Nextcloud 14) milestone Mar 12, 2018
fancycode added a commit that referenced this issue May 9, 2018
@fancycode
Move loading of chat messages to signaling class.
31eb8f7 
No functional changes yet, but this will allow later to use the standalone
signaling backend to notify clients about new chat messages without having
to poll (see #624).

Signed-off-by: Joachim Bauch <[email protected]>
fancycode added a commit that referenced this issue May 9, 2018
@fancycode
Move loading of chat messages to signaling class.
f6c32a3 
No functional changes yet, but this will allow later to use the standalone
signaling backend to notify clients about new chat messages without having
to poll (see #624).

Signed-off-by: Joachim Bauch <[email protected]>
This was referenced May 9, 2018
Merged
Merged
nickvergessen pushed a commit that referenced this issue Jul 12, 2018
@fancycode @nickvergessen
Move loading of chat messages to signaling class.
18c090f 
No functional changes yet, but this will allow later to use the standalone
signaling backend to notify clients about new chat messages without having
to poll (see #624).

Signed-off-by: Joachim Bauch <[email protected]>
@nickvergessen nickvergessen modified the milestones: 5.0.0, 💚 Next Major Dec 7, 2018
@nickvergessen
Copy link
Member

From my POV we basically listen to the event for new chat/system messages, add a "pull for messages please" message to the signaling and remove the chat polling.
The problem I see with this is that the ping was replaced with the chat polling, so this needs to be replaced/kept in mind too.

@fancycode
Copy link
Member Author

The standalone signaling server "pings" connected clients internally, so this shouldn't be a problem.

@nickvergessen nickvergessen mentioned this issue Dec 13, 2018
Closed
@PVince81
Copy link
Member

Just noticed this happening and was wondering why we still poll.

I didn't see any signaling events for message sending, so I guess it's not implemented yet in the signaling server ?

@nickvergessen
Copy link
Member

It's not implemented in the signaling backend in talks php.
But also it's something for later

@tcitworld
Copy link
Member

But also it's something for later

Is this still valid? What blocks this?

@nickvergessen
Copy link
Member

Allocating time on all clients.
It's still favored for the future, but nothing short term, but it was discussed recently again as potential improvement for scaling.

@nickvergessen
Copy link
Member

  • Skip system messages (language + you) unless not shown (reaction, edit, delete)
  • Skip file shares (due to path)
  • Skip federation
  • Add features flag to hello-request to HPB
  • Backend sends both (chat-refresh and chat-relay)
  • Make sure readmarker update does not actually do more requests (delay ~2-5 seconds)
  • Keep in mind the message order between relays and refreshes from system messages
  • Keep in mind that the HPB does not tell you about "past messages", so pull once when joining the conversation
  • Keep in mind to only pull history after connection (or joining the room) to the HPB, otherwise you have a gap (and deal with duplicates)
  • Confirm the token of the chat-relay message to only add them for the correct room

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

@fancycode fancycode

Labels
1. to develop enhancement feature: chat 💬 Chat and system messages
Projects
None yet
Milestone
💔 Backlog
Development

No branches or pull requests
4 participants
@nickvergessen @fancycode @PVince81 @tcitworld