fdb-kubernetes-operator icon indicating copy to clipboard operation
fdb-kubernetes-operator copied to clipboard

customParameters should be merged together

Open johscheuer opened this issue 4 years ago • 3 comments

I tried to use this modified sample cluster spec (I removed the default fields):

processes:
    general:
      customParameters:
        - "locality_test=1"
        - "trace_format=json"
    storage:
      customParameters:
        - "knob_disable_posix_kernel_aio=1"

I would expect that the customParameters are merged and all storage processes will have all 3 custom parameters but only the most specific list of customParameters will be used e.g. in my case only knob_disable_posix_kernel_aio=1 will be set. This is the resulting fdbmonitor.conf:

root@sample-cluster-storage-4:/var/fdb# cat /var/dynamic-conf/fdbmonitor.conf
[general]
kill_on_configuration_change = false
restart_delay = 60
[fdbserver.1]
command = /usr/bin/fdbserver
cluster_file = /var/fdb/data/fdb.cluster
seed_cluster_file = /var/dynamic-conf/fdb.cluster
public_address = 10.1.8.225:4501
class = storage
logdir = /var/log/fdb-trace-logs
loggroup = sample-cluster
datadir = /var/fdb/data
locality_instance_id = storage-4
locality_machineid = sample-cluster-storage-4
locality_zoneid = sample-cluster-storage-4
knob_disable_posix_kernel_aio=1

I think it will be useful to merge the customParameters and actually that would be something that I expect.

johscheuer avatar Apr 27 '21 08:04 johscheuer

I think this is a good feature, but unless we de-dupe them it's a breaking change. We may also need to determine how to support having a specific process type whose custom parameters do not include all of the parameters from the general type.

brownleej avatar Apr 27 '21 16:04 brownleej

I would say that the latter is not supported in the first version, one could also argue that such knobs shouldn't be defined in the general section.

johscheuer avatar Apr 30 '21 05:04 johscheuer

I think this is a good feature, but unless we de-dupe them it's a breaking change.

@brownleej duplicates are currently going to trigger an error, I just created this related issue

gm42 avatar Mar 18 '24 16:03 gm42