search

trinodb / charts

Apache License 2.0
151 stars 173 forks source link

Option to disable Pod restart when coordinator or worker configmap changes #259

Open sdaberdaku opened 1 day ago

sdaberdaku commented 1 day ago

Currently, both the coordinator and worker Deployments add an annotation to their Pods with the corresponding configmap checksum, which ensures that the Pods are restarted upon configuration changes. When managing configuration files with the additionalConfigFiles property, sometimes it can be useful to avoid restarts if the content of one of these files is changes (i.e. configurations with the refresh-period setting that are automatically refreshed by Trino).

Could we add an option to disable these annotations if the users desires to do so?

nineinchnick commented 1 day ago

If only the contents of files referenced in additionalConfigFiles change, and no other setting, the checksum should be the same, right?

I'm not against the suggestion, I'm only thinking how to write a test case for it.

sdaberdaku commented 1 day ago

If only the contents of files referenced in additionalConfigFiles change, and no other setting, the checksum should be the same, right?

not really, I've tested it locally and the checksum changes if you change the content of an additionalConfigFile, even if it is templated.

Sorry I misread your comment. If the files referenced change and they are not mounted through this configmap then yes, the checksum does not change.

I think it would be hard to test, how would we simulate a configuration change at runtime?

sdaberdaku commented 1 day ago

We could just test if the annotation is present or not...

sdaberdaku commented 1 day ago

So let me rephrase the use case: if I want to mount a custom templated config file, I have to do it through Helm, which would make this feature useful. If the mounted file does not need templating, I would mount it with a custom configmap external to the Trino Chart.