Skip to content

Commit

Permalink
Deprecate twopc_enable flag and change input type for `twopc_abando…
Browse files Browse the repository at this point in the history
…n_age` to time.Duration (#17279)

Signed-off-by: Harshit Gangal <[email protected]>
  • Loading branch information
harshit-gangal authored Dec 9, 2024
1 parent 9181115 commit 9b71606
Show file tree
Hide file tree
Showing 22 changed files with 234 additions and 143 deletions.
19 changes: 19 additions & 0 deletions changelog/22.0/22.0.0/summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@
### 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)**
- **[VTOrc Config File Changes](#vtorc-config-file-changes)**
- **[Minor Changes](#minor-changes)**
- **[VTTablet Flags](#flags-vttablet)**


## <a id="major-changes"/>Major Changes</a>
Expand All @@ -16,6 +20,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. Usage of TwoPC commit will be determined by the `transaction_mode` set on VTGate via flag or session variable.

### <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
Expand Down Expand Up @@ -48,3 +58,12 @@ The following fields can be dynamically changed -
13. `change-tablets-with-errant-gtid-to-drained`

To upgrade to the newer version of the configuration file, first switch to using the flags in your current deployment before upgrading. Then you can switch to using the configuration file in the newer release.


## <a id="minor-changes"/>Minor Changes</a>

#### <a id="flags-vttablet"/>VTTablet Flags</a>

- `twopc_abandon_age` flag now supports values in the time.Duration format (e.g., 1s, 2m, 1h).
While the flag will continue to accept float values (interpreted as seconds) for backward compatibility,
**float inputs are deprecated** and will be removed in a future release.
3 changes: 1 addition & 2 deletions go/flags/endtoend/vtcombo.txt
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,7 @@ Flags:
--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)
--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.
--twopc_abandon_age time.Duration Any unresolved transaction older than this time will be sent to the coordinator to be resolved. NOTE: Providing time as seconds (float64) is deprecated. Use time.Duration format (e.g., '1s', '2m', '1h'). (default 15m0s)
--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.
Expand Down
3 changes: 1 addition & 2 deletions go/flags/endtoend/vttablet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,7 @@ Flags:
--transaction_limit_by_subcomponent Include CallerID.subcomponent when considering who the user is for the purpose of transaction limit.
--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.
--twopc_abandon_age time.Duration Any unresolved transaction older than this time will be sent to the coordinator to be resolved. NOTE: Providing time as seconds (float64) is deprecated. Use time.Duration format (e.g., '1s', '2m', '1h'). (default 15m0s)
--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.
Expand Down
71 changes: 71 additions & 0 deletions go/flagutil/float_to_duration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
Copyright 2024 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 flagutil

import (
"errors"
"strconv"
"time"

"github.com/spf13/pflag"
)

// FloatOrDuration is a flag that can be set with either a float64 (interpreted as seconds) or a time.Duration
// The parsed value is stored in the Duration field, and the target pointer is updated with the parsed value
type FloatOrDuration struct {
Target *time.Duration // Pointer to the external duration
Duration time.Duration // Stores the current parsed value
}

// String returns the current value as a string
func (f *FloatOrDuration) String() string {
return f.Duration.String()
}

// Set parses the input and updates the duration
func (f *FloatOrDuration) Set(value string) error {
// Try to parse as float64 first (interpreted as seconds)
if floatVal, err := strconv.ParseFloat(value, 64); err == nil {
f.Duration = time.Duration(floatVal * float64(time.Second))
*f.Target = f.Duration // Update the target pointer
return nil
}

// Try to parse as time.Duration
if duration, err := time.ParseDuration(value); err == nil {
f.Duration = duration
*f.Target = f.Duration // Update the target pointer
return nil
}

return errors.New("value must be either a float64 (interpreted as seconds) or a valid time.Duration")
}

// Type returns the type description
func (f *FloatOrDuration) Type() string {
return "time.Duration"
}

// FloatDuration defines a flag with the specified name, default value, and usage string and binds it to a time.Duration variable.
func FloatDuration(fs *pflag.FlagSet, p *time.Duration, name string, defaultValue time.Duration, usage string) {
*p = defaultValue
fd := FloatOrDuration{
Target: p,
Duration: defaultValue,
}
fs.Var(&fd, name, usage)
}
104 changes: 104 additions & 0 deletions go/flagutil/float_to_duration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
Copyright 2024 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 flagutil

import (
"testing"
"time"

"github.com/spf13/pflag"
"github.com/stretchr/testify/assert"
)

// TestFloatOrDuration_ValidFloat64Input verifies that a float64 input
// (representing seconds) is correctly converted to a time.Duration.
func TestFloatOrDuration_ValidFloat64Input(t *testing.T) {
var duration time.Duration
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)

FloatDuration(fs, &duration, "test_flag", 10*time.Second, "Test flag")
err := fs.Parse([]string{"--test_flag=2"})
assert.NoError(t, err)
assert.Equal(t, 2*time.Second, duration)
}

// TestFloatOrDuration_ValidDurationInput verifies that a valid time.Duration
// input (e.g., "1m30s") is parsed and stored correctly.
func TestFloatOrDuration_ValidDurationInput(t *testing.T) {
var duration time.Duration
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)

FloatDuration(fs, &duration, "test_flag", 10*time.Second, "Test flag")
err := fs.Parse([]string{"--test_flag=1m30s"})
assert.NoError(t, err)
assert.Equal(t, 90*time.Second, duration)
}

// TestFloatOrDuration_DefaultValue ensures that the default value is correctly
// assigned to the duration when the flag is not provided.
func TestFloatOrDuration_DefaultValue(t *testing.T) {
var duration time.Duration
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)

defaultValue := 15 * time.Second
FloatDuration(fs, &duration, "test_flag", defaultValue, "Test flag")
err := fs.Parse([]string{})
assert.NoError(t, err)
assert.Equal(t, defaultValue, duration)
}

// TestFloatOrDuration_InvalidInput verifies that an invalid input string
// results in an appropriate error.
func TestFloatOrDuration_InvalidInput(t *testing.T) {
var duration time.Duration
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)

FloatDuration(fs, &duration, "test_flag", 10*time.Second, "Test flag")
err := fs.Parse([]string{"--test_flag=invalid"})
assert.Error(t, err)
assert.Contains(t, err.Error(), "value must be either a float64 (interpreted as seconds) or a valid time.Duration")
}

// TestFloatOrDuration_MultipleFlags ensures that multiple FloatDuration flags
// can coexist and maintain independent values.
func TestFloatOrDuration_MultipleFlags(t *testing.T) {
var duration1, duration2 time.Duration
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)

FloatDuration(fs, &duration1, "flag1", 10*time.Second, "First test flag")
FloatDuration(fs, &duration2, "flag2", 20*time.Second, "Second test flag")

err := fs.Parse([]string{"--flag1=2.5", "--flag2=1m"})
assert.NoError(t, err)
assert.Equal(t, 2500*time.Millisecond, duration1)
assert.Equal(t, 1*time.Minute, duration2)
}

// TestFloatOrDuration_HelpMessage verifies that the help message includes
// the correct flag name, description, and default value.
func TestFloatOrDuration_HelpMessage(t *testing.T) {
var duration time.Duration
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)

defaultValue := 10 * time.Second
FloatDuration(fs, &duration, "test_flag", defaultValue, "Test flag with default value")

helpOutput := fs.FlagUsages()
assert.Contains(t, helpOutput, "--test_flag time.Duration")
assert.Contains(t, helpOutput, "Test flag with default value")
assert.Contains(t, helpOutput, "(default 10s)")
}
1 change: 0 additions & 1 deletion go/test/endtoend/tabletmanager/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}

Expand Down
1 change: 0 additions & 1 deletion go/test/endtoend/transaction/twopc/fuzz/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
Expand Down
1 change: 0 additions & 1 deletion go/test/endtoend/transaction/twopc/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion go/test/endtoend/transaction/twopc/metric/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
Expand Down
1 change: 0 additions & 1 deletion go/test/endtoend/transaction/twopc/stress/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion go/test/endtoend/transaction/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}

Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtexplain/vtexplain_vttablet.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"reflect"
"strings"
"sync"
"time"

"vitess.io/vitess/go/stats"
"vitess.io/vitess/go/vt/sidecardb"
Expand Down Expand Up @@ -113,8 +114,7 @@ func (vte *VTExplain) newTablet(ctx context.Context, env *vtenv.Environment, opt
config := tabletenv.NewCurrentConfig()
config.TrackSchemaVersions = false
if opts.ExecutionMode == ModeTwoPC {
config.TwoPCAbandonAge = 1.0
config.TwoPCEnable = true
config.TwoPCAbandonAge = 1 * time.Second
}
config.EnableOnlineDDL = false
config.EnableTableGC = false
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vttablet/endtoend/connecttcp/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"os"
"testing"
"time"

"vitess.io/vitess/go/mysql"
vttestpb "vitess.io/vitess/go/vt/proto/vttest"
Expand Down Expand Up @@ -86,8 +87,7 @@ func TestMain(m *testing.M) {
defer cancel()

config := tabletenv.NewDefaultConfig()
config.TwoPCEnable = true
config.TwoPCAbandonAge = 1
config.TwoPCAbandonAge = 1 * time.Second

if err := framework.StartCustomServer(ctx, connParams, connAppDebugParams, cluster.DbName(), config); err != nil {
fmt.Fprintf(os.Stderr, "%v", err)
Expand Down
3 changes: 1 addition & 2 deletions go/vt/vttablet/endtoend/framework/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ 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.TwoPCAbandonAge = 1 * time.Second
config.HotRowProtection.Mode = tabletenv.Enable
config.TrackSchemaVersions = true
config.GracePeriods.Shutdown = 2 * time.Second
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vttablet/endtoend/twopc/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"os"
"testing"
"time"

"vitess.io/vitess/go/mysql"
"vitess.io/vitess/go/vt/vttablet/endtoend/framework"
Expand Down Expand Up @@ -83,8 +84,7 @@ func TestMain(m *testing.M) {
defer cancel()

config := tabletenv.NewDefaultConfig()
config.TwoPCEnable = true
config.TwoPCAbandonAge = 1
config.TwoPCAbandonAge = 1 * time.Second
err := framework.StartCustomServer(ctx, connParams, connAppDebugParams, cluster.DbName(), config)
if err != nil {
fmt.Fprintf(os.Stderr, "%v", err)
Expand Down
6 changes: 4 additions & 2 deletions go/vt/vttablet/tabletserver/dt_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

Expand Down
13 changes: 5 additions & 8 deletions go/vt/vttablet/tabletserver/query_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1514,20 +1514,17 @@ 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 {
cfg.TwoPCAbandonAge = 0.5
if flags&noTwopc > 0 {
cfg.TwoPCAbandonAge = 0
} else if flags&shortTwopcAge > 0 {
cfg.TwoPCAbandonAge = 500 * time.Millisecond
} else {
cfg.TwoPCAbandonAge = 10
cfg.TwoPCAbandonAge = 10 * time.Second
}
if flags&smallResultSize > 0 {
cfg.Oltp.MaxRows = 2
Expand Down
Loading

0 comments on commit 9b71606

Please sign in to comment.