ASKBOT / askbot-devel

Askbot is a Django/Python Q&A forum. **Contributors README**: https://github.com/ASKBOT/askbot-devel#how-to-contribute. Commercial hosting of Askbot and support are available at https://askbot.com
Other
1.56k stars 627 forks source link

Use django messaging #817

Closed martin-bts closed 4 years ago

martin-bts commented 5 years ago
martin-bts commented 5 years ago

Using Django messaging reduces load on the database and that's about all the advantages we get from it. We have three distinct use cases where we need to send notifications to users that may or may not be online. In these use cases we cannot easily access the corresponding request objects so Django messaging is not available. Especially not if the notification is sent to a user that is offline at the moment.

As a result, this commit would add Django messaging to the existing approach and not remove/replace anything. For all use cases where Django messages can be used, we have two equally viable solutions with very different APIs (one request based, one user based).

Maybe this is not an ideal extension to Askbot at the moment. And maybe we should discuss different approaches first and not merge this PR now.

evgenyfadeev commented 5 years ago

Continuing our conversation re: what can be done to improve the messaging system:

I'm sure this can be done. Until then we can keep using the current working albeit a flawed system.

martin-bts commented 5 years ago

I do no know why, but while Django easily allows for user objects in requests, i.e. request.user, the other way round does not exist. AFAIK there is no ready to use method to obtain all sessions for a given user. However, this user->session mapping is essential for having an API that allows us to write messages to a specific user and, as soon as possible, render them.

To remove DB interactions we must use some message cache and I think the best idea would be to use Django's caching subsystem. At least I cannot see the motivation to develop something new here.

We either solve the user->session mapping, which will allow us to use Django sessions as cache, or we devise a user->cache_entry mapping, which allows our messaging API and every active session, i.e. every request, to compute the "this-users-messages" key and fetch/send messages to/from the users message box stored in the cache.

The current system, which always relies on the database stores all messages until they are retrieved through a request. Using a cache makes messages a lot more volatile. On the other hand, we are talking about messages like "thank you for your feedback", "you have received the xyz badge" and "you must be logged in to view this post". Maybe volatile questions are ok.

When we write a new messages API, I think we should be more explicit. There can be generic user.askbot_message.send for writing arbitrary text notifications and in addition we have use case sepcific messaging for instance user.askbot_message.award_badge and others. The various methods store the messages differently, which essentially implies the message type and allows for all kinds of use case specific handling, for instance, add a badge name to an existing badge-notify-list, rather than simply appending the n+1st message to the user's only message queue. There can also be a more specific API for retrieving messages. This would allow us to handle messages at a higher abstraction level which gives us more degrees of freedom for different use cases. I am particularly thinking about messages to/from Ajax requests, because those are handled 100% differently at the moment but they are also messages from the system to the user.

evgenyfadeev commented 5 years ago

Ajax responses should excluded from this topic at least for now because those messages are very operation-specific, also most of them display differently - in the in-place popups. Therefore they are coming in the ajax response payloads.

I kind of think that the solution is using a combination of some middleware, a context processor, global per-request maybe a thread-local variable/queue (not in a cache - commenting more on that below). That is for the messages that are for the immediate emission. Messages to/from the other users would be written to/read from the database (for this the cache is too volatile imo).

The middleware would capture the request user id. When we send a message to a user - we check if id of the user matches - and in that case store the message in the in-memory queue. If the user is different - store it in the database.

In the context processor we combine the messages from both places - both the in-memory queue and the database.

Re: where to store the volatile messages - the in-memory variable is the best imo, b/c the messages will be used up anyway within the same request.

As a bonus we might implement message batching for the storage in the database - i.e. not write them immediately, but store in a batch operation at the end of the cycle, maybe even in a celery job.

martin-bts commented 5 years ago

I suppose it can work like that.

Without Ajax messages, IIRC, there is only messages meant for immediate delivery(, with three exceptions). Immediate delivery means all generated messages will be part of the response, sent to the user as a consequence of the currently processed request. Each individual request is always processed by exactly one thread/process. Consequently thread/process local variables should be sufficient.

Vice versa, a thread/process answers one request at a time. The middleware simply needs to set up a global empty list and messages from that list must be part of the response this thread will generate. A global variable, also created by the middleware really only needs to store the user's id. Since all of this is thread local, the thread does not need to handle different users or many instances of the same user.