[RFE] Support keepers that will never become master/sync

[lawrencejones]

We have a use case for creating keeper nodes that should be managed by the stolon cluster but never become eligible for promotion. The additional nodes may be far less resourced than the rest of the cluster, hence why we'd never want them to become primary, or have different configuration parameters applied (like frequent checkpointing intervals).

Proposal

The keeper should support two new flags, --never-master and --never-synchronous-replica. The values from the command line flags seem most appropriately placed in the KeeperSpec struct, if updated with NeverMaster, NeverSynchronousReplica bool fields. The keeper is not able to directly manipulate the spec, so the suggestion is to:

• Add and store these fields into the KeeperInfo struct
• Amend the updateKeepersStatus method to extract the NeverMaster and NeverSynchronousReplica fields into the KeeperSpec objects inside ClusterData

The updateKeepersStatus is run on every clusterSentinelCheck, thereby ensuring we populate the KeeperSpecs before ever calling updateCluster and making decisions about cluster orientation.

Finally, we'd modify the Sentinel to remove any --never-synchronous-replica keepers from those considered for synchronous standbys (by applying filtering here) and make findBestNewMasters remove --never-master keepers.

Anticipated questions

Why KeeperInfo -> KeeperSpec?

Not being the original author of this code, it's not fully clear what the intent of KeeperInfo vs KeeperState vs KeeperSpec vs DBSpec is. Our understanding is that:

• KeeperInfo is information about the keeper that is managed by the keeper
• KeeperSpec (while presently empty) should contain specifications about the keeper, managed by the Sentinel
• KeeperStatus is up-to-date information about the current keeper state, managed by the Sentinel
• DBSpec is a specification for a database, managed by the Sentinel
• DBStatus is an up-to-date reflection of database status, managed by the Sentinel

It feels most natural to specify the keeper constraints as keeper flags, but it would be good to confirm it's not weird to be pushing this into the KeeperInfo and having the Sentinel draw them into the KeeperSpec. @sgotti will be best placed to answer this?

Can we already do this?

While you can do this using pitr mode and external standbys, Stolon provides a load of additional functionality via its resync flow to fully manage a standby, ensuring whatever happens in the cluster the standby will be resync'd to match the selected primary. We'd also prefer to model these nodes as part of the same cluster, sharing the same authentication material. Having Stolon manage this would be much better than leaning on the existing external standby configuration.

[sgotti]

@lawrencejones The logic looks good to me. I'm not sure about you use case (can you provide an example?). Just few details/question:

• KeeperInfo is information about the keeper that is managed by the keeper

Yes. KeeperInfo contains the current keeper status reported by the keeper (no persistent data). So if your need is to report to the sentinel a per keeper command line flag then this is the right (and unique) place to do this.

and having the Sentinel draw them into the KeeperSpec.

You mean KeeperStatus right?

If so KeeperStatus is the right place to save the status reported by the keeper (via KeeperInfo)

[lawrencejones]

I'm not sure about you use case (can you provide an example?)

Sure! We're building a new backup and disaster recovery system for Postgres at GoCardless. One of the components of that system would be a stolon keeper connected to each of our main clusters that would likely be of a way lower machine class (n2-highmem-32 is a bit extreme for a replica) and have a persistent disk attached that we'll configure GCE to snapshot at regular intervals.

We don't want to snapshot any of the replicas in the cluster that may become the master as snapshots can have an impact on performance, along with it being weird that just one of those three nodes is arbitrarily chosen and may be down during operation. We also want to provision the replica slightly differently with (for example) very frequent checkpointing to ensure our disk snapshots require a minimal amount of time to recover at startup.

You mean KeeperStatus right?

I didn't! I was actually talking about KeeperSpec: https://github.com/sorintlab/stolon/blob/master/internal/cluster/cluster.go#L571

Right now KeeperSpec is an empty struct, but this felt like the right place to put these variables. That's going with the idea that *Spec represents a declarative ideal state while *Status is a reported-at-runtime status. Whether a keeper should be prevented from ever being master/sync would feel a declarative specification, not a status, but I may have got this all mixed up.

[sgotti]

I didn't! I was actually talking about KeeperSpec: https://github.com/sorintlab/stolon/blob/master/internal/cluster/cluster.go#L571

KeeperSpec is the keeper specification managed by the sentinel and represent something that the keeper must observe. It's empty because there's currently no use case for a field in it. As an example take a look at the DBSpec it contains the db specification that the keeper must observe or the ProxySpec that contains the require state that the proxies must observe.

You should instead use KeeperStatus: KeeperStatus is the up-to-date information about the keeper state. Since inside the keeper (using a per keeper cli option) you're specifying a specific keeper "property" and reporting this via the KeeperInfo. Then you'll update KeeperStatus so the sentinel will use this new fields to compute the final cluster state.

[lawrencejones]

You should instead use KeeperStatus

Ah, we didn't realise that *Spec is intended for Sentinel to set and others to consume. If that's the pattern then KeeperStatus does make more sense.

We'll have a PR for this coming in today. Thanks for the help!

[maksm90]

Hi @lawrencejones !

I think this feature could be incorporated into setting keeper's priority #622 specifying some dedicated values to nodes to not become synchronous replica or master. That feature is more generalized because provides to make directed switchover from async replica to sync one or from replica to master issuing setkeeperpriority <candidate_node> and failkeeper <sync_replica or master> commands.

What do you think about it?

[sgotti]

I just merged #697 that implements this.

As proposed by @maksm90 there's also #492 (with dead PR in #622) that should coexist with this or cover this case. If someone wants to work on it let's continue discussing on #492.