-
Notifications
You must be signed in to change notification settings - Fork 60
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.
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
how to delete buffer after certain time? #2
Comments
I'm interested in this too. I'm using this to write a notification service and I'd like to have several categories and the notifications to expiry after a configurable amount of time, ex: 1 hour. So I don't keep a lot of buffers with notifications that will never reach the users. |
These are good, useful ideas. I can look into adding some sort of configurable expiration, and also an optional param to the subscription http handler that allows web clients to mark notifications for deletion. I should have time to tackle this starting a couple of days from now. This way, notifications can auto-expire, or if users want to explicitly delete a notification earlier, they could do that too. Thoughts? |
@jcuga in my case just auto-expire is relevant as I'm broadcasting an event and I'm unaware how my clients have subscribed, so the event can't be deleted by the client as I don't know how many clients must receive the event. |
@jcuga how would you implement this expiration? |
@jcuga what about using a skiplist with unixtime as key and category(string) array as value (as some events could happen at the same time) over engineering? good solution? |
this https://github.com/diegobernardes/ttlcache could be useful too |
@mrwnmonm, one way to implement the expiration would be to remove any events in a given event buffer that are older than X amount of time whenever a new item is added to the event buffer. Events exist in chronological order, so starting at the one end of the buffer you could just keep deleting until you get to an event that is recent enough. The function GetEventsSince shows how the code is already inspecting the buffers for events that fall after a given input time. This approach would not cover the case where a given category doesn't have any new events for quite some time, so we'd never perform an insert-new-and-remove-expired-events operation. Perhaps by associating a timestamp with each event subscription category that records the newest event in that channel, and then by periodically purging channels that haven't updated in a while. In the next week or two I plan on sitting down and trying to provide options to allow expiration behavior. My initial thoughts are something along the lines of a cleanup upon new event insert, and then a periodic flush of old channels. But performing a flush on old channels can't be too expensive, or else performance might suffer. So I'll have to take a closer look at your suggested links and see what's the best option. |
Just to let you all know, I haven't forgotten about this issue. It's going to take some pretty major changes to support it, so I plan on making a branch and will start to tackle it sometime this week. Stay tuned. |
@jcuga thanks! If you need any help testing or discussing something feel free to ping me. |
great
|
Okay, so I've given some thought to this. Since this is pretty major, I want to make sure I'm building the right thing from the start. So I tried to think of how I would be able to configure this new behavior. What I've come up with is an Options structure that I've added here: The new options would be:
See the code comments for a full explanation of each option. @mrwnmonm I believe these would support your needs. By the way, If you are calling @fabioxgn all you would need to do is set If these options make sense, I can go ahead and make a new factory function that takes an |
@jcuga liked the About the |
@jcuga like you have said to get a better view on what i'm doing. my notifications is exactly like the facebook notifications
if the user is active, and some event happened, i do this:
if the user is not active, and some event happened, i will do exactly the same so notice i don't get the actual notifications or the unread notifications count from the poll, i'm using it for just notifying the current users, if the user has been notified i delete the event immediately, if not i have no interest to keep the event i hope this describes my case well. |
I think you all are right about holding off on Once that option is implemented, you can test it via
|
Update! I have made all of the feature changes in the branch 'buffer_cleanup' Once I add a bunch of unit tests, do some more stress testing, and update the README, I will merge this into master and tag a new release. Until then, you all can try out the new changes by just doing a Here's what I ended up doing. I deprecated the This Options struct has the options:
The last two options are the ones that control how long events and buffers exist. EventTimeToLiveSeconds can be set to golongpoll.FOREVER to disable event expiration. The way I implemented event/buffer expiration is that any time a new event is published on a category or any time a client starts a longpoll on a category, that specific category's event buffer is checked for expired events and they are removed. At those times we're already referencing the event buffer and the events are already stored in order so clearing expired events isn't hard or expensive to do. This approach of cleaning up old events and deleting empty buffers whenever there is activity on a given subscription category solves most of the problem. But you could have an inactive category where there are no events being published and no clients requesting a longpoll. An example of this scenario would be if you're using per-user categories and that user is no longer on your web app. How do we purge that user's events after they've left? For that case, the expiration cleanup is more difficult. To solve this problem, the code now keeps track of how old the event buffers are by keeping a priority queue (min heap) of references to the event buffers sorted by least-recent activity. Every so often the code will check that data structure and delete events/buffers that are too old. The min heap is generally O(log(n)) and since we're cleaning up old buffers the size of the heap should be roughly the number of recent/active categories, plus whatever hasn't expired yet. Also, we don't bother keeping the heap if we have EventTimeToLiveSeconds set to golongpoll.FOREVER. It was a lot of work to get this event expiration behavior, but I think these changes should do a good job covering it. It was a good coding experience to think about the data structures. If either of you want to try things out feel free to do so. |
These features are now merged into master. I will update the README on the new Options in about a day or so. @mrwnmonm , @fabioxgn Are there any objections to me closing this issue? |
@jcuga nice, thanks! No objections, I'll update my project and if I find any issues I'll report. |
@jcuga thanks for you work, it will be also a good time for me to understand how you implemented this features |
Hi, i'm using this package to implement notifications (it's awesome by the way)
i use one category for each user
after the user gets the notifications i'm not interested in keeping them in the buffer, so i'm deleting the user buffer after every publish (i did this already, i would do a pull request if you like)
also i don't want to keep the buffer if the user left the site for a long time
so, how i could do that?
The text was updated successfully, but these errors were encountered: