NodeBB / nodebb-plugin-global-chat

Global chat room for NodeBB
MIT License
4 stars 3 forks source link

Forum with many users performance issues #5

Open fgallese opened 5 years ago

fgallese commented 5 years ago

I have imported a forum from phpbb with more than 1M posts and 10K+ users in order to try nodebb out cause we want to move away from phpbb.

While using this plugin, I noticed that the global chat hangs the complete forum (causing http 504 errors and "Looks like your connection to NodeBB was lost, please wait while we try to reconnect." messages) everytime a message is posted in it.

Is this to be expected ? Has this plugin been tested with more than 10K users ?

Is there any way I can make the global chat contain only the logged in users in order to prevent this ?

Thank you for your time.

pitaj commented 5 years ago

I hadn't considered anything like this but it does make sense: every time a message is sent, all users in the chat room would be notified.

If that's a lot of users, that could be an issue. I don't think there's really anything I can do in this plugin to ameliorate this issue. To fix this, there will have to be changes made to core.

julianlam commented 5 years ago

We could batch process the notifications in core. Not sure if we do that right now.

arranka commented 5 years ago

in the global chat should only be users who are online. Also, you would have to have a history file so that the users who were offline could see the previous conversations, let's say that this file only allows to see 2 days ago. Please consider this

fgallese commented 5 years ago

I made some modifications to the plugin in order to make it create the chat room with only the online users, similar to what @arranka mentions.

Also, I used the login hook to add users to the room when they login, and I plan to remove them when they logout.

It seems to be working fine.

julianlam commented 5 years ago

That is an interesting strategy... :+1:

pitaj commented 5 years ago

With such changes, users would not see any of the messages that were sent while they were offline. Perhaps there is a way to remedy that issue, though.

arranka commented 5 years ago

the chat should be on the main page of the forum, ie foro.com the one on this page has to be inside the chat If a user enters any of the topics of the forum, they will no longer see the chat, therefore they will not see each other in the chat When I return to the main page of the forum, there I would be back in the chat and be able to see the conversations. If a user wants to see the conversations that were in the chat, he will have to look at the history file with 2 or 3 days and look at it. At least that's the way it is in phpbb

pitaj commented 5 years ago

Here's my current plan:

This will require modifications to core to enable the first two. The last one will also be fixed in core to be less blocking than it currently is.

fgallese commented 5 years ago

Wow, that sounds awesome and it's exactly what I have been waiting for in nodebb (and I'm sure many more people too) since the shoutbox plugin stopped being worked on and is currently unusable.

Just one thing to note from your approach: In the global chat you wouldn't know who are you talking with. With the way I implemented, only the online users are present in the chat, so you can see exactly who is there with you.

If you go with the notifications solution, you would have everyone in the room, and with no way of knowing who is actually seeing your messages. Maybe showing the status of the users in the settings button of the room ? Currently it doesn't show the status of each user, only the fact that is in the room.

Thank you very much for your time looking into it, I appreciate it.

fais3000 commented 5 years ago

@pitaj We've 95K+ users in our forum. After installing the plugin on staging, the nodebb doesn't even load the home page. I guess the cause is load on mongodb trying to register all the users in the chat. Any tips for us with this kind of scenario?

FaizanZahid commented 5 years ago

I am loving the @pitaj chat improvements issues, i hope you guys resolve it to make node bb a global modern chat platform like whatsapp.

fgallese commented 5 years ago

For anyone with a big forum using this plugin, we implemented a dirty solution.

The issue with many users is that any time a new message arrives to this global chat, the forum hangs cause it needs to deliver notifications to every user in the forum.

To avoid this, we just placed a contidional in this part of the code, if the message was placed in this global chat, it will notify ONLY the online users:

src/messaging/notifications.js

Messaging.notifyUsersInRoom = function (fromUid, roomId, messageObj) {
    async.waterfall([
        function (next) {
            if (roomId != 5) { // 5 Is the ID of the ID of the global chat room.
                Messaging.getUidsInRoom(roomId, 0, -1, next); // Proceed as normal.
             } else {
                 user.getUidsFromSet('users:online', 0, -1, next); // Only notify online users.
             }
    },

Hope this is helpful for anyone in the same situation as us while we wait for NodeBB developers to implement a proper solution.

DroidBV8 commented 1 year ago

Hello @fgallese

Your fix above is inclkuded on the plugin or I must do this manually ?

Because having quite a few users, global chat tends to be slow to open and display messages.

Currently with the number of users, my message takes 10s to appear!

DroidBV8 commented 1 year ago

Hello @julianlam @pitaj @barisusakli @fgallese

I want to test fix by @fgallese above

Is this still relevant?

Where in the file should I put the code?

I don't want to crash my forum

DroidBV8 commented 1 year ago

Anyone ?

barisusakli commented 1 year ago

You need to modify core if you want to test that fix, the relevant code is here https://github.com/NodeBB/NodeBB/blob/master/src/messaging/notifications.js#L14

barisusakli commented 1 year ago

Actually I would just change this function https://github.com/NodeBB/nodebb-plugin-global-chat/blob/master/index.js#L155-L162 to

exports.shouldNotify = async function (data) {
    if (parseInt(data.roomId, 10) === roomId) {
        data.uids = [];  
    }

    return data;
};

That would disable all notifications from global chat. but @pitaj is right having fast global chats requires big changes to core to change how messages are stored&retrieved etc.

DroidBV8 commented 1 year ago

ok @barisusakli

I think on my last test, this solution didn't work (too lol). It was even necessary to refresh all nodebb to recover the message but I will retest as soon as possible on my dev env

Overall, to be frank or direct without language of wood (as we say in France :) ) :

is this something you will be working on or do I need to look to other plugins or integrations or solutions ? I tested discord or rocket chat integrations (blah blah) but nothing very satisfactory

It's really sad because it's one of the few bulletin board without shoutbox worthy of the name

I'm complaining but I'm very grateful for your work

Have a good day and keep the good worK !

barisusakli commented 1 year ago

Yeah that code would prevent the message from being delivered as well. It has to be changed to something like this

exports.shouldNotify = async function (data) {
    if (parseInt(data.roomId, 10) === roomId) {
            data.uids = await user.getUidsFromSet('users:online', 0, -1); // Only notify online users.
    }

    return data;
};

Then it will notify and send the message to online users only, instead of all registered users.

With that said for a global chat https://github.com/NodeBB-Community/nodebb-plugin-shoutbox is a better idea. I think it also adds full page route for using the shoutbox instead of just a widget.

Because of the way messaging system works adding every single user to the chat room is not scalable right now.

I don't have time to work any of these right now since I am working on 3.x

DroidBV8 commented 1 year ago

@barisusakli

Yep but this shoutbox plugin doesn't work I guess. https://github.com/NodeBB-Community/nodebb-plugin-shoutbox

I know you don't have the time right now but I'm hopeful that it will happen one day or not. Hence my question if this is something to consider or not.

barisusakli commented 1 year ago

Shoutbox plugin works on 2.x

image

It could use a little bit of styling to make it better though

DroidBV8 commented 1 year ago

The last time I test it = doesn't work. OK I test the install now

DroidBV8 commented 1 year ago

It seem to work @barisusakli but I see this when I click on Shoutbox Name and go to https://XXXXXXXX/shoutbox

The shoutbox is duplicated and we can't do anything in the second

image

barisusakli commented 1 year ago

Check your widgets, shoutbox can be placed as a widget too maybe it is in the global footer.

DroidBV8 commented 1 year ago

Nope just one Widget on Global Header @barisusakli I have verified :(

DroidBV8 commented 1 year ago

Verified on all .tpl

--> Here a gif to see the problem :

SUPPR

DroidBV8 commented 1 year ago

@barisusakli

I test with delete cache and other browser (firefoxs, vivaldi). It's the same when I go to https://XXXXXX/shoutbox

damage

barisusakli commented 1 year ago

Remove the widget from the global header when you go to /shoutbox page you are getting 2 shoutboxes because of that

DroidBV8 commented 1 year ago

Yep but I want to have the shoutbox on Global header @barisusakli and not just on https://xxxxxx/shoutbox otherwise why have the possibility of putting a widget. it's a bug ?

Sorry I want to understand

barisusakli commented 1 year ago

In that case you don't need the /shoutbox page, just keep the widget in the global header.

image

DroidBV8 commented 1 year ago

Ok I understand @baris in this case can I delete the link present in the title of the Widget because a lot of people will click on it and see this little "Bug"

DroidBV8 commented 1 year ago

@baris

i can remove the link like this :

.shoutbox .panel-title {
    display: inline-block;
    pointer-events: none;
}

last question:

could you just add the date and/or time (timestamp) next to the nickname? It's really something very practical to know when the message was posted !!!

thanks for your answer. With the addition of the timestamp, this would fit my need

barisusakli commented 1 year ago

published nodebb-plugin-shoutbox@1.0.4, timeago strings should show up now, also fixed the double shoutbox on the /shoutbox page

DroidBV8 commented 1 year ago

Very big thanks. I will test this version ! I love you 👍 🥇 :)

DroidBV8 commented 1 year ago

Hello @barisusakli

since we are talking about it here: I suggest improvements:

Or maybe I have to turn to @oplik0 to make the roll work on the shoutbox plugin but I think he needs his event systems. i believe he filter them with filter:messaging.save and Messaging.addSystemMessage but I'm not a dev :)

DroidBV8 commented 1 year ago

@baris

here a answer by @oplik0

https://github.com/Wieloswiat/nodebb-plugin-ws-dice/issues/108#issuecomment-1398573895