Skip to content
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

Improve config reloading #2002

Closed
ghost opened this issue Mar 8, 2020 · 10 comments · Fixed by #2034
Closed

Improve config reloading #2002

ghost opened this issue Mar 8, 2020 · 10 comments · Fixed by #2034
Assignees
Labels
domain: config Anything related to configuring Vector type: enhancement A value-adding code change that enhances its existing functionality.

Comments

@ghost
Copy link

ghost commented Mar 8, 2020

Currently there are two issues with --watch-config feature:

  1. If running vector --watch-config --config vector.toml initially got valid config, but then the config was changed to be invalid (for example, TOML syntax was violated), then subsequent change of the config making it valid again don't cause config reloading.
  2. It would be nice to have an additional option to not crash if an invalid config is provided, but wait and try to reload it if it changes. Such behavior can be enabled by running, for example, vector --watch-config=always (or, short version, vector -ww).

The overall goal of such improvements would be making the following workflow possible:

  • Run vector generate ... > vector.toml to create a stub config.
  • Run vector -ww -c vector.toml to start watching changes in the config and report about errors if it is not valid.
  • Open vector.toml in another split or window and do live development.

Such interactive mode could be especially useful for developing complex transforms, such as the scripted transforms.

Ref fluent/fluent-bit#365

@ghost ghost added the domain: config Anything related to configuring Vector label Mar 8, 2020
@LucioFranco
Copy link
Contributor

I see how this is a confusing thing, I am curious if we should only treat the first load of the config as failable then if its reloaded vector does not crash? Though this presents issues around detecting if the config was loaded or not.

@binarylogic
Copy link
Contributor

I thought this was the behavior. Why would we stop the Vector process if an invalid config is detected? Shouldn't we just reject the invalid config and proceed as usual?

@LucioFranco
Copy link
Contributor

Though, I expect if you expect a config change to happen, then you would want it to crash (aka things like kube/systemd should restart it). I think continuing with the old config would be confusing since you don't know if that is the new config or not. I would assume in config changes that could cause issues. I think its better to bail out but may be worth it to let users configure that.

@ktff
Copy link
Contributor

ktff commented Mar 10, 2020

then subsequent change of the config making it valid again don't cause config reloading.

That's not a feature, but a bug. Traced it down to notify library, and just a few days before, an issue was opened for it.

@ktff
Copy link
Contributor

ktff commented Mar 10, 2020

It seems to be related to inotify or it's configuration.

I'll add a fix on our side.

@ktff
Copy link
Contributor

ktff commented Mar 10, 2020

  1. It would be nice to have an additional option to not crash if an invalid config is provided

@a-rodin Do you mean when initial configuration is invalid? Otherwise, what crashes are you experiencing?

@ghost
Copy link
Author

ghost commented Mar 10, 2020

@ktff I mean when initial config exists, but invalid, for example is just an empty file. Then, if such an option is enabled, Vector would print the error message, but instead of exiting would wait for new configuration.

I’m not sure does it make sense to crash if the config file is not present at all. If it is an additional option, distinct from normal watching behavior, then probably it would make sense to not crash even if the config file is not present.

@ktff
Copy link
Contributor

ktff commented Mar 11, 2020

@a-rodin like this #1816

@ghost
Copy link
Author

ghost commented Mar 11, 2020

@ktff Yes!

@ktff
Copy link
Contributor

ktff commented Mar 13, 2020

Then 2. point is a duplicate of #1816, and 1. point is resolved with #2034, so #2034 will close this issue.

@binarylogic binarylogic added the type: enhancement A value-adding code change that enhances its existing functionality. label Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: config Anything related to configuring Vector type: enhancement A value-adding code change that enhances its existing functionality.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants