Deprecate twopc_enable flag and change input type for twopc_abandon_age to time.Duration

changelog/22.0/22.0.0/summary.md

@@ -3,6 +3,8 @@
 ### Table of Contents
 
 - **[Major Changes](#major-changes)**
+  - **[Deprecations and Deletions](#deprecations-and-deletions)**
+    - [Deprecated VTTablet Flags](#vttablet-flags)
   - **[RPC Changes](#rpc-changes)**
   - **[Prefer not promoting a replica that is currently taking a backup](#reparents-prefer-not-backing-up)**
 
@@ -15,6 +17,12 @@ These are the RPC changes made in this release -
 
 1. `GetTransactionInfo` RPC has been added to both `VtctldServer`, and `TabletManagerClient` interface. These RPCs are used to fecilitate the users in reading the state of an unresolved distributed transaction. This can be useful in debugging what went wrong and how to fix the problem.
 
+### <a id="deprecations-and-deletions"/>Deprecations and Deletions</a>
+
+#### <a id="vttablet-flags"/>Deprecated VTTablet Flags</a>
+
+- `twopc_enable` flag is deprecated, `twopc_abandonage` flag if set will be used to determine if 2PC is enabled or not.
+
 ### <a id="reparents-prefer-not-backing-up"/>Prefer not promoting a replica that is currently taking a backup
 
 Emergency reparents now prefer not promoting replicas that are currently taking backups with a backup engine other than

go/flags/endtoend/vtcombo.txt

@@ -396,7 +396,6 @@ Flags:
       --transaction_mode string                                          SINGLE: disallow multi-db transactions, MULTI: allow multi-db transactions with best effort commit, TWOPC: allow multi-db transactions with 2pc commit (default "MULTI")
       --truncate-error-len int                                           truncate errors sent to client if they are longer than this value (0 means do not truncate)
       --twopc_abandon_age float                                          time in seconds. Any unresolved transaction older than this time will be sent to the coordinator to be resolved.
-      --twopc_enable                                                     if the flag is on, 2pc is enabled. Other 2pc flags must be supplied.
       --tx-throttler-config string                                       Synonym to -tx_throttler_config (default "target_replication_lag_sec:2 max_replication_lag_sec:10 initial_rate:100 max_increase:1 emergency_decrease:0.5 min_duration_between_increases_sec:40 max_duration_between_increases_sec:62 min_duration_between_decreases_sec:20 spread_backlog_across_sec:20 age_bad_rate_after_sec:180 bad_rate_increase:0.1 max_rate_approach_threshold:0.9")
       --tx-throttler-default-priority int                                Default priority assigned to queries that lack priority information (default 100)
       --tx-throttler-dry-run                                             If present, the transaction throttler only records metrics about requests received and throttled, but does not actually throttle any requests.

go/flags/endtoend/vttablet.txt

@@ -396,7 +396,6 @@ Flags:
       --transaction_limit_by_username                                    Include VTGateCallerID.username when considering who the user is for the purpose of transaction limit. (default true)
       --transaction_limit_per_user float                                 Maximum number of transactions a single user is allowed to use at any time, represented as fraction of -transaction_cap. (default 0.4)
       --twopc_abandon_age float                                          time in seconds. Any unresolved transaction older than this time will be sent to the coordinator to be resolved.
-      --twopc_enable                                                     if the flag is on, 2pc is enabled. Other 2pc flags must be supplied.
       --tx-throttler-config string                                       Synonym to -tx_throttler_config (default "target_replication_lag_sec:2 max_replication_lag_sec:10 initial_rate:100 max_increase:1 emergency_decrease:0.5 min_duration_between_increases_sec:40 max_duration_between_increases_sec:62 min_duration_between_decreases_sec:20 spread_backlog_across_sec:20 age_bad_rate_after_sec:180 bad_rate_increase:0.1 max_rate_approach_threshold:0.9")
       --tx-throttler-default-priority int                                Default priority assigned to queries that lack priority information (default 100)
       --tx-throttler-dry-run                                             If present, the transaction throttler only records metrics about requests received and throttled, but does not actually throttle any requests.

go/test/endtoend/tabletmanager/main_test.go

@@ -105,7 +105,6 @@ func TestMain(m *testing.M) {
 			"--heartbeat_enable",
 			"--health_check_interval", tabletHealthcheckRefreshInterval.String(),
 			"--unhealthy_threshold", tabletUnhealthyThreshold.String(),
-			"--twopc_enable",
 			"--twopc_abandon_age", "200",
 		}
 

go/test/endtoend/transaction/twopc/fuzz/main_test.go

@@ -70,7 +70,6 @@ func TestMain(m *testing.M) {
 			"--tablet_refresh_interval", "2s",
 		)
 		clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs,
-			"--twopc_enable",
 			"--twopc_abandon_age", "1",
 			"--migration_check_interval", "2s",
 		)

go/test/endtoend/transaction/twopc/main_test.go

@@ -82,7 +82,6 @@ func TestMain(m *testing.M) {
 			"--grpc_use_effective_callerid",
 		)
 		clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs,
-			"--twopc_enable",
 			"--twopc_abandon_age", "1",
 			"--queryserver-config-transaction-cap", "3",
 			"--queryserver-config-transaction-timeout", "400s",

go/test/endtoend/transaction/twopc/metric/main_test.go

@@ -69,7 +69,6 @@ func TestMain(m *testing.M) {
 			"--grpc_use_effective_callerid",
 		)
 		clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs,
-			"--twopc_enable",
 			"--twopc_abandon_age", "1",
 			"--queryserver-config-transaction-cap", "100",
 		)

go/test/endtoend/transaction/twopc/stress/main_test.go

@@ -70,7 +70,6 @@ func TestMain(m *testing.M) {
 			"--tablet_refresh_interval", "2s",
 		)
 		clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs,
-			"--twopc_enable",
 			"--twopc_abandon_age", "1",
 			"--migration_check_interval", "2s",
 			"--onterm_timeout", "1s",

go/test/endtoend/transaction/tx_test.go

@@ -57,7 +57,6 @@ func TestMain(m *testing.M) {
 		clusterInstance.VtgateGrpcPort = clusterInstance.GetAndReservePort()
 		// Set extra tablet args for twopc
 		clusterInstance.VtTabletExtraArgs = []string{
-			"--twopc_enable",
 			"--twopc_abandon_age", "3600",
 		}
 

go/vt/vtexplain/vtexplain_vttablet.go

@@ -114,7 +114,6 @@ func (vte *VTExplain) newTablet(ctx context.Context, env *vtenv.Environment, opt
 	config.TrackSchemaVersions = false
 	if opts.ExecutionMode == ModeTwoPC {
 		config.TwoPCAbandonAge = 1.0
-		config.TwoPCEnable = true
 	}
 	config.EnableOnlineDDL = false
 	config.EnableTableGC = false

go/vt/vttablet/endtoend/connecttcp/main_test.go

@@ -86,7 +86,6 @@ func TestMain(m *testing.M) {
 		defer cancel()
 
 		config := tabletenv.NewDefaultConfig()
-		config.TwoPCEnable = true
 		config.TwoPCAbandonAge = 1
 
 		if err := framework.StartCustomServer(ctx, connParams, connAppDebugParams, cluster.DbName(), config); err != nil {

go/vt/vttablet/endtoend/framework/server.go

@@ -108,7 +108,6 @@ func StartCustomServer(ctx context.Context, connParams, connAppDebugParams mysql
 func StartServer(ctx context.Context, connParams, connAppDebugParams mysql.ConnParams, dbName string) error {
 	config := tabletenv.NewDefaultConfig()
 	config.StrictTableACL = true
-	config.TwoPCEnable = true
 	config.TwoPCAbandonAge = 1
 	config.HotRowProtection.Mode = tabletenv.Enable
 	config.TrackSchemaVersions = true

go/vt/vttablet/endtoend/twopc/main_test.go

@@ -83,7 +83,6 @@ func TestMain(m *testing.M) {
 		defer cancel()
 
 		config := tabletenv.NewDefaultConfig()
-		config.TwoPCEnable = true
 		config.TwoPCAbandonAge = 1
 		err := framework.StartCustomServer(ctx, connParams, connAppDebugParams, cluster.DbName(), config)
 		if err != nil {

go/vt/vttablet/tabletserver/dt_executor_test.go

@@ -705,8 +705,10 @@ func TestNoTwopc(t *testing.T) {
 
 	want := "2pc is not enabled"
 	for _, tc := range testcases {
-		err := tc.fun()
-		require.EqualError(t, err, want)
+		t.Run(tc.desc, func(t *testing.T) {
+			err := tc.fun()
+			require.EqualError(t, err, want)
+		})
 	}
 }
 

go/vt/vttablet/tabletserver/query_executor_test.go

@@ -1514,17 +1514,14 @@ func newTestTabletServer(ctx context.Context, flags executorFlags, db *fakesqldb
 	} else {
 		cfg.StrictTableACL = false
 	}
-	if flags&noTwopc > 0 {
-		cfg.TwoPCEnable = false
-	} else {
-		cfg.TwoPCEnable = true
-	}
 	if flags&disableOnlineDDL > 0 {
 		cfg.EnableOnlineDDL = false
 	} else {
 		cfg.EnableOnlineDDL = true
 	}
-	if flags&shortTwopcAge > 0 {
+	if flags&noTwopc > 0 {
+		cfg.TwoPCAbandonAge = 0
+	} else if flags&shortTwopcAge > 0 {
 		cfg.TwoPCAbandonAge = 0.5
 	} else {
 		cfg.TwoPCAbandonAge = 10

go/vt/vttablet/tabletserver/tabletenv/config.go

@@ -156,8 +156,11 @@ func registerTabletEnvFlags(fs *pflag.FlagSet) {
 	fs.BoolVar(&currentConfig.WatchReplication, "watch_replication_stream", false, "When enabled, vttablet will stream the MySQL replication stream from the local server, and use it to update schema when it sees a DDL.")
 	fs.BoolVar(&currentConfig.TrackSchemaVersions, "track_schema_versions", false, "When enabled, vttablet will store versions of schemas at each position that a DDL is applied and allow retrieval of the schema corresponding to a position")
 	fs.Int64Var(&currentConfig.SchemaVersionMaxAgeSeconds, "schema-version-max-age-seconds", 0, "max age of schema version records to kept in memory by the vreplication historian")
-	fs.BoolVar(&currentConfig.TwoPCEnable, "twopc_enable", defaultConfig.TwoPCEnable, "if the flag is on, 2pc is enabled. Other 2pc flags must be supplied.")
+
+	_ = fs.Bool("twopc_enable", true, "TwoPC is enabled")
+	_ = fs.MarkDeprecated("twopc_enable", "TwoPC is always enabled, the transaction abandon age can be configured")
 	SecondsVar(fs, &currentConfig.TwoPCAbandonAge, "twopc_abandon_age", defaultConfig.TwoPCAbandonAge, "time in seconds. Any unresolved transaction older than this time will be sent to the coordinator to be resolved.")
+
 	// Tx throttler config
 	flagutil.DualFormatBoolVar(fs, &currentConfig.EnableTxThrottler, "enable_tx_throttler", defaultConfig.EnableTxThrottler, "If true replication-lag-based throttling on transactions will be enabled.")
 	flagutil.DualFormatVar(fs, currentConfig.TxThrottlerConfig, "tx_throttler_config", "The configuration of the transaction throttler as a text-formatted throttlerdata.Configuration protocol buffer message.")
@@ -335,7 +338,6 @@ type TabletConfig struct {
 	StrictTableACL       bool    `json:"-"`
 	EnableTableACLDryRun bool    `json:"-"`
 	TableACLExemptACL    string  `json:"-"`
-	TwoPCEnable          bool    `json:"-"`
 	TwoPCAbandonAge      Seconds `json:"-"`
 
 	EnableTxThrottler              bool                          `json:"-"`
@@ -1054,6 +1056,8 @@ var defaultConfig = TabletConfig{
 	},
 
 	EnablePerWorkloadTableMetrics: false,
+
+	// TwoPCAbandonAge: Seconds((10 * time.Minute).Seconds()),
 }
 
 // defaultTxThrottlerConfig returns the default TxThrottlerConfigFlag object based on

go/vt/vttablet/tabletserver/tx_engine.go

@@ -116,19 +116,18 @@ func NewTxEngine(env tabletenv.Env, dxNotifier func()) *TxEngine {
 	te.txPool = NewTxPool(env, limiter)
 	// We initially allow twoPC (handles vttablet restarts).
 	// We will disallow them for a few reasons -
-	//	1. when a new tablet is promoted if semi-sync is turned off.
+	//	1. When a new tablet is promoted if semi-sync is turned off.
 	//  2. TabletControls have been set by a Resharding workflow.
 	te.twopcAllowed = make([]bool, TwoPCAllowed_Len)
 	for idx := range te.twopcAllowed {
 		te.twopcAllowed[idx] = true
 	}
-	te.twopcEnabled = config.TwoPCEnable
-	if te.twopcEnabled {
-		if config.TwoPCAbandonAge <= 0 {
-			log.Error("2PC abandon age not specified: Disabling 2PC")
-			te.twopcEnabled = false
-		}
+	te.twopcEnabled = true
+	if config.TwoPCAbandonAge <= 0 {
+		log.Error("2PC abandon age not specified: Disabling 2PC")
+		te.twopcEnabled = false
 	}
+
 	te.abandonAge = config.TwoPCAbandonAge.Get()
 	te.ticks = timer.NewTimer(te.abandonAge / 2)
 

go/vt/vttablet/tabletserver/tx_engine_test.go

@@ -613,7 +613,6 @@ func TestCheckReceivedError(t *testing.T) {
 	cfg := tabletenv.NewDefaultConfig()
 	cfg.DB = newDBConfigs(db)
 	env := tabletenv.NewEnv(vtenv.NewTestEnv(), cfg, "TabletServerTest")
-	env.Config().TwoPCEnable = true
 	env.Config().TwoPCAbandonAge = 5
 	te := NewTxEngine(env, nil)
 	te.AcceptReadWrite()
@@ -791,7 +790,6 @@ func TestPrepareTx(t *testing.T) {
 			db.AddRejectedQuery("retryable query", sqlerror.NewSQLError(sqlerror.CRConnectionError, "", "Retryable error"))
 			cfg := tabletenv.NewDefaultConfig()
 			cfg.DB = newDBConfigs(db)
-			cfg.TwoPCEnable = true
 			cfg.TwoPCAbandonAge = 200
 			te := NewTxEngine(tabletenv.NewEnv(vtenv.NewTestEnv(), cfg, "TabletServerTest"), nil)
 			te.AcceptReadWrite()