From 5aa470967506f7edcff1e1841b494377204d58cd Mon Sep 17 00:00:00 2001 From: akilan Date: Tue, 23 Jun 2020 22:36:18 +0400 Subject: [PATCH] Handled ineffectual errors Signed-off-by: akilan --- go/cmd/zkctld/zkctld.go | 7 +- go/mysql/fakesqldb/server.go | 13 ++- go/mysql/replication_constants.go | 93 +++++++++++-------- go/test/endtoend/keyspace/keyspace_test.go | 6 +- go/test/endtoend/vtgate/buffer/buffer_test.go | 41 ++++++-- go/vt/binlog/binlogplayer/framework_test.go | 8 +- go/vt/discovery/healthcheck.go | 12 ++- go/vt/discovery/healthcheck_test.go | 14 ++- go/vt/discovery/legacy_healthcheck.go | 6 +- .../legacy_healthcheck_flaky_test.go | 8 +- .../legacy_tablet_stats_cache_test.go | 10 +- go/vt/discovery/tablet_picker_test.go | 15 ++- .../max_replication_lag_module_test.go | 8 +- go/vt/throttler/memory_test.go | 42 +++++++-- go/vt/throttler/throttlerz.go | 15 ++- go/vt/topo/consultopo/server_flaky_test.go | 23 ++++- go/vt/topo/etcd2topo/lock.go | 5 +- go/vt/topo/etcd2topo/server_test.go | 12 ++- go/vt/topo/zk2topo/lock.go | 5 +- go/vt/vtgate/discoverygateway_test.go | 6 +- .../vtgate/endtoend/deletetest/delete_test.go | 7 +- sonar-project.properties | 4 +- tools/all_test_for_coverage.sh | 2 +- tools/statsd.go | 2 - 24 files changed, 271 insertions(+), 93 deletions(-) diff --git a/go/cmd/zkctld/zkctld.go b/go/cmd/zkctld/zkctld.go index 674a6b2d464..908b3d4b657 100644 --- a/go/cmd/zkctld/zkctld.go +++ b/go/cmd/zkctld/zkctld.go @@ -69,6 +69,11 @@ func main() { log.Infof("server shut down on its own") case <-sig: log.Infof("signal received, shutting down server") - zkd.Shutdown() + + // Action to perform if there is an error + if err := zkd.Shutdown(); err != nil { + log.Errorf("error during shutdown:%v", err) + exit.Return(1) + } } } diff --git a/go/mysql/fakesqldb/server.go b/go/mysql/fakesqldb/server.go index c7b6f570b56..74adde48b59 100644 --- a/go/mysql/fakesqldb/server.go +++ b/go/mysql/fakesqldb/server.go @@ -29,6 +29,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/sqltypes" @@ -347,7 +349,11 @@ func (db *DB) HandleQuery(c *mysql.Conn, query string, callback func(*sqltypes.R // Check if we should close the connection and provoke errno 2013. if db.shouldClose { c.Close() - callback(&sqltypes.Result{}) + + //log error + if err := callback(&sqltypes.Result{}); err != nil { + log.Errorf("callback failed : %v", err) + } return nil } @@ -355,7 +361,10 @@ func (db *DB) HandleQuery(c *mysql.Conn, query string, callback func(*sqltypes.R // may send this at connection time, and we don't want it to // interfere. if key == "set names utf8" { - callback(&sqltypes.Result{}) + //log error + if err := callback(&sqltypes.Result{}); err != nil { + log.Errorf("callback failed : %v", err) + } return nil } diff --git a/go/mysql/replication_constants.go b/go/mysql/replication_constants.go index 51b8643ab32..da0a4740744 100644 --- a/go/mysql/replication_constants.go +++ b/go/mysql/replication_constants.go @@ -152,42 +152,55 @@ const ( // These constants describe the event types. // See: http://dev.mysql.com/doc/internals/en/binlog-event-type.html const ( - eUnknownEvent = 0 - eStartEventV3 = 1 - eQueryEvent = 2 - eStopEvent = 3 - eRotateEvent = 4 - eIntVarEvent = 5 - eLoadEvent = 6 - eSlaveEvent = 7 - eCreateFileEvent = 8 - eAppendBlockEvent = 9 - eExecLoadEvent = 10 - eDeleteFileEvent = 11 - eNewLoadEvent = 12 - eRandEvent = 13 - eUserVarEvent = 14 + //eUnknownEvent = 0 + // Unused + //eStartEventV3 = 1 + eQueryEvent = 2 + //eStopEvent = 3 + eRotateEvent = 4 + eIntVarEvent = 5 + // Unused + //eLoadEvent = 6 + // Unused + //eSlaveEvent = 7 + // Unused + //eCreateFileEvent = 8 + // Unused + //eAppendBlockEvent = 9 + //eExecLoadEvent = 10 + // Unused + //eDeleteFileEvent = 11 + // Unused + //eNewLoadEvent = 12 + eRandEvent = 13 + // Unused + //eUserVarEvent = 14 eFormatDescriptionEvent = 15 eXIDEvent = 16 - eBeginLoadQueryEvent = 17 - eExecuteLoadQueryEvent = 18 - eTableMapEvent = 19 - eWriteRowsEventV0 = 20 - eUpdateRowsEventV0 = 21 - eDeleteRowsEventV0 = 22 - eWriteRowsEventV1 = 23 - eUpdateRowsEventV1 = 24 - eDeleteRowsEventV1 = 25 - eIncidentEvent = 26 - eHeartbeatEvent = 27 - eIgnorableEvent = 28 - eRowsQueryEvent = 29 - eWriteRowsEventV2 = 30 - eUpdateRowsEventV2 = 31 - eDeleteRowsEventV2 = 32 - eGTIDEvent = 33 - eAnonymousGTIDEvent = 34 - ePreviousGTIDsEvent = 35 + //Unused + //eBeginLoadQueryEvent = 17 + //Unused + //eExecuteLoadQueryEvent = 18 + eTableMapEvent = 19 + eWriteRowsEventV0 = 20 + eUpdateRowsEventV0 = 21 + eDeleteRowsEventV0 = 22 + eWriteRowsEventV1 = 23 + eUpdateRowsEventV1 = 24 + eDeleteRowsEventV1 = 25 + // Unused + //eIncidentEvent = 26 + //eHeartbeatEvent = 27 + // Unused + //eIgnorableEvent = 28 + // Unused + //eRowsQueryEvent = 29 + eWriteRowsEventV2 = 30 + eUpdateRowsEventV2 = 31 + eDeleteRowsEventV2 = 32 + eGTIDEvent = 33 + eAnonymousGTIDEvent = 34 + ePreviousGTIDsEvent = 35 // MySQL 5.7 events. Unused. //eTransactionContextEvent = 36 @@ -195,11 +208,13 @@ const ( //eXAPrepareLogEvent = 38 // MariaDB specific values. They start at 160. - eMariaAnnotateRowsEvent = 160 - eMariaBinlogCheckpointEvent = 161 - eMariaGTIDEvent = 162 - eMariaGTIDListEvent = 163 - eMariaStartEncryptionEvent = 164 + eMariaAnnotateRowsEvent = 160 + // Unused + //eMariaBinlogCheckpointEvent = 161 + eMariaGTIDEvent = 162 + eMariaGTIDListEvent = 163 + // Unused + //eMariaStartEncryptionEvent = 164 ) // These constants describe the type of status variables in q Query packet. diff --git a/go/test/endtoend/keyspace/keyspace_test.go b/go/test/endtoend/keyspace/keyspace_test.go index 0bcff99b6cb..53b4e9ea85e 100644 --- a/go/test/endtoend/keyspace/keyspace_test.go +++ b/go/test/endtoend/keyspace/keyspace_test.go @@ -60,7 +60,7 @@ var ( "column": "keyspace_id", "name": "hash_index" } - ] + ] } } }` @@ -246,7 +246,7 @@ func TestDeleteKeyspace(t *testing.T) { // TODO: Fix this test, not running in CI // tells that in zone2 after deleting shard, there is no shard #264 and in zone1 there is only 1 #269 -func RemoveKeyspaceCell(t *testing.T) { +/*func RemoveKeyspaceCell(t *testing.T) { _ = clusterForKSTest.VtctlclientProcess.ExecuteCommand("CreateKeyspace", "test_delete_keyspace_removekscell") _ = clusterForKSTest.VtctlclientProcess.ExecuteCommand("CreateShard", "test_delete_keyspace_removekscell/0") _ = clusterForKSTest.VtctlclientProcess.ExecuteCommand("CreateShard", "test_delete_keyspace_removekscell/1") @@ -314,7 +314,7 @@ func RemoveKeyspaceCell(t *testing.T) { // Clean up _ = clusterForKSTest.VtctlclientProcess.ExecuteCommand("DeleteKeyspace", "-recursive", "test_delete_keyspace_removekscell") -} +} */ func TestShardCountForAllKeyspaces(t *testing.T) { defer cluster.PanicHandler(t) diff --git a/go/test/endtoend/vtgate/buffer/buffer_test.go b/go/test/endtoend/vtgate/buffer/buffer_test.go index 8c895fe35eb..c632bdbec64 100644 --- a/go/test/endtoend/vtgate/buffer/buffer_test.go +++ b/go/test/endtoend/vtgate/buffer/buffer_test.go @@ -142,7 +142,10 @@ func updateExecute(c *threadParams, conn *mysql.Conn) error { attempt := c.i // Value used in next UPDATE query. Increased after every query. c.i++ - conn.ExecuteFetch("begin", 1000, true) + + if _, err := conn.ExecuteFetch("begin", 1000, true); err != nil { + log.Errorf("begin failed:%v", err) + } result, err := conn.ExecuteFetch(fmt.Sprintf("UPDATE buffer SET msg='update %d' WHERE id = %d", attempt, updateRowID), 1000, true) @@ -269,9 +272,11 @@ func testBufferBase(t *testing.T, isExternalParent bool) { externalReparenting(ctx, t, clusterInstance) } else { //reparent call - clusterInstance.VtctlclientProcess.ExecuteCommand("PlannedReparentShard", "-keyspace_shard", + if err := clusterInstance.VtctlclientProcess.ExecuteCommand("PlannedReparentShard", "-keyspace_shard", fmt.Sprintf("%s/%s", keyspaceUnshardedName, "0"), - "-new_master", clusterInstance.Keyspaces[0].Shards[0].Vttablets[1].Alias) + "-new_master", clusterInstance.Keyspaces[0].Shards[0].Vttablets[1].Alias); err != nil { + log.Errorf("clusterInstance.VtctlclientProcess.ExecuteCommand(\"PlannedRepare... caused an error : %v", err) + } } <-readThreadInstance.waitForNotification @@ -354,7 +359,12 @@ func externalReparenting(ctx context.Context, t *testing.T, clusterInstance *clu newMaster := replica master.VttabletProcess.QueryTablet(demoteMasterQuery, keyspaceUnshardedName, true) if master.VttabletProcess.EnableSemiSync { - master.VttabletProcess.QueryTablet(disableSemiSyncMasterQuery, keyspaceUnshardedName, true) + + //log error + if _, err := master.VttabletProcess.QueryTablet(disableSemiSyncMasterQuery, keyspaceUnshardedName, true); err != nil { + log.Errorf("master.VttabletProcess.QueryTablet(disableSemi... caused an error : %v", err) + } + } // Wait for replica to catch up to master. @@ -368,11 +378,16 @@ func externalReparenting(ctx context.Context, t *testing.T, clusterInstance *clu time.Sleep(time.Duration(w) * time.Second) } - // Promote replica to new master. - replica.VttabletProcess.QueryTablet(promoteSlaveQuery, keyspaceUnshardedName, true) + //Promote replica to new master and log error + if _, err := replica.VttabletProcess.QueryTablet(promoteSlaveQuery, keyspaceUnshardedName, true); err != nil { + log.Errorf("replica.VttabletProcess.QueryTablet(promoteSlaveQuery... caused an error : %v", err) + } if replica.VttabletProcess.EnableSemiSync { - replica.VttabletProcess.QueryTablet(enableSemiSyncMasterQuery, keyspaceUnshardedName, true) + //Log error + if _, err := replica.VttabletProcess.QueryTablet(enableSemiSyncMasterQuery, keyspaceUnshardedName, true); err != nil { + log.Errorf("replica.VttabletProcess.QueryTablet caused an error : %v", err) + } } // Configure old master to replicate from new master. @@ -382,8 +397,14 @@ func externalReparenting(ctx context.Context, t *testing.T, clusterInstance *clu // Use 'localhost' as hostname because Travis CI worker hostnames // are too long for MySQL replication. changeMasterCommands := fmt.Sprintf("RESET SLAVE;SET GLOBAL gtid_slave_pos = '%s';CHANGE MASTER TO MASTER_HOST='%s', MASTER_PORT=%d ,MASTER_USER='vt_repl', MASTER_USE_GTID = slave_pos;START SLAVE;", gtID, "localhost", newMaster.MySQLPort) - oldMaster.VttabletProcess.QueryTablet(changeMasterCommands, keyspaceUnshardedName, true) - // Notify the new vttablet master about the reparent. - clusterInstance.VtctlclientProcess.ExecuteCommand("TabletExternallyReparented", newMaster.Alias) + //Log error + if _, err := oldMaster.VttabletProcess.QueryTablet(changeMasterCommands, keyspaceUnshardedName, true); err != nil { + log.Errorf("oldMaster.VttabletProcess.QueryTablet caused an error : %v", err) + } + + //Notify the new vttablet master about the reparent and Log error + if err := clusterInstance.VtctlclientProcess.ExecuteCommand("TabletExternallyReparented", newMaster.Alias); err != nil { + log.Errorf("clusterInstance.VtctlclientProcess.ExecuteCommand caused an error : %v", err) + } } diff --git a/go/vt/binlog/binlogplayer/framework_test.go b/go/vt/binlog/binlogplayer/framework_test.go index 1db72bc27da..b733000012b 100644 --- a/go/vt/binlog/binlogplayer/framework_test.go +++ b/go/vt/binlog/binlogplayer/framework_test.go @@ -21,6 +21,8 @@ import ( "reflect" "testing" + "vitess.io/vitess/go/vt/log" + "github.com/golang/protobuf/proto" "golang.org/x/net/context" @@ -116,5 +118,9 @@ var globalFBC *fakeBinlogClient func init() { RegisterClientFactory("test", func() Client { return globalFBC }) - flag.Set("binlog_player_protocol", "test") + + //log error + if err := flag.Set("binlog_player_protocol", "test"); err != nil { + log.Errorf("failed to set flag \"binlog_player_protocol\" to \"test\":%v", err) + } } diff --git a/go/vt/discovery/healthcheck.go b/go/vt/discovery/healthcheck.go index 504a367f9d0..1f9e08df99f 100644 --- a/go/vt/discovery/healthcheck.go +++ b/go/vt/discovery/healthcheck.go @@ -715,13 +715,21 @@ func (hc *HealthCheckImpl) ServeHTTP(w http.ResponseWriter, _ *http.Request) { status := hc.CacheStatus() b, err := json.MarshalIndent(status, "", " ") if err != nil { - w.Write([]byte(err.Error())) + //Error logged + if _, err := w.Write([]byte(err.Error())); err != nil { + log.Errorf("write to buffer error failed: %v", err) + } + return } buf := bytes.NewBuffer(nil) json.HTMLEscape(buf, b) - w.Write(buf.Bytes()) + + //Error logged + if _, err := w.Write(buf.Bytes()); err != nil { + log.Errorf("write to buffer bytes failed: %v", err) + } } // servingConnStats returns the number of serving tablets per keyspace/shard/tablet type. diff --git a/go/vt/discovery/healthcheck_test.go b/go/vt/discovery/healthcheck_test.go index 79497b2d80b..71f890254b6 100644 --- a/go/vt/discovery/healthcheck_test.go +++ b/go/vt/discovery/healthcheck_test.go @@ -27,6 +27,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/test/utils" "vitess.io/vitess/go/vt/vttablet/queryservice/fakes" @@ -48,7 +50,11 @@ import ( func init() { tabletconn.RegisterDialer("fake_gateway", tabletDialer) - flag.Set("tablet_protocol", "fake_gateway") + + //log error + if err := flag.Set("tablet_protocol", "fake_gateway"); err != nil { + log.Errorf("failed to set flag \"tablet_protocol\" to \"fake_gateway\":%v", err) + } } func TestHealthCheck(t *testing.T) { @@ -730,7 +736,11 @@ func TestTemplate(t *testing.T) { } func TestDebugURLFormatting(t *testing.T) { - flag.Set("tablet_url_template", "https://{{.GetHostNameLevel 0}}.bastion.{{.Tablet.Alias.Cell}}.corp") + + //log error + if err2 := flag.Set("tablet_url_template", "https://{{.GetHostNameLevel 0}}.bastion.{{.Tablet.Alias.Cell}}.corp"); err2 != nil { + log.Errorf("flag.Set(\"tablet_url_template\", \"https://{{.GetHostNameLevel 0}}.bastion.{{.Tablet.Alias.Cell}}.corp\") failed : %v", err2) + } ParseTabletURLTemplateFromFlag() tablet := topo.NewTablet(0, "cell", "host.dc.domain") diff --git a/go/vt/discovery/legacy_healthcheck.go b/go/vt/discovery/legacy_healthcheck.go index 69522fd8555..736cb3a2f9f 100644 --- a/go/vt/discovery/legacy_healthcheck.go +++ b/go/vt/discovery/legacy_healthcheck.go @@ -222,7 +222,11 @@ func (e LegacyTabletStats) NamedStatusURL() string { // {{.NamedStatusURL}} -> test-0000000001/debug/status func (e LegacyTabletStats) getTabletDebugURL() string { var buffer bytes.Buffer - tabletURLTemplate.Execute(&buffer, e) + + //Error logged + if err := tabletURLTemplate.Execute(&buffer, e); err != nil { + log.Errorf("tabletURLTemplate.Execute(&buffer, e) failed: %v", err) + } return buffer.String() } diff --git a/go/vt/discovery/legacy_healthcheck_flaky_test.go b/go/vt/discovery/legacy_healthcheck_flaky_test.go index 8d28ad60abd..7f4beebef80 100644 --- a/go/vt/discovery/legacy_healthcheck_flaky_test.go +++ b/go/vt/discovery/legacy_healthcheck_flaky_test.go @@ -26,6 +26,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/log" + "golang.org/x/net/context" "vitess.io/vitess/go/vt/grpcclient" "vitess.io/vitess/go/vt/status" @@ -42,7 +44,11 @@ var connMap map[string]*fakeConn func init() { tabletconn.RegisterDialer("fake_discovery", discoveryDialer) - flag.Set("tablet_protocol", "fake_discovery") + + //log error + if err := flag.Set("tablet_protocol", "fake_discovery"); err != nil { + log.Errorf("flag.Set(\"tablet_protocol\", \"fake_discovery\") failed : %v", err) + } connMap = make(map[string]*fakeConn) } diff --git a/go/vt/discovery/legacy_tablet_stats_cache_test.go b/go/vt/discovery/legacy_tablet_stats_cache_test.go index 4a50109cafe..60cc2a5a695 100644 --- a/go/vt/discovery/legacy_tablet_stats_cache_test.go +++ b/go/vt/discovery/legacy_tablet_stats_cache_test.go @@ -20,6 +20,8 @@ import ( "context" "testing" + "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/topo" "vitess.io/vitess/go/vt/topo/memorytopo" @@ -35,7 +37,9 @@ func TestLegacyTabletStatsCache(t *testing.T) { Cells: []string{"cell", "cell1"}, } - ts.CreateCellsAlias(context.Background(), "region1", cellsAlias) + if err := ts.CreateCellsAlias(context.Background(), "region1", cellsAlias); err != nil { + log.Errorf("creating cellsAlias \"region1\" failed: %v", err) + } defer ts.DeleteCellsAlias(context.Background(), "region1") @@ -43,7 +47,9 @@ func TestLegacyTabletStatsCache(t *testing.T) { Cells: []string{"cell2"}, } - ts.CreateCellsAlias(context.Background(), "region2", cellsAlias) + if err := ts.CreateCellsAlias(context.Background(), "region2", cellsAlias); err != nil { + log.Errorf("creating cellsAlias \"region2\" failed: %v", err) + } defer ts.DeleteCellsAlias(context.Background(), "region2") diff --git a/go/vt/discovery/tablet_picker_test.go b/go/vt/discovery/tablet_picker_test.go index dcbd84e649d..5ff24df64c7 100644 --- a/go/vt/discovery/tablet_picker_test.go +++ b/go/vt/discovery/tablet_picker_test.go @@ -20,6 +20,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/log" + "github.com/golang/protobuf/proto" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -169,7 +171,14 @@ func addTablet(te *pickerTestEnv, id int, tabletType topodatapb.TabletType, serv } func deleteTablet(te *pickerTestEnv, tablet *topodatapb.Tablet) { - te.topoServ.DeleteTablet(context.Background(), tablet.Alias) - // This is not automatically removed from shard replication, which results in log spam. - topo.DeleteTabletReplicationData(context.Background(), te.topoServ, tablet) + + //log error + if err := te.topoServ.DeleteTablet(context.Background(), tablet.Alias); err != nil { + log.Errorf("failed to DeleteTablet with alias : %v", err) + } + + //This is not automatically removed from shard replication, which results in log spam and log error + if err := topo.DeleteTabletReplicationData(context.Background(), te.topoServ, tablet); err != nil { + log.Errorf("failed to automatically remove from shard replication: %v", err) + } } diff --git a/go/vt/throttler/max_replication_lag_module_test.go b/go/vt/throttler/max_replication_lag_module_test.go index 812cde0a0ad..e1514722542 100644 --- a/go/vt/throttler/max_replication_lag_module_test.go +++ b/go/vt/throttler/max_replication_lag_module_test.go @@ -22,6 +22,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/discovery" querypb "vitess.io/vitess/go/vt/proto/query" @@ -451,8 +453,10 @@ func TestMaxReplicationLagModule_Increase_BadRateUpperBound(t *testing.T) { t.Fatal(err) } - // Assume that a bad value of 150 was set @ 30s. - tf.m.memory.markBad(150, sinceZero(30*time.Second)) + //Assume that a bad value of 150 was set @ 30s and log error + if err := tf.m.memory.markBad(150, sinceZero(30*time.Second)); err != nil { + log.Errorf("tf.m.memory.markBad(150, sinceZero(30*time.Second)) falied : %v", err) + } // r2 @ 70s, 0s lag tf.ratesHistory.add(sinceZero(69*time.Second), 100) diff --git a/go/vt/throttler/memory_test.go b/go/vt/throttler/memory_test.go index cb5cba582a2..899e175672a 100644 --- a/go/vt/throttler/memory_test.go +++ b/go/vt/throttler/memory_test.go @@ -19,21 +19,37 @@ package throttler import ( "testing" "time" + + "vitess.io/vitess/go/vt/log" ) func TestMemory(t *testing.T) { m := newMemory(5, 1*time.Second, 0.10) + // Add several good rates. - m.markGood(201) + if err := m.markGood(201); err != nil { + log.Errorf("m.markGood(201) failed :%v ", err) + } + want200 := int64(200) if got := m.highestGood(); got != want200 { t.Fatalf("memory with one good entry: got = %v, want = %v", got, want200) } - m.markGood(101) + + //log error + if err := m.markGood(101); err != nil { + log.Errorf("m.markGood(101) failed :%v ", err) + } + if got := m.highestGood(); got != want200 { t.Fatalf("wrong order within memory: got = %v, want = %v", got, want200) } - m.markGood(301) + + //log error + if err := m.markGood(301); err != nil { + log.Errorf(" m.markGood(301) failed :%v ", err) + } + want300 := int64(300) if got := m.highestGood(); got != want300 { t.Fatalf("wrong order within memory: got = %v, want = %v", got, want300) @@ -48,7 +64,12 @@ func TestMemory(t *testing.T) { if got := m.lowestBad(); got != 0 { t.Fatalf("lowestBad should return zero value when no bad rate is recorded yet: got = %v", got) } - m.markBad(300, sinceZero(0)) + + //log error + if err := m.markBad(300, sinceZero(0)); err != nil { + log.Errorf(" m.markBad(300, sinceZero(0)) failed :%v ", err) + } + if got, want := m.lowestBad(), want300; got != want { t.Fatalf("bad rate was not recorded: got = %v, want = %v", got, want) } @@ -56,7 +77,11 @@ func TestMemory(t *testing.T) { t.Fatalf("new lower bad rate did not invalidate previous good rates: got = %v, want = %v", got, want200) } - m.markBad(311, sinceZero(0)) + //log error + if err := m.markBad(311, sinceZero(0)); err != nil { + log.Errorf(" m.markBad(311, sinceZero(0)) failed :%v ", err) + } + if got := m.lowestBad(); got != want300 { t.Fatalf("bad rates higher than the current one should be ignored: got = %v, want = %v", got, want300) } @@ -73,7 +98,12 @@ func TestMemory(t *testing.T) { } // 199 will be rounded up to 200. - m.markBad(199, sinceZero(0)) + err := m.markBad(199, sinceZero(0)) + + if err != nil { + t.Fatalf(" m.markBad(199, sinceZero(0)) failed :%v ", err) + } + if got := m.lowestBad(); got != want200 { t.Fatalf("bad rate was not updated: got = %v, want = %v", got, want200) } diff --git a/go/vt/throttler/throttlerz.go b/go/vt/throttler/throttlerz.go index 9e3dbf544ed..1793a9c33d4 100644 --- a/go/vt/throttler/throttlerz.go +++ b/go/vt/throttler/throttlerz.go @@ -21,6 +21,8 @@ import ( "html/template" "net/http" "strings" + + "vitess.io/vitess/go/vt/log" ) const listHTML = ` @@ -73,11 +75,18 @@ func throttlerzHandler(w http.ResponseWriter, r *http.Request, m *managerImpl) { func listThrottlers(w http.ResponseWriter, m *managerImpl) { throttlers := m.Throttlers() - listTemplate.Execute(w, map[string]interface{}{ + + // Log error + if err := listTemplate.Execute(w, map[string]interface{}{ "Throttlers": throttlers, - }) + }); err != nil { + log.Errorf("listThrottlers failed :%v", err) + } } func showThrottlerDetails(w http.ResponseWriter, name string) { - detailsTemplate.Execute(w, name) + // Log error + if err := detailsTemplate.Execute(w, name); err != nil { + log.Errorf("showThrottlerDetails failed :%v", err) + } } diff --git a/go/vt/topo/consultopo/server_flaky_test.go b/go/vt/topo/consultopo/server_flaky_test.go index ab641309ac9..f926782f4d6 100644 --- a/go/vt/topo/consultopo/server_flaky_test.go +++ b/go/vt/topo/consultopo/server_flaky_test.go @@ -26,6 +26,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/log" + "github.com/hashicorp/consul/api" "golang.org/x/net/context" "vitess.io/vitess/go/testfiles" @@ -129,8 +131,15 @@ func TestConsulTopo(t *testing.T) { // Start a single consul in the background. cmd, configFilename, serverAddr := startConsul(t, "") defer func() { - cmd.Process.Kill() - cmd.Wait() + // Alerts command did not run successful + if err := cmd.Process.Kill(); err != nil { + log.Errorf("cmd process kill has an error: %v", err) + } + // Alerts command did not run successful + if err := cmd.Wait(); err != nil { + log.Errorf("cmd wait has an error: %v", err) + } + os.Remove(configFilename) }() @@ -166,8 +175,14 @@ func TestConsulTopoWithAuth(t *testing.T) { // Start a single consul in the background. cmd, configFilename, serverAddr := startConsul(t, "123456") defer func() { - cmd.Process.Kill() - cmd.Wait() + // Alerts command did not run successful + if err := cmd.Process.Kill(); err != nil { + log.Errorf("cmd process kill has an error: %v", err) + } + // Alerts command did not run successful + if err := cmd.Wait(); err != nil { + log.Errorf("cmd process wait has an error: %v", err) + } os.Remove(configFilename) }() diff --git a/go/vt/topo/etcd2topo/lock.go b/go/vt/topo/etcd2topo/lock.go index 9827a6af4e3..2ec54a1891c 100644 --- a/go/vt/topo/etcd2topo/lock.go +++ b/go/vt/topo/etcd2topo/lock.go @@ -56,7 +56,10 @@ func (s *Server) newUniqueEphemeralKV(ctx context.Context, cli *clientv3.Client, // succeeded or not. In any case, let's try to // delete the node, so we don't leave an orphan // node behind for *leaseTTL time. - cli.Delete(context.Background(), newKey) + + if _, err := cli.Delete(context.Background(), newKey); err != nil { + log.Errorf("cli.Delete(context.Background(), newKey) failed :%v", err) + } } return "", 0, convertError(err, newKey) } diff --git a/go/vt/topo/etcd2topo/server_test.go b/go/vt/topo/etcd2topo/server_test.go index ffe72c4b13f..076b0b92e62 100644 --- a/go/vt/topo/etcd2topo/server_test.go +++ b/go/vt/topo/etcd2topo/server_test.go @@ -25,6 +25,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/tlstest" "github.com/coreos/etcd/pkg/transport" @@ -181,8 +183,14 @@ func startEtcdWithTLS(t *testing.T) (string, *tlstest.ClientServerKeyPairs, func } stopEtcd := func() { - cmd.Process.Kill() - cmd.Wait() + // log error + if err := cmd.Process.Kill(); err != nil { + log.Errorf("cmd.Process.Kill() failed : %v", err) + } + // log error + if err := cmd.Wait(); err != nil { + log.Errorf("cmd.wait() failed : %v", err) + } os.RemoveAll(dataDir) } diff --git a/go/vt/topo/zk2topo/lock.go b/go/vt/topo/zk2topo/lock.go index 6f4138f2a14..fdd8da40ceb 100644 --- a/go/vt/topo/zk2topo/lock.go +++ b/go/vt/topo/zk2topo/lock.go @@ -61,7 +61,10 @@ func (zs *Server) Lock(ctx context.Context, dirPath, contents string) (topo.Lock // Regardless of the reason, try to cleanup. log.Warningf("Failed to obtain action lock: %v", err) - zs.conn.Delete(ctx, nodePath, -1) + + if err := zs.conn.Delete(ctx, nodePath, -1); err != nil { + log.Warningf("Failed to close connection :%v", err) + } // Show the other locks in the directory dir := path.Dir(nodePath) diff --git a/go/vt/vtgate/discoverygateway_test.go b/go/vt/vtgate/discoverygateway_test.go index d2fc547f293..678e580350f 100644 --- a/go/vt/vtgate/discoverygateway_test.go +++ b/go/vt/vtgate/discoverygateway_test.go @@ -21,6 +21,8 @@ import ( "strings" "testing" + "vitess.io/vitess/go/vt/log" + "golang.org/x/net/context" "vitess.io/vitess/go/sqltypes" @@ -247,7 +249,9 @@ func TestDiscoveryGatewayGetTabletsWithRegion(t *testing.T) { dg := NewDiscoveryGateway(context.Background(), hc, srvTopo, "local", 2) - ts.CreateCellsAlias(context.Background(), "local", cellsAlias) + if err := ts.CreateCellsAlias(context.Background(), "local", cellsAlias); err != nil { + log.Errorf("ts.CreateCellsAlias(context.Background()... %v", err) + } defer ts.DeleteCellsAlias(context.Background(), "local") diff --git a/go/vt/vtgate/endtoend/deletetest/delete_test.go b/go/vt/vtgate/endtoend/deletetest/delete_test.go index 2263ca62723..82c9c359c5b 100644 --- a/go/vt/vtgate/endtoend/deletetest/delete_test.go +++ b/go/vt/vtgate/endtoend/deletetest/delete_test.go @@ -23,6 +23,8 @@ import ( "os" "testing" + "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/sqltypes" vschemapb "vitess.io/vitess/go/vt/proto/vschema" @@ -140,7 +142,10 @@ func TestMain(m *testing.M) { } if err := cluster.Setup(); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) - cluster.TearDown() + //log error + if err := cluster.TearDown(); err != nil { + log.Errorf("cluster.TearDown() did not work: ", err) + } return 1 } defer cluster.TearDown() diff --git a/sonar-project.properties b/sonar-project.properties index 7ae2bcf9864..83cda29a829 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -5,8 +5,8 @@ sonar.exclusions=**/*_test.go,**/vendor/**,**/java/**,go/test/endtoend/**,**/*.p sonar.tests=. sonar.test.inclusions=**/*_test.go sonar.test.exclusions=**/vendor/** - - +## Path of coverage report +sonar.go.coverage.reportPaths=/tmp/*.out # Change the host.url to point to the # sonarqube server (default localhost) sonar.host.url=http://localhost:9000 diff --git a/tools/all_test_for_coverage.sh b/tools/all_test_for_coverage.sh index e01298f185b..07fff9f40cb 100755 --- a/tools/all_test_for_coverage.sh +++ b/tools/all_test_for_coverage.sh @@ -31,7 +31,7 @@ all_except_endtoend_tests=$(echo "$packages_with_all_tests" | grep -v "endtoend" counter=0 for pkg in $all_except_endtoend_tests do - go test -coverpkg=vitess.io/vitess/go/... -coverprofile "/tmp/unit_$counter.out" $pkg -v -p=1 || : + go test -coverpkg=vitess.io/vitess/go/... -coverprofile "/tmp/unit_$counter.out" -json > "/tmp/unit_$counter.json" $pkg -v -p=1 || : counter=$((counter+1)) done diff --git a/tools/statsd.go b/tools/statsd.go index f2ccd91e69b..3699d7ba864 100644 --- a/tools/statsd.go +++ b/tools/statsd.go @@ -82,8 +82,6 @@ type Stats struct { type TestStats struct { Pass, Fail, Flake int PassTime time.Duration - - name string } func testPassed(name string, passTime time.Duration) {