diff --git a/go/vt/topo/keyspace.go b/go/vt/topo/keyspace.go index 9cbd167d174..844b4eb4454 100755 --- a/go/vt/topo/keyspace.go +++ b/go/vt/topo/keyspace.go @@ -43,6 +43,11 @@ import ( // Default concurrency to use in order to avoid overhwelming the topo server. var DefaultConcurrency = 32 +// shardKeySuffix is the suffix of a shard key. +// The full key looks like this: +// /vitess/global/keyspaces/customer/shards/80-/Shard +const shardKeySuffix = "Shard" + func registerFlags(fs *pflag.FlagSet) { fs.IntVar(&DefaultConcurrency, "topo_read_concurrency", DefaultConcurrency, "Concurrency of topo reads.") } @@ -214,6 +219,13 @@ func (ts *Server) FindAllShardsInKeyspace(ctx context.Context, keyspace string, for _, entry := range kvpairs { // The shard key looks like this: /vitess/global/keyspaces/commerce/shards/-80/Shard shardKey := string(entry.Key) + // We don't want keys that aren't Shards. For example: + // /vitess/global/keyspaces/commerce/shards/0/locks/7587876423742065323 + // This example key can happen with Shards because you can get a shard + // lock in the topo via TopoServer.LockShard(). + if path.Base(shardKey) != shardKeySuffix { + continue + } shardName := path.Base(path.Dir(shardKey)) // The base part of the dir is "-80" // Validate the extracted shard name. if _, _, err := ValidateShardName(shardName); err != nil { diff --git a/go/vt/topo/keyspace_external_test.go b/go/vt/topo/keyspace_external_test.go index 4d62c052976..38ff1c8ce7b 100644 --- a/go/vt/topo/keyspace_external_test.go +++ b/go/vt/topo/keyspace_external_test.go @@ -95,6 +95,19 @@ func TestServerGetServingShards(t *testing.T) { keyspace := "ks1" errNoListImpl := topo.NewError(topo.NoImplementation, "don't be doing no listing round here") + // This is needed because memorytopo doesn't implement locks using + // keys in the topo. So we simulate the behavior of other topo server + // implementations and how they implement TopoServer.LockShard(). + createSimulatedShardLock := func(ctx context.Context, ts *topo.Server, keyspace, shard string) error { + conn, err := ts.ConnForCell(ctx, topo.GlobalCell) + if err != nil { + return err + } + lockKey := fmt.Sprintf("keyspaces/%s/shards/%s/locks/1234", keyspace, shard) + _, err = conn.Create(ctx, lockKey, []byte("lock")) + return err + } + tests := []struct { shards int // Number of shards to create err string // Error message we expect, if any @@ -138,10 +151,17 @@ func TestServerGetServingShards(t *testing.T) { if tt.shards > 0 { shardNames, err = key.GenerateShardRanges(tt.shards) require.NoError(t, err) + require.Equal(t, tt.shards, len(shardNames)) for _, shardName := range shardNames { err = ts.CreateShard(ctx, keyspace, shardName) require.NoError(t, err) } + // A shard lock typically becomes a key in the topo like this: + // /vitess/global/keyspaces//shards//locks/XXXX + // We want to confirm that this key is ignored when building + // the results. + err = createSimulatedShardLock(ctx, ts, keyspace, shardNames[0]) + require.NoError(t, err) } // Verify that we return a complete list of shards and that each