Implement role re-assignment based on failure domains

[ktsakalozos]

Allow node roles to follow failure domains.

If we do not have voters on all failure domains and we have a domains with more than one voters we should try to start voters on the failure domains without voters.

Fixes: #267

Understandably this may not be the direction we want to move as the role assignment has recently moved to the dqlite project.

[cole-miller]

Thanks! I agree that this is a problem that needs to be fixed, and we should fix it in go-dqlite, which doesn't use the server-side role management support (yet).

I have a concern about the behavior change here. The new logic may result in promoting a newly-joining node to voter even if we've already hit the voter target, if if's from a failure domain that's not represented yet and at least one other failure domain is "overpopulated" with voters. After the promotion succeeds, the adjustment logic will notice that there are too many voters and will try to demote one of them -- but the demotion logic is not aware of failure domains, so it's possible that we pick a node for demotion that's the only voter in its failure domain, and the end result is no better than what we started with. To fix this, we should use sortCandidates (or a variant) to guide the demotion process for online voters.

[ktsakalozos]

After the promotion succeeds, the adjustment logic will notice that there are too many voters and will try to demote one of them -- but the demotion logic is not aware of failure domains, so it's possible that we pick a node for demotion that's the only voter in its failure domain, and the end result is no better than what we started with.

I did not think about this. You are right. The latest commit adds a domain aware sorting function to be used during demotion. To reproduce this problematic behavior I used the following test:

// If cluster is imbalanced (all voters in one failure domain), roles get re-shuffled.
func TestRolesAdjustment_ImbalancedFailureDomainBounce(t *testing.T) {
	n := 4
	apps := make([]*app.App, n)
	cleanups := make([]func(), n)

	for i := 0; i < n; i++ {
		addr := fmt.Sprintf("127.0.0.1:900%d", i+1)
		// Half of the nodes will go to failure domain 0 and half on failure domain 1
		fd := 0
		if i == 1 {
			fd = 1
		}
		options := []app.Option{
			app.WithAddress(addr),
			app.WithRolesAdjustmentFrequency(4 * time.Second),
			app.WithFailureDomain(uint64(fd)),
		}
		if i > 0 {
			options = append(options, app.WithCluster([]string{"127.0.0.1:9001"}))
		}

		// Nodes on failure domain 0 are started first so all voters are initially there.
		app, cleanup := newApp(t, options...)

		require.NoError(t, app.Ready(context.Background()))
		time.Sleep(1 * time.Second)

		apps[i] = app
		cleanups[i] = cleanup
	}

	for i := 0; i < n; i++ {
		defer cleanups[i]()
	}

	for i := 0; i < n; i++ {
		cli, err := apps[i].Client(context.Background())
		require.NoError(t, err)
		require.NoError(t, cli.Weight(context.Background(), uint64(n-i)))
		defer cli.Close()
	}

	apps[1].Handover(context.Background())

	time.Sleep(20 * time.Second)

	cli, err := apps[0].Leader(context.Background())
	require.NoError(t, err)
	defer cli.Close()

	cluster, err := cli.Cluster(context.Background())
	require.NoError(t, err)

	domain := map[int]bool{
		0: false,
		1: false,
	}
	for i := 0; i < n; i++ {
		// We know we have started half of the nodes in failure domain 0 and the other half on failure domain 1
		fd := 0
		if i == 1 {
			fd = 1
		}
		if cluster[i].Role == client.Voter {
			t.Log(fmt.Sprintf("%d is voter", i))
			domain[fd] = true
		}
	}

	// All domain must have a voter
	for _, voters := range domain {
		assert.True(t, voters)
	}
}

This test is not part of the code because it triggers the bouncing of the voters but it does not detect it. You need a debugger or more logging to see the voter bouncing.

[ktsakalozos]

@cole-miller any thoughts on the latest updates?

[cole-miller]

@ktsakalozos -- sorry for the delay, I've been on PTO. Taking a look at the latest branch now.