synchronous_node_count support #1484
Conversation
Menzorg
requested review from
CyberDem0n,
erthalion,
FxKu,
Jan-M,
RafiaSabih and
sdudoladov
as code owners
|
Not full tested, but builded and checked with |
| @@ -254,6 +254,8 @@ explanation of `ttl` and `loop_wait` parameters. | |||
| * **synchronous_mode_strict** | |||
| Patroni `synchronous_mode_strict` parameter value. Can be used in addition to `synchronous_mode`. The default is set to `false`. Optional. | |||
|
|
|||
| * **synchronous_node_count** | |||
There was a problem hiding this comment.
Should add the info here that it's only available for Spilo images with Patroni 2.0+
There was a problem hiding this comment.
It should be right here at 257 or in description in 258?
There was a problem hiding this comment.
Should I add ❗ before notification?
| @@ -298,6 +298,7 @@ var unmarshalCluster = []struct { | |||
| LoopWait: 10, | |||
| RetryTimeout: 10, | |||
| MaximumLagOnFailover: 33554432, | |||
| SynchronousNodeCount: 1, | |||
There was a problem hiding this comment.
This is not enough here. You have to introduce synchronous_node_count option in all the other expected output examples, e.g. search for maximum_lag_on_failover and you find the places I mean.
There was a problem hiding this comment.
Hmm. I introduced synchronous_node_count in all places, where i can find synchronous_mode. If i will try to search maximum_lag_on_failover there is only one place where i did not intoduced synchronous_node_count.
It is pkg/apis/acid.zalan.do/v1/util_test.go in a place like this
But i dont understand is it the right place to introduce synchronous_node_count. Could you help, please? What should i do to help to introduce it?
There was a problem hiding this comment.
if you run the unit tests with make test or go test ./... you should be able to see where unit tests are failing
|
👍 |
1 similar comment
|
👍 |
|
Thanks again @Menzorg ! |
CRD support was previously added in zalando#1484, but comparing desired SynchronousNodeCount to the actual patroni config was missing
CRD support was previously added in zalando#1484, but comparing desired SynchronousNodeCount to the actual patroni config was missing
CRD support for synchronous_node_count was previously added in zalando#1484, however the desired SynchronousNodeCount was not compared to the actual patroni configuration, which meant it was never updated.
patroni synchronous_node_count support #1479