diff --git a/go/vt/topo/consultopo/version.go b/go/vt/topo/consultopo/version.go index 49071136024..6157ba4dc71 100644 --- a/go/vt/topo/consultopo/version.go +++ b/go/vt/topo/consultopo/version.go @@ -18,8 +18,6 @@ package consultopo import ( "fmt" - - "vitess.io/vitess/go/vt/topo" ) // ConsulVersion is consul's idea of a version. @@ -31,13 +29,3 @@ type ConsulVersion uint64 func (v ConsulVersion) String() string { return fmt.Sprintf("%v", uint64(v)) } - -// VersionFromInt is used by old-style functions to create a proper -// Version: if version is -1, returns nil. Otherwise returns the -// ConsulVersion object. -func VersionFromInt(version int64) topo.Version { - if version == -1 { - return nil - } - return ConsulVersion(version) -} diff --git a/go/vt/topo/etcd2topo/version.go b/go/vt/topo/etcd2topo/version.go index 5fc0a704af8..004719e1522 100644 --- a/go/vt/topo/etcd2topo/version.go +++ b/go/vt/topo/etcd2topo/version.go @@ -18,8 +18,6 @@ package etcd2topo import ( "fmt" - - "vitess.io/vitess/go/vt/topo" ) // EtcdVersion is etcd's idea of a version. @@ -31,13 +29,3 @@ type EtcdVersion int64 func (v EtcdVersion) String() string { return fmt.Sprintf("%v", int64(v)) } - -// VersionFromInt is used by old-style functions to create a proper -// Version: if version is -1, returns nil. Otherwise returns the -// EtcdVersion object. -func VersionFromInt(version int64) topo.Version { - if version == -1 { - return nil - } - return EtcdVersion(version) -} diff --git a/go/vt/topo/helpers/tee.go b/go/vt/topo/helpers/tee.go deleted file mode 100644 index b2178144087..00000000000 --- a/go/vt/topo/helpers/tee.go +++ /dev/null @@ -1,250 +0,0 @@ -/* -Copyright 2019 The Vitess Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package helpers - -import ( - "context" - - "vitess.io/vitess/go/vt/log" - "vitess.io/vitess/go/vt/topo" -) - -// TeeFactory is an implementation of topo.Factory that uses a primary -// underlying topo.Server for all changes, but also duplicates the -// changes to a secondary topo.Server. It also locks both topo servers -// when needed. It is meant to be used during transitions from one -// topo.Server to another. -// -// - primary: we read everything from it, and write to it. We also create -// LeaderParticipation from it. -// - secondary: we write to it as well, but we usually don't fail. -// - we lock primary/secondary if reverseLockOrder is False, -// -// or secondary/primary if reverseLockOrder is True. -type TeeFactory struct { - primary *topo.Server - secondary *topo.Server - reverseLockOrder bool -} - -// HasGlobalReadOnlyCell is part of the topo.Factory interface. -func (f *TeeFactory) HasGlobalReadOnlyCell(serverAddr, root string) bool { - return false -} - -// Create is part of the topo.Factory interface. -func (f *TeeFactory) Create(cell, serverAddr, root string) (topo.Conn, error) { - ctx := context.Background() - primaryConn, err := f.primary.ConnForCell(ctx, cell) - if err != nil { - return nil, err - } - secondaryConn, err := f.secondary.ConnForCell(ctx, cell) - if err != nil { - return nil, err - } - - lockFirst := primaryConn - lockSecond := secondaryConn - if f.reverseLockOrder { - lockFirst = secondaryConn - lockSecond = primaryConn - } - - return &TeeConn{ - primary: primaryConn, - secondary: secondaryConn, - lockFirst: lockFirst, - lockSecond: lockSecond, - }, nil -} - -// NewTee returns a new topo.Server object. It uses a TeeFactory. -func NewTee(primary, secondary *topo.Server, reverseLockOrder bool) (*topo.Server, error) { - f := &TeeFactory{ - primary: primary, - secondary: secondary, - reverseLockOrder: reverseLockOrder, - } - return topo.NewWithFactory(f, "" /*serverAddress*/, "" /*root*/) -} - -// TeeConn implements the topo.Conn interface. -type TeeConn struct { - primary topo.Conn - secondary topo.Conn - - lockFirst topo.Conn - lockSecond topo.Conn -} - -// Close is part of the topo.Conn interface. -func (c *TeeConn) Close() { - c.primary.Close() - c.secondary.Close() -} - -// ListDir is part of the topo.Conn interface. -func (c *TeeConn) ListDir(ctx context.Context, dirPath string, full bool) ([]topo.DirEntry, error) { - return c.primary.ListDir(ctx, dirPath, full) -} - -// Create is part of the topo.Conn interface. -func (c *TeeConn) Create(ctx context.Context, filePath string, contents []byte) (topo.Version, error) { - primaryVersion, err := c.primary.Create(ctx, filePath, contents) - if err != nil { - return nil, err - } - - // This is critical enough that we want to fail. However, we support - // an unconditional update if the file already exists. - _, err = c.secondary.Create(ctx, filePath, contents) - if topo.IsErrType(err, topo.NodeExists) { - _, err = c.secondary.Update(ctx, filePath, contents, nil) - } - if err != nil { - return nil, err - } - - return primaryVersion, nil -} - -// Update is part of the topo.Conn interface. -func (c *TeeConn) Update(ctx context.Context, filePath string, contents []byte, version topo.Version) (topo.Version, error) { - primaryVersion, err := c.primary.Update(ctx, filePath, contents, version) - if err != nil { - // Failed on primary, not updating secondary. - return nil, err - } - - // Always do an unconditional update on secondary. - if _, err = c.secondary.Update(ctx, filePath, contents, nil); err != nil { - log.Warningf("secondary.Update(%v,unconditonal) failed: %v", filePath, err) - } - return primaryVersion, nil -} - -// Get is part of the topo.Conn interface. -func (c *TeeConn) Get(ctx context.Context, filePath string) ([]byte, topo.Version, error) { - return c.primary.Get(ctx, filePath) -} - -// List is part of the topo.Conn interface. -func (c *TeeConn) List(ctx context.Context, filePathPrefix string) ([]topo.KVInfo, error) { - return c.primary.List(ctx, filePathPrefix) -} - -// Delete is part of the topo.Conn interface. -func (c *TeeConn) Delete(ctx context.Context, filePath string, version topo.Version) error { - // If primary fails, no need to go further. - if err := c.primary.Delete(ctx, filePath, version); err != nil { - return err - } - - // Always do an unconditonal delete on secondary. - if err := c.secondary.Delete(ctx, filePath, nil); err != nil && !topo.IsErrType(err, topo.NoNode) { - // Secondary didn't work, and the node wasn't gone already. - log.Warningf("secondary.Delete(%v) failed: %v", filePath, err) - } - - return nil -} - -// Watch is part of the topo.Conn interface -func (c *TeeConn) Watch(ctx context.Context, filePath string) (*topo.WatchData, <-chan *topo.WatchData, error) { - return c.primary.Watch(ctx, filePath) -} - -func (c *TeeConn) WatchRecursive(ctx context.Context, path string) ([]*topo.WatchDataRecursive, <-chan *topo.WatchDataRecursive, error) { - return c.primary.WatchRecursive(ctx, path) -} - -// -// Lock management. -// - -// teeTopoLockDescriptor implements the topo.LockDescriptor interface. -type teeTopoLockDescriptor struct { - c *TeeConn - dirPath string - firstLockDescriptor topo.LockDescriptor - secondLockDescriptor topo.LockDescriptor -} - -// Lock is part of the topo.Conn interface. -func (c *TeeConn) Lock(ctx context.Context, dirPath, contents string) (topo.LockDescriptor, error) { - return c.lock(ctx, dirPath, contents) -} - -// TryLock is part of the topo.Conn interface. Its implementation is same as Lock -func (c *TeeConn) TryLock(ctx context.Context, dirPath, contents string) (topo.LockDescriptor, error) { - return c.Lock(ctx, dirPath, contents) -} - -// Lock is part of the topo.Conn interface. -func (c *TeeConn) lock(ctx context.Context, dirPath, contents string) (topo.LockDescriptor, error) { - // Lock lockFirst. - fLD, err := c.lockFirst.Lock(ctx, dirPath, contents) - if err != nil { - return nil, err - } - - // Lock lockSecond. - sLD, err := c.lockSecond.Lock(ctx, dirPath, contents) - if err != nil { - if err := fLD.Unlock(ctx); err != nil { - log.Warningf("Failed to unlock lockFirst after failed lockSecond lock for %v: %v", dirPath, err) - } - return nil, err - } - - // Remember both locks in teeTopoLockDescriptor. - return &teeTopoLockDescriptor{ - c: c, - dirPath: dirPath, - firstLockDescriptor: fLD, - secondLockDescriptor: sLD, - }, nil -} - -// Check is part of the topo.LockDescriptor interface. -func (ld *teeTopoLockDescriptor) Check(ctx context.Context) error { - if err := ld.firstLockDescriptor.Check(ctx); err != nil { - return err - } - return ld.secondLockDescriptor.Check(ctx) -} - -// Unlock is part of the topo.LockDescriptor interface. -func (ld *teeTopoLockDescriptor) Unlock(ctx context.Context) error { - // Unlock lockSecond, then lockFirst. - serr := ld.secondLockDescriptor.Unlock(ctx) - ferr := ld.firstLockDescriptor.Unlock(ctx) - - if serr != nil { - if ferr != nil { - log.Warningf("First Unlock(%v) failed: %v", ld.dirPath, ferr) - } - return serr - } - return ferr -} - -// NewLeaderParticipation is part of the topo.Conn interface. -func (c *TeeConn) NewLeaderParticipation(name, id string) (topo.LeaderParticipation, error) { - return c.primary.NewLeaderParticipation(name, id) -} diff --git a/go/vt/topo/helpers/tee_test.go b/go/vt/topo/helpers/tee_test.go deleted file mode 100644 index 1fbba807937..00000000000 --- a/go/vt/topo/helpers/tee_test.go +++ /dev/null @@ -1,72 +0,0 @@ -/* -Copyright 2019 The Vitess Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package helpers - -import ( - "context" - "reflect" - "testing" - - "github.com/stretchr/testify/require" - - "vitess.io/vitess/go/vt/sqlparser" - - topodatapb "vitess.io/vitess/go/vt/proto/topodata" -) - -func TestTee(t *testing.T) { - ctx := context.Background() - - // create the setup, copy the data - fromTS, toTS := createSetup(ctx, t) - CopyKeyspaces(ctx, fromTS, toTS, sqlparser.NewTestParser()) - CopyShards(ctx, fromTS, toTS) - CopyTablets(ctx, fromTS, toTS) - - // create a tee and check it implements the interface. - teeTS, err := NewTee(fromTS, toTS, true) - require.NoError(t, err) - - // create a keyspace, make sure it is on both sides - if err := teeTS.CreateKeyspace(ctx, "keyspace2", &topodatapb.Keyspace{}); err != nil { - t.Fatalf("tee.CreateKeyspace(keyspace2) failed: %v", err) - } - teeKeyspaces, err := teeTS.GetKeyspaces(ctx) - if err != nil { - t.Fatalf("tee.GetKeyspaces() failed: %v", err) - } - expected := []string{"keyspace2", "test_keyspace"} - if !reflect.DeepEqual(expected, teeKeyspaces) { - t.Errorf("teeKeyspaces mismatch, got %+v, want %+v", teeKeyspaces, expected) - } - fromKeyspaces, err := fromTS.GetKeyspaces(ctx) - if err != nil { - t.Fatalf("fromTS.GetKeyspaces() failed: %v", err) - } - expected = []string{"keyspace2", "test_keyspace"} - if !reflect.DeepEqual(expected, fromKeyspaces) { - t.Errorf("fromKeyspaces mismatch, got %+v, want %+v", fromKeyspaces, expected) - } - toKeyspaces, err := toTS.GetKeyspaces(ctx) - if err != nil { - t.Fatalf("toTS.GetKeyspaces() failed: %v", err) - } - expected = []string{"keyspace2", "test_keyspace"} - if !reflect.DeepEqual(expected, toKeyspaces) { - t.Errorf("toKeyspaces mismatch, got %+v, want %+v", toKeyspaces, expected) - } -} diff --git a/go/vt/topo/helpers/tee_topo_test.go b/go/vt/topo/helpers/tee_topo_test.go deleted file mode 100644 index 8a4c5690846..00000000000 --- a/go/vt/topo/helpers/tee_topo_test.go +++ /dev/null @@ -1,39 +0,0 @@ -/* -Copyright 2019 The Vitess Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package helpers - -import ( - "testing" - - "vitess.io/vitess/go/test/utils" - "vitess.io/vitess/go/vt/topo" - "vitess.io/vitess/go/vt/topo/memorytopo" - "vitess.io/vitess/go/vt/topo/test" -) - -func TestTeeTopo(t *testing.T) { - ctx := utils.LeakCheckContext(t) - test.TopoServerTestSuite(t, ctx, func() *topo.Server { - s1 := memorytopo.NewServer(ctx, test.LocalCellName) - s2 := memorytopo.NewServer(ctx, test.LocalCellName) - tee, err := NewTee(s1, s2, false) - if err != nil { - t.Fatalf("NewTee() failed: %v", err) - } - return tee - }, []string{"checkTryLock", "checkShardWithLock"}) -} diff --git a/go/vt/topo/memorytopo/version.go b/go/vt/topo/memorytopo/version.go index 468ef6f2110..0cf8ab098d1 100644 --- a/go/vt/topo/memorytopo/version.go +++ b/go/vt/topo/memorytopo/version.go @@ -18,8 +18,6 @@ package memorytopo import ( "fmt" - - "vitess.io/vitess/go/vt/topo" ) // NodeVersion is the local topo.Version implementation @@ -28,13 +26,3 @@ type NodeVersion uint64 func (v NodeVersion) String() string { return fmt.Sprintf("%v", uint64(v)) } - -// VersionFromInt is used by old-style functions to create a proper -// Version: if version is -1, returns nil. Otherwise returns the -// NodeVersion object. -func VersionFromInt(version int64) topo.Version { - if version == -1 { - return nil - } - return NodeVersion(version) -} diff --git a/go/vt/topo/shard.go b/go/vt/topo/shard.go index 7343db2c0ab..b9554bf789f 100644 --- a/go/vt/topo/shard.go +++ b/go/vt/topo/shard.go @@ -77,39 +77,6 @@ func removeCellsFromList(toRemove, fullList []string) []string { return leftoverCells } -// removeCells will remove the cells from the provided list. It returns -// the new list, and a boolean that indicates the returned list is empty. -func removeCells(cells, toRemove, fullList []string) ([]string, bool) { - // The assumption here is we already migrated something, - // and we're reverting that part. So we're gonna remove - // records only. - leftoverCells := make([]string, 0, len(cells)) - if len(cells) == 0 { - // we migrated all the cells already, take the full list - // and remove all the ones we're not reverting - for _, cell := range fullList { - if !InCellList(cell, toRemove) { - leftoverCells = append(leftoverCells, cell) - } - } - } else { - // we migrated a subset of the cells, - // remove the ones we're reverting - for _, cell := range cells { - if !InCellList(cell, toRemove) { - leftoverCells = append(leftoverCells, cell) - } - } - } - - if len(leftoverCells) == 0 { - // we don't have any cell left, we need to clear this record - return nil, true - } - - return leftoverCells, false -} - // IsShardUsingRangeBasedSharding returns true if the shard name // implies it is using range based sharding. func IsShardUsingRangeBasedSharding(shard string) bool { @@ -223,12 +190,7 @@ func (ts *Server) GetShard(ctx context.Context, keyspace, shard string) (*ShardI if err = value.UnmarshalVT(data); err != nil { return nil, vterrors.Wrapf(err, "GetShard(%v,%v): bad shard data", keyspace, shard) } - return &ShardInfo{ - keyspace: keyspace, - shardName: shard, - version: version, - Shard: value, - }, nil + return NewShardInfo(keyspace, shard, value, version), nil } // updateShard updates the shard data, with the right version. diff --git a/go/vt/topo/shard_test.go b/go/vt/topo/shard_test.go index 2c0b9082816..ccef80944a9 100644 --- a/go/vt/topo/shard_test.go +++ b/go/vt/topo/shard_test.go @@ -77,24 +77,6 @@ func TestRemoveCellsFromList(t *testing.T) { } } -func TestRemoveCells(t *testing.T) { - var cells []string - allCells := []string{"first", "second", "third"} - - // remove from empty list should return allCells - what we remove - var emptyResult bool - cells, emptyResult = removeCells(cells, []string{"second"}, allCells) - if emptyResult || !reflect.DeepEqual(cells, []string{"first", "third"}) { - t.Fatalf("removeCells(full)-second failed: got %v", cells) - } - - // removethe next two cells, should return empty list - cells, emptyResult = removeCells(cells, []string{"first", "third"}, allCells) - if !emptyResult { - t.Fatalf("removeCells(full)-first-third is not empty: %v", cells) - } -} - func lockedKeyspaceContext(keyspace string) context.Context { ctx := context.Background() return context.WithValue(ctx, locksKey, &locksInfo{ diff --git a/go/vt/topo/tablet.go b/go/vt/topo/tablet.go index f412233b43a..493e448752b 100644 --- a/go/vt/topo/tablet.go +++ b/go/vt/topo/tablet.go @@ -77,29 +77,6 @@ func IsRunningQueryService(tt topodatapb.TabletType) bool { return false } -// IsSubjectToLameduck returns if a tablet is subject to being -// lameduck. Lameduck is a transition period where we are still -// allowed to serve, but we tell the clients we are going away -// soon. Typically, a vttablet will still serve, but broadcast a -// non-serving state through its health check. then vtgate will catch -// that non-serving state, and stop sending queries. -// -// Primaries are not subject to lameduck, as we usually want to transition -// them as fast as possible. -// -// Replica and rdonly will use lameduck when going from healthy to -// unhealthy (either because health check fails, or they're shutting down). -// -// Other types are probably not serving user visible traffic, so they -// need to transition as fast as possible too. -func IsSubjectToLameduck(tt topodatapb.TabletType) bool { - switch tt { - case topodatapb.TabletType_REPLICA, topodatapb.TabletType_RDONLY: - return true - } - return false -} - // IsRunningUpdateStream returns if a tablet is running the update stream // RPC service. func IsRunningUpdateStream(tt topodatapb.TabletType) bool { @@ -134,36 +111,6 @@ func NewTablet(uid uint32, cell, host string) *topodatapb.Tablet { } } -// TabletEquality returns true iff two Tablet are representing the same tablet -// process: same uid/cell, running on the same host / ports. -func TabletEquality(left, right *topodatapb.Tablet) bool { - if !topoproto.TabletAliasEqual(left.Alias, right.Alias) { - return false - } - if left.Hostname != right.Hostname { - return false - } - if left.MysqlHostname != right.MysqlHostname { - return false - } - if left.MysqlPort != right.MysqlPort { - return false - } - if len(left.PortMap) != len(right.PortMap) { - return false - } - for key, lvalue := range left.PortMap { - rvalue, ok := right.PortMap[key] - if !ok { - return false - } - if lvalue != rvalue { - return false - } - } - return true -} - // TabletInfo is the container for a Tablet, read from the topology server. type TabletInfo struct { version Version // node version - used to prevent stomping concurrent writes diff --git a/go/vt/topo/topoproto/tablet.go b/go/vt/topo/topoproto/tablet.go index 63e71807119..31ac41e14a0 100644 --- a/go/vt/topo/topoproto/tablet.go +++ b/go/vt/topo/topoproto/tablet.go @@ -80,11 +80,6 @@ func TabletAliasString(ta *topodatapb.TabletAlias) string { return fmt.Sprintf("%v-%010d", ta.Cell, ta.Uid) } -// TabletAliasUIDStr returns a string version of the uid -func TabletAliasUIDStr(ta *topodatapb.TabletAlias) string { - return fmt.Sprintf("%010d", ta.Uid) -} - const tabletAliasFormat = "^(?P[-_.a-zA-Z0-9]+)-(?P[0-9]+)$" var tabletAliasRegexp = regexp.MustCompile(tabletAliasFormat) @@ -290,13 +285,6 @@ func TabletDbName(tablet *topodatapb.Tablet) string { return VtDbPrefix + tablet.Keyspace } -// TabletIsAssigned returns if this tablet is assigned to a keyspace and shard. -// A "scrap" node will show up as assigned even though its data cannot be used -// for serving. -func TabletIsAssigned(tablet *topodatapb.Tablet) bool { - return tablet != nil && tablet.Keyspace != "" && tablet.Shard != "" -} - // IsServingType returns true if the tablet type is one that should be serving to be healthy, or false if the tablet type // should not be serving in it's healthy state. func IsServingType(tabletType topodatapb.TabletType) bool { diff --git a/go/vt/topo/zk2topo/zk_conn.go b/go/vt/topo/zk2topo/zk_conn.go index 432fc472db0..22bd2046b60 100644 --- a/go/vt/topo/zk2topo/zk_conn.go +++ b/go/vt/topo/zk2topo/zk_conn.go @@ -76,11 +76,6 @@ func Time(i int64) time.Time { return time.Unix(i/1000, i%1000*1000000) } -// ZkTime returns a ZK time (int64) from a time.Time -func ZkTime(t time.Time) int64 { - return t.Unix()*1000 + int64(t.Nanosecond()/1000000) -} - // ZkConn is a wrapper class on top of a zk.Conn. // It will do a few things for us: // - add the context parameter. However, we do not enforce its deadlines diff --git a/go/vt/topotools/tablet.go b/go/vt/topotools/tablet.go index 76f9e3d6cec..397af9ddf7c 100644 --- a/go/vt/topotools/tablet.go +++ b/go/vt/topotools/tablet.go @@ -45,7 +45,6 @@ import ( "vitess.io/vitess/go/vt/topo/topoproto" "vitess.io/vitess/go/vt/vterrors" - querypb "vitess.io/vitess/go/vt/proto/query" topodatapb "vitess.io/vitess/go/vt/proto/topodata" "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/proto/vttime" @@ -241,11 +240,6 @@ func TabletIdent(tablet *topodatapb.Tablet) string { return fmt.Sprintf("%s-%d (%s%s)", tablet.Alias.Cell, tablet.Alias.Uid, tablet.Hostname, tagStr) } -// TargetIdent returns a concise string representation of a query target -func TargetIdent(target *querypb.Target) string { - return fmt.Sprintf("%s/%s (%s)", target.Keyspace, target.Shard, target.TabletType) -} - // TabletEquality returns true iff two Tablets are identical for testing purposes func TabletEquality(left, right *topodatapb.Tablet) bool { if left.Keyspace != right.Keyspace {