-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
refactor Filters::Base periodic_flush option #1839
Comments
also relates to #1818 |
@colinsurprenant do you think you can target this refactoring for 1.5.0 or can you recommand how to patch the filters that should flush "by default" like metrics,elapsed... |
@colinsurprenant is this possible to do in 1.5? I merged the fix for metrics based on comments in #1818 |
resuming work on this, let's get that right for 1.5! |
let's discuss to see if this should be a blocker for 1.5 - I do not think it should. the metrics filter issue will be solved by logstash-plugins/logstash-filter-metrics#5 |
We can move it out. periodic_flush was added a bit by mistake and we can just direct everyone to not use it until we remove it probably in 1.5.1 Not required for 1.5.0 |
I don't see it as a priority for 1.5. the important thing was to get the metrics flush working, which we did. its a nice code cleanup, but not something I would hold up 1.5 for |
well, it's only really used in the multiline filter where its correct to be configurable and hacked to be non-configurable in the metrics filter. so, yeah, let's keep an eye on new plugins implementations that would like to use that non-feature :P and we can refactor soon but not blocker for 1.5 |
New to logstash plugin development and starting to hack away on the collate filter. Forgive my ignorance but wanted to share what I'm seeing for collate. It creates a Rufus::Scheduler to do internal flushing at the configurable interval. That makes the plugin a lot more complicated. Ideally, that plugin could leverage core periodic flush. I guess my question is: if I wanted to work on collate or a plugin similar to it, should I try to leverage something in logstash core or continue implementing plugin specific flushing? |
this need to be re-assessed WRT the new 2.2+ batched pipeline implementation @erik-stephens sorry for the late followup, I believe this is a very valid question and this is something we should take into account in the new batched pipeline implementation. I will add the 2.3 milestone tag so that we keep track of this in the short term. |
With the new pipeline there should no longer be any need to flush IMHO. I'd like to hear about any use cases that would require a flush. |
Closing this. Please reopen if there are any requirements for flush |
as discussed in #1545 and #1805 the periodic_flush should not be user-configurable.
The text was updated successfully, but these errors were encountered: