diff --git a/go/vt/discovery/healthcheck.go b/go/vt/discovery/healthcheck.go index 0255594cd74..38d070ae801 100644 --- a/go/vt/discovery/healthcheck.go +++ b/go/vt/discovery/healthcheck.go @@ -29,6 +29,7 @@ import ( "time" log "github.com/golang/glog" + "github.com/golang/protobuf/proto" "github.com/youtube/vitess/go/netutil" "github.com/youtube/vitess/go/stats" querypb "github.com/youtube/vitess/go/vt/proto/query" @@ -148,6 +149,21 @@ func (e *TabletStats) String() string { return fmt.Sprint(*e) } +// DeepEqual compares two TabletStats. Since we include protos, we +// need to use proto.Equal on these. +func (e *TabletStats) DeepEqual(f *TabletStats) bool { + return e.Key == f.Key && + proto.Equal(e.Tablet, f.Tablet) && + e.Name == f.Name && + proto.Equal(e.Target, f.Target) && + e.Up == f.Up && + e.Serving == f.Serving && + e.TabletExternallyReparentedTimestamp == f.TabletExternallyReparentedTimestamp && + proto.Equal(e.Stats, f.Stats) && + ((e.LastError == nil && f.LastError == nil) || + (e.LastError != nil && f.LastError != nil && e.LastError.Error() == f.LastError.Error())) +} + // HealthCheck defines the interface of health checking module. // The goal of this object is to maintain a StreamHealth RPC // to a lot of tablets. Tablets are added / removed by calling the diff --git a/go/vt/discovery/replicationlag_test.go b/go/vt/discovery/replicationlag_test.go index 64d2f442b32..27a5ca52a6f 100644 --- a/go/vt/discovery/replicationlag_test.go +++ b/go/vt/discovery/replicationlag_test.go @@ -1,7 +1,6 @@ package discovery import ( - "reflect" "testing" querypb "github.com/youtube/vitess/go/vt/proto/query" @@ -29,7 +28,7 @@ func TestFilterByReplicationLag(t *testing.T) { if len(got) != 1 { t.Errorf("len(FilterByReplicationLag([{Tablet: {Uid: 1}, Serving: true}, {Tablet: {Uid: 2}, Serving: false}])) = %v, want 1", len(got)) } - if len(got) > 0 && !reflect.DeepEqual(got[0], ts1) { + if len(got) > 0 && !got[0].DeepEqual(ts1) { t.Errorf("FilterByReplicationLag([{Tablet: {Uid: 1}, Serving: true}, {Tablet: {Uid: 2}, Serving: false}]) = %+v, want %+v", got[0], ts1) } // lags of (1s, 1s, 1s, 30s) @@ -54,7 +53,7 @@ func TestFilterByReplicationLag(t *testing.T) { Stats: &querypb.RealtimeStats{SecondsBehindMaster: 30}, } got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4}) - if len(got) != 4 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) || !reflect.DeepEqual(got[2], ts3) || !reflect.DeepEqual(got[3], ts4) { + if len(got) != 4 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) || !got[2].DeepEqual(ts3) || !got[3].DeepEqual(ts4) { t.Errorf("FilterByReplicationLag([1s, 1s, 1s, 30s]) = %+v, want all", got) } // lags of (5s, 10s, 15s, 120s) @@ -79,7 +78,7 @@ func TestFilterByReplicationLag(t *testing.T) { Stats: &querypb.RealtimeStats{SecondsBehindMaster: 120}, } got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4}) - if len(got) != 3 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) || !reflect.DeepEqual(got[2], ts3) { + if len(got) != 3 || !got[0].DeepEqual(got[0]) || !got[1].DeepEqual(ts2) || !got[2].DeepEqual(ts3) { t.Errorf("FilterByReplicationLag([5s, 10s, 15s, 120s]) = %+v, want [5s, 10s, 15s]", got) } // lags of (30m, 35m, 40m, 45m) @@ -104,7 +103,7 @@ func TestFilterByReplicationLag(t *testing.T) { Stats: &querypb.RealtimeStats{SecondsBehindMaster: 45 * 60}, } got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4}) - if len(got) != 4 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) || !reflect.DeepEqual(got[2], ts3) || !reflect.DeepEqual(got[3], ts4) { + if len(got) != 4 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) || !got[2].DeepEqual(ts3) || !got[3].DeepEqual(ts4) { t.Errorf("FilterByReplicationLag([30m, 35m, 40m, 45m]) = %+v, want all", got) } // lags of (1s, 1s, 1m, 40m, 40m) - not run filter the second time as first run removed two items. @@ -134,7 +133,7 @@ func TestFilterByReplicationLag(t *testing.T) { Stats: &querypb.RealtimeStats{SecondsBehindMaster: 40 * 60}, } got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4, ts5}) - if len(got) != 3 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) || !reflect.DeepEqual(got[2], ts3) { + if len(got) != 3 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) || !got[2].DeepEqual(ts3) { t.Errorf("FilterByReplicationLag([1s, 1s, 1m, 40m, 40m]) = %+v, want [1s, 1s, 1m]", got) } // lags of (1s, 1s, 10m, 40m) - run filter twice to remove two items @@ -159,7 +158,7 @@ func TestFilterByReplicationLag(t *testing.T) { Stats: &querypb.RealtimeStats{SecondsBehindMaster: 40 * 60}, } got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4}) - if len(got) != 2 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) { + if len(got) != 2 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) { t.Errorf("FilterByReplicationLag([1s, 1s, 10m, 40m]) = %+v, want [1s, 1s]", got) } // lags of (1m, 100m) - return at least 2 items to avoid overloading if the 2nd one is not delayed too much @@ -174,7 +173,7 @@ func TestFilterByReplicationLag(t *testing.T) { Stats: &querypb.RealtimeStats{SecondsBehindMaster: 100 * 60}, } got = FilterByReplicationLag([]*TabletStats{ts1, ts2}) - if len(got) != 2 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) { + if len(got) != 2 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) { t.Errorf("FilterByReplicationLag([1m, 100m]) = %+v, want all", got) } // lags of (1m, 3h) - return 1 if the 2nd one is delayed too much @@ -189,7 +188,7 @@ func TestFilterByReplicationLag(t *testing.T) { Stats: &querypb.RealtimeStats{SecondsBehindMaster: 3 * 60 * 60}, } got = FilterByReplicationLag([]*TabletStats{ts1, ts2}) - if len(got) != 1 || !reflect.DeepEqual(got[0], ts1) { + if len(got) != 1 || !got[0].DeepEqual(ts1) { t.Errorf("FilterByReplicationLag([1m, 3h]) = %+v, want [1m]", got) } } diff --git a/go/vt/discovery/tablet_stats_cache_test.go b/go/vt/discovery/tablet_stats_cache_test.go index 9644e2b9edb..b387e83b0b5 100644 --- a/go/vt/discovery/tablet_stats_cache_test.go +++ b/go/vt/discovery/tablet_stats_cache_test.go @@ -1,7 +1,6 @@ package discovery import ( - "reflect" "testing" "github.com/youtube/vitess/go/vt/topo" @@ -40,11 +39,11 @@ func TestTabletStatsCache(t *testing.T) { // check it's there a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_REPLICA) - if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) { + if len(a) != 1 || !ts1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_REPLICA) - if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) { + if len(a) != 1 || !ts1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } @@ -61,11 +60,11 @@ func TestTabletStatsCache(t *testing.T) { // check the previous ts1 is still there, as the new one is ignored. a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_REPLICA) - if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) { + if len(a) != 1 || !ts1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_REPLICA) - if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) { + if len(a) != 1 || !ts1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } @@ -82,11 +81,11 @@ func TestTabletStatsCache(t *testing.T) { // check it's there a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_REPLICA) - if len(a) != 1 || !reflect.DeepEqual(*notHealthyTs1, a[0]) { + if len(a) != 1 || !notHealthyTs1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_REPLICA) - if len(a) != 1 || !reflect.DeepEqual(*notHealthyTs1, a[0]) { + if len(a) != 1 || !notHealthyTs1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } @@ -110,7 +109,7 @@ func TestTabletStatsCache(t *testing.T) { if a[0].Tablet.Alias.Uid == 11 { a[0], a[1] = a[1], a[0] } - if !reflect.DeepEqual(*ts1, a[0]) || !reflect.DeepEqual(*ts2, a[1]) { + if !ts1.DeepEqual(&a[0]) || !ts2.DeepEqual(&a[1]) { t.Errorf("unexpected result: %v", a) } } @@ -121,7 +120,7 @@ func TestTabletStatsCache(t *testing.T) { if a[0].Tablet.Alias.Uid == 11 { a[0], a[1] = a[1], a[0] } - if !reflect.DeepEqual(*ts1, a[0]) || !reflect.DeepEqual(*ts2, a[1]) { + if !ts1.DeepEqual(&a[0]) || !ts2.DeepEqual(&a[1]) { t.Errorf("unexpected result: %v", a) } } @@ -138,12 +137,12 @@ func TestTabletStatsCache(t *testing.T) { if a[0].Tablet.Alias.Uid == 11 { a[0], a[1] = a[1], a[0] } - if !reflect.DeepEqual(*ts1, a[0]) || !reflect.DeepEqual(*ts2, a[1]) { + if !ts1.DeepEqual(&a[0]) || !ts2.DeepEqual(&a[1]) { t.Errorf("unexpected result: %v", a) } } a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_REPLICA) - if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) { + if len(a) != 1 || !ts1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } @@ -158,13 +157,13 @@ func TestTabletStatsCache(t *testing.T) { // check we only have one replica left a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_REPLICA) - if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) { + if len(a) != 1 || !ts1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } // check we have a master now a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_MASTER) - if len(a) != 1 || !reflect.DeepEqual(*ts2, a[0]) { + if len(a) != 1 || !ts2.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } @@ -182,14 +181,14 @@ func TestTabletStatsCache(t *testing.T) { t.Errorf("unexpected result: %v", a) } a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_MASTER) - if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) { + if len(a) != 1 || !ts1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } // old master sending an old ping should be ignored tsc.StatsUpdate(ts2) a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_MASTER) - if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) { + if len(a) != 1 || !ts1.DeepEqual(&a[0]) { t.Errorf("unexpected result: %v", a) } } diff --git a/go/vt/discovery/utils_test.go b/go/vt/discovery/utils_test.go index fe60b6795b4..41585cb849b 100644 --- a/go/vt/discovery/utils_test.go +++ b/go/vt/discovery/utils_test.go @@ -1,7 +1,6 @@ package discovery import ( - "reflect" "testing" querypb "github.com/youtube/vitess/go/vt/proto/query" @@ -47,8 +46,15 @@ func TestRemoveUnhealthyTablets(t *testing.T) { } for _, tc := range testcases { - if got := RemoveUnhealthyTablets(tc.input); !reflect.DeepEqual(got, tc.want) { + got := RemoveUnhealthyTablets(tc.input) + if len(got) != len(tc.want) { t.Errorf("test case '%v' failed: RemoveUnhealthyTablets(%v) = %#v, want: %#v", tc.desc, tc.input, got, tc.want) + } else { + for i := range tc.want { + if !got[i].DeepEqual(&tc.want[i]) { + t.Errorf("test case '%v' failed: RemoveUnhealthyTablets(%v) = %#v, want: %#v", tc.desc, tc.input, got, tc.want) + } + } } } } diff --git a/go/vt/vtctld/realtime_status_test.go b/go/vt/vtctld/realtime_status_test.go index 152f747fb6a..1e0230237fa 100644 --- a/go/vt/vtctld/realtime_status_test.go +++ b/go/vt/vtctld/realtime_status_test.go @@ -2,7 +2,6 @@ package vtctld import ( "fmt" - "reflect" "testing" "time" @@ -106,7 +105,7 @@ func checkStats(realtimeStats *realtimeStats, tablet *testlib.FakeTablet, want * if err != nil { continue } - if reflect.DeepEqual(result, discovery.TabletStats{}) { + if result.DeepEqual(&discovery.TabletStats{}) { continue } got := result.Stats diff --git a/go/vt/vtctld/tablet_stats_cache_test.go b/go/vt/vtctld/tablet_stats_cache_test.go index 7279329b8ba..98b8cb4f7fa 100644 --- a/go/vt/vtctld/tablet_stats_cache_test.go +++ b/go/vt/vtctld/tablet_stats_cache_test.go @@ -22,26 +22,26 @@ func TestStatsUpdate(t *testing.T) { // Insert tablet1. tabletStatsCache.StatsUpdate(tablet1Stats1) results1 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA] - if got, want := results1[0], tablet1Stats1; !reflect.DeepEqual(got, want) { + if got, want := results1[0], tablet1Stats1; !got.DeepEqual(want) { t.Errorf("got: %v, want: %v", got, want) } // Update tablet1. tabletStatsCache.StatsUpdate(tablet1Stats2) results2 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA] - if got, want := results2[0], tablet1Stats2; !reflect.DeepEqual(got, want) { + if got, want := results2[0], tablet1Stats2; !got.DeepEqual(want) { t.Errorf("got: %v, want: %v", got, want) } // Insert tablet. List of tablets will be resorted. tabletStatsCache.StatsUpdate(tablet2Stats1) results3 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA] - if got, want := results3[0], tablet2Stats1; !reflect.DeepEqual(got, want) { + if got, want := results3[0], tablet2Stats1; !got.DeepEqual(want) { t.Errorf("got: %v, want: %v", got, want) } results4 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA] - if got, want := results4[1], tablet1Stats2; !reflect.DeepEqual(got, want) { + if got, want := results4[1], tablet1Stats2; !got.DeepEqual(want) { t.Errorf("got: %v, want: %v", got, want) } @@ -50,7 +50,7 @@ func TestStatsUpdate(t *testing.T) { tabletStatsCache.StatsUpdate(tablet2Stats1) results5 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA] for _, stat := range results5 { - if reflect.DeepEqual(stat, tablet2Stats1) { + if stat.DeepEqual(tablet2Stats1) { t.Errorf("not deleleted from statusesByAliases") } } @@ -329,14 +329,14 @@ func TestTabletStats(t *testing.T) { // Test 1: tablet1 and tablet2 are updated with the stats received by the HealthCheck module. got1, err := tabletStatsCache.tabletStats(ts1.Tablet.Alias) - want1 := *ts1 - if err != nil || !reflect.DeepEqual(got1, want1) { + want1 := ts1 + if err != nil || !got1.DeepEqual(want1) { t.Errorf("got: %v, want: %v", got1, want1) } got2, err := tabletStatsCache.tabletStats(ts2.Tablet.Alias) - want2 := *ts2 - if err != nil || !reflect.DeepEqual(got2, want2) { + want2 := ts2 + if err != nil || !got2.DeepEqual(want2) { t.Errorf("got: %v, want: %v", got2, want2) }