-
Notifications
You must be signed in to change notification settings - Fork 286
Option to limit file size? #168
Comments
Funnily enough, I was just think about this last weekend. At Segment we limit the size of the file at the application layer to 15MB artificially — each queued event must be <15kb and we remove the oldest element if the queue size is over 1000. I think there are two APIs we could offer for this:
|
Yeah I can see both being very useful. It'd be great to be able to make sure that the file won't blow up, for one (esp. on mobile. also large files seem to have issues per #150). And when the queue keeps growing it's a sign that the backlog may never be cleared at the current processing speed, so being able to drop elements may be very important. |
What behavior would you expect here though when you're at the queue size?
Auto-evict oldest element? Block the insertion? Give a callback with the
expunged elements? Something else?
…On Mon, Jul 24, 2017 at 8:47 PM Danqing Liu ***@***.***> wrote:
Yeah I can see both being very useful. It'd be great to be able to make
sure that the file won't blow up, for one (esp. on mobile. also large files
seem to have issues per #150 <#150>).
And when the queue keeps growing it's a sign that the backlog may never be
cleared at the current processing speed, so being able to drop elements may
be very important.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#168 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEdjZ-UP_6SEMZvot93nx_etuYqWQks5sRPTbgaJpZM4Ohnwh>
.
|
Personally I'd like to see auto-eviction. When a size limit is set and we are about to go over it (i.e. when we absolutely need to discard items), discarding the oldest one seem to make the most sense to me (also since we are in FIFO world). I'm not sure what @f2prateek would like - if auto-eviction is the most common, maybe the |
In Segment's case, eviction makes the most sense for us. But I see Jake's point that the desired behaviour might be different for different use cases. For instance with payments, it would likely make more sense to block the insertion and notify the user the current transaction failed, rather than drop a queued transaction. @danqing QueueFile isn't designed for extension the way you've described. What I'd do is either wrap QueueFile or ObjectQueue in your own decorator for the add methods and apply your logic to evict entries or block insertion based on file size or number of entries. |
So based on my very limited understanding, as long as we can swap logic for It seems to be the thing called when we hit the limit, and currently the behavior is to double the size. If it can be swapped, then a class can:
Do you suggest creating additional methods to short-circuit |
Do you have any suggested workaround to limit the file size without modifying the current code? I see that you have |
@jsachania Could you make a delegating ObjectQueue that checks the file size in |
I think checking the file size may not work because Tape doubles the file size when it runs out of room in the file. We need to know the actual bytes used or remaining bytes to make estimate. |
Is it possible to provide the functionality to limit the underlying file size (automatically remove oldest element if the file reaches a certain size and new elements are coming)?
If there's no intention to provide such functionality maybe it's possible to change some function to
protected
(e.g.expandIfNecessary
) so a subclass can add it?Thanks!
The text was updated successfully, but these errors were encountered: