4496 topo serving shards refactor

[rafael]

Description

• First stab at global topology refactor as described in [topology] Global Topology Serving Shards refactor #4496.

Things to keep an eye

• Something that we didn't caught during the design, is that we still need to keep the concept of a tablet control. This allow MigrateServedTypes to first disable query service in sources, enable in destinations and then update the srvKeyspace.
• @sougou - I didn't remove SourceShards as an element to disable/enable query service in tablets. It was not required for this refactor and thought that it might be better to that as a separate refactor.
• One part that I don't like, is that there are still some things in the global topo, that could be move down to the cells. Specifically vertical resharding. Once we move that, this refactor will be complete. In the meantime, I feel like we are going in the right direction, but it will be a bit messy.

Pending Tasks

• Add tests for the newly added functions in srv_keyspace.
• Add tests for multi shard split in go/vt/wrangler/testlib. I realized there are cases that are covered in integration tests that are not here.
• Implement some utility functions to manipulate SrvKeyspace by hand. This used to be an option that I think we should keep for emergencies. I still need to do that.
• Add backwards compatibility for ServedTypes when generating serving keyspace. Moving forward, we are using the isMasterServing attribute in the shard to decide if we should include a shard in the serving keyspace. We need to make sure that this works for legacy topologies that won't have that set.
• Fix some tests that are still failing on travis.
• Do not rebuild keyspace if there are tablet control records set. This means that we are in the process of migrate serving types.

[sougou]

I think this function should not exist. We should instead just use all cells without assuming anything. And then deal with absence of ShardReference only if it's actually relevant.

Also, I think a ShardReference is supposed to be there in all cells, right? The only time it can be missing is if someone didn't do a Rebuild after creating a new shard.

In any case, it's better to go through all cells for these things.

[rafael]

This makes sense. I'll update accordingly.

[rafael]

Sugu - I was looking into this a bit more and I was thinking that it might be better do this in a future iteration.

There are some parts where I will need to make some slight modifications. For instance:

	servingCells, err := wr.ts.GetShardServingCells(ctx, shardInfo)
	if err != nil {
		return err
	}
	// Check the Serving map for the shard, we don't want to
	// remove a serving shard if not absolutely sure.
	if !evenIfServing && len(servingCells) > 0 {
		return fmt.Errorf("shard %v/%v is still serving, cannot delete it, use even_if_serving flag if needed", keyspace, shard)
	}

or

func (wr *Wrangler) RemoveShardCell(ctx context.Context, keyspace, shard, cell string, force, recursive bool) error {
	shardInfo, err := wr.ts.GetShard(ctx, keyspace, shard)
	if err != nil {
		return err
	}

	shardServingCells, err := wr.ts.GetShardServingCells(ctx, shardInfo)

	// check the cell is in the list already
	if !topo.InCellList(cell, shardServingCells) {
		return fmt.Errorf("cell %v in not in shard info", cell)
	}

It's not really big, but given that this PR is already huge. Thinking that making this change as a separate PR makes sense.

[sougou]

💯

That Add/Delete split didn't get that much better, but I think the call sites read better. So, I guess it's a net win.

[dweitzman]

I'm slowly looking through parts of this diff to have a better understanding of what'll change when we merge it. Sending a few comments/questions.

[dweitzman]

It's not immediately clear to me why it wouldn't be necessarily anymore to update partition information for replica and rdonly served types for a shard even if the master isn't serving from that shard.

[rafael]

• This is one of the core parts of this change. @sougou and I realized that if a master is serving, from a logical perspective that's the only thing you need to know to generate the SrvKeyspaceGraph. You could be in one of these scenarios:

1. User creates a new keyspace with new shards. In init tablet, because there are no overlapping shards IsMasterServing will be set to true. When you build the keyspace graph for this newly created keyspace, this code will generate the serving graph for all types: rdonly, replica, master.
2. If you are creating shards in an existent keyspaces (to prepare for a split), InitTablet will IsMasterServing to false, and rebuild keyspace won't take those new shards into consideration for any type.

When you are in the process of migrating serving types, there is code there that will modify the serving keyspace graph to add/remove new shards accordingly. But that does not go through this code path.

[dweitzman]

Seems like SetShardServedTypes() should probably be deleted along with the vtctl command.

If not, this method probably at least raise an error