-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add flush timeout setting to filebeat registrar #5146
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a CHANGELOG entry?
@@ -21,6 +21,7 @@ const ( | |||
type Config struct { | |||
Prospectors []*common.Config `config:"prospectors"` | |||
RegistryFile string `config:"registry_file"` | |||
RegistryFlush time.Duration `config:"registry_flush"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer registry.flush
and registry.file
but too late to change I think.
@@ -34,12 +38,13 @@ var ( | |||
registryWrites = monitoring.NewInt(nil, "registrar.writes") | |||
) | |||
|
|||
func New(registryFile string, out successLogger) (*Registrar, error) { | |||
func New(registryFile string, flushTimeout time.Duration, out successLogger) (*Registrar, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we call the config also flush_timeout? Would make it more explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean filebeat.registry_flush_timeout
? Well, actually it's more like a 'delay' setting...
@@ -198,6 +212,17 @@ func (r *Registrar) Stop() { | |||
r.wg.Wait() | |||
} | |||
|
|||
func (r *Registrar) flushRegistry() { | |||
if err := r.writeRegistry(); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a check here that if bufferedStateUpdates==0
it just returns? Like this flushTimeout could be set very low and still not have frequent writes in case there are no updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timer is only started after an event has been added.
6b53f28
to
3dc174b
Compare
(cherry picked from commit 0d89cc7)
No description provided.