dragonflydb / dragonfly

A modern replacement for Redis and Memcached
https://www.dragonflydb.io/
Other
24.46k stars 884 forks source link

fix(server): Rename confusing flag `replica_reconnect_on_master_restart` #3193

Closed chakaz closed 1 week ago

chakaz commented 1 week ago

That was a misleading name, as the logic was the exact opposite (oops :facepalm:)

This PR introduces a new name for the same flag: break_replication_on_master_restart

We're keeping the previous flag for now, to make transition easier. We'll remove it in a later Dragonfly version (>= 1.22)

Fixes #3192

ashotland commented 1 week ago

@romange - that is going to be easy to miss

Why not respect the old flag for some time ?

While we can already code that conditioned flag configuration,I think muting the flag is worse than making dragonfly fail on unknown flag - better have something explicit IMO and will allow a more reasonable migration process.

romange commented 1 week ago

The price of missing is not critical. @chakaz up to you.

chakaz commented 1 week ago

Sure, you're all right. Let's keep the old flag with the bad name, and add another one (same behavior, better name).