From 1380e76004e394a264f637b647ac8c6d45228ba4 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Sat, 22 Jan 2022 22:06:11 +0530 Subject: [PATCH] feat: make fixSemiSync use the durability policy information Signed-off-by: Manan Gupta --- .../vttablet/tabletmanager/rpc_replication.go | 25 +--- .../tabletmanager/rpc_replication_test.go | 119 ------------------ 2 files changed, 4 insertions(+), 140 deletions(-) diff --git a/go/vt/vttablet/tabletmanager/rpc_replication.go b/go/vt/vttablet/tabletmanager/rpc_replication.go index fd4dbfd88e3..a204438d31a 100644 --- a/go/vt/vttablet/tabletmanager/rpc_replication.go +++ b/go/vt/vttablet/tabletmanager/rpc_replication.go @@ -885,29 +885,12 @@ func isPrimaryEligible(tabletType topodatapb.TabletType) bool { } func (tm *TabletManager) fixSemiSync(tabletType topodatapb.TabletType, semiSync SemiSyncAction) error { - if !*enableSemiSync { - // Semi-sync handling is not enabled. - if semiSync == SemiSyncActionTrue { - log.Error("invalid configuration - semi-sync should be setup according to durability policies, but enable_semi_sync is not set") - } - return nil - } - - // Only enable if we're eligible for becoming primary (REPLICA type). - // Ineligible tablets (RDONLY) shouldn't ACK because we'll never promote them. - if !isPrimaryEligible(tabletType) { - if semiSync == SemiSyncActionTrue { - log.Error("invalid configuration - semi-sync should be setup according to durability policies, but the tablet is not primaryEligible") - } + if semiSync == SemiSyncActionTrue { + return tm.MysqlDaemon.SetSemiSyncEnabled(tabletType == topodatapb.TabletType_PRIMARY, true) + } else if semiSync == SemiSyncActionFalse { return tm.MysqlDaemon.SetSemiSyncEnabled(false, false) } - - if semiSync == SemiSyncActionFalse { - log.Error("invalid configuration - enabling semi sync even though not specified by durability policies. Possibly in the process of upgrading.") - } - // Always enable replica-side since it doesn't hurt to keep it on for a primary. - // The primary-side needs to be off for a replica, or else it will get stuck. - return tm.MysqlDaemon.SetSemiSyncEnabled(tabletType == topodatapb.TabletType_PRIMARY, true) + return nil } func (tm *TabletManager) isPrimarySideSemiSyncEnabled() bool { diff --git a/go/vt/vttablet/tabletmanager/rpc_replication_test.go b/go/vt/vttablet/tabletmanager/rpc_replication_test.go index b9608bc7ed2..a6cf48a6a61 100644 --- a/go/vt/vttablet/tabletmanager/rpc_replication_test.go +++ b/go/vt/vttablet/tabletmanager/rpc_replication_test.go @@ -17,16 +17,11 @@ limitations under the License. package tabletmanager import ( - "bytes" "context" - "io" - "os" "testing" "github.com/stretchr/testify/require" - "vitess.io/vitess/go/vt/mysqlctl/fakemysqldaemon" - "vitess.io/vitess/go/vt/proto/topodata" "vitess.io/vitess/go/vt/topo/memorytopo" ) @@ -43,117 +38,3 @@ func TestPromoteReplicaHealthTicksStopped(t *testing.T) { require.NoError(t, err) require.False(t, tm.replManager.ticks.Running()) } - -func captureStderr(f func()) (string, error) { - old := os.Stderr // keep backup of the real stderr - r, w, err := os.Pipe() - if err != nil { - return "", err - } - os.Stderr = w - - outC := make(chan string) - // copy the output in a separate goroutine so printing can't block indefinitely - go func() { - var buf bytes.Buffer - io.Copy(&buf, r) - outC <- buf.String() - }() - - // calling function which stderr we are going to capture: - f() - - // back to normal state - w.Close() - os.Stderr = old // restoring the real stderr - return <-outC, nil -} - -func TestTabletManager_fixSemiSync(t *testing.T) { - tests := []struct { - name string - tabletType topodata.TabletType - semiSync SemiSyncAction - logOutput string - shouldEnableSemiSync bool - }{ - { - name: "enableSemiSync=true(primary eligible),durabilitySemiSync=true", - tabletType: topodata.TabletType_REPLICA, - semiSync: SemiSyncActionTrue, - logOutput: "", - shouldEnableSemiSync: true, - }, { - name: "enableSemiSync=true(primary eligible),durabilitySemiSync=false", - tabletType: topodata.TabletType_REPLICA, - semiSync: SemiSyncActionFalse, - logOutput: "invalid configuration - enabling semi sync even though not specified by durability policies.", - shouldEnableSemiSync: true, - }, { - name: "enableSemiSync=true(primary eligible),durabilitySemiSync=none", - tabletType: topodata.TabletType_REPLICA, - semiSync: SemiSyncActionNone, - logOutput: "", - shouldEnableSemiSync: true, - }, { - name: "enableSemiSync=true(primary not-eligible),durabilitySemiSync=true", - tabletType: topodata.TabletType_DRAINED, - semiSync: SemiSyncActionTrue, - logOutput: "invalid configuration - semi-sync should be setup according to durability policies, but the tablet is not primaryEligible", - shouldEnableSemiSync: true, - }, { - name: "enableSemiSync=true(primary not-eligible),durabilitySemiSync=false", - tabletType: topodata.TabletType_DRAINED, - semiSync: SemiSyncActionFalse, - logOutput: "", - shouldEnableSemiSync: true, - }, { - name: "enableSemiSync=true(primary not-eligible),durabilitySemiSync=none", - tabletType: topodata.TabletType_DRAINED, - semiSync: SemiSyncActionNone, - logOutput: "", - shouldEnableSemiSync: true, - }, { - name: "enableSemiSync=false,durabilitySemiSync=true", - tabletType: topodata.TabletType_REPLICA, - semiSync: SemiSyncActionTrue, - logOutput: "invalid configuration - semi-sync should be setup according to durability policies, but enable_semi_sync is not set", - shouldEnableSemiSync: false, - }, { - name: "enableSemiSync=false,durabilitySemiSync=false", - tabletType: topodata.TabletType_REPLICA, - semiSync: SemiSyncActionFalse, - logOutput: "", - shouldEnableSemiSync: false, - }, { - name: "enableSemiSync=false,durabilitySemiSync=none", - tabletType: topodata.TabletType_REPLICA, - semiSync: SemiSyncActionNone, - logOutput: "", - shouldEnableSemiSync: false, - }, - } - oldEnableSemiSync := *enableSemiSync - defer func() { - *enableSemiSync = oldEnableSemiSync - }() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - *enableSemiSync = tt.shouldEnableSemiSync - fakeMysql := fakemysqldaemon.NewFakeMysqlDaemon(nil) - tm := &TabletManager{ - MysqlDaemon: fakeMysql, - } - logOutput, err := captureStderr(func() { - err := tm.fixSemiSync(tt.tabletType, tt.semiSync) - require.NoError(t, err) - }) - require.NoError(t, err) - if tt.logOutput != "" { - require.Contains(t, logOutput, tt.logOutput) - } else { - require.Equal(t, "", logOutput) - } - }) - } -}