Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unmanaged tablet flag at vttablet level #14871

Merged
merged 11 commits into from
Mar 14, 2024
7 changes: 7 additions & 0 deletions changelog/20.0/20.0.0/summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- **[Major Changes](#major-changes)**
- **[Breaking changes](#breaking-changes)**
- [`shutdown_grace_period` Default Change](#shutdown-grace-period-default)
- [New `unmanaged` Flag and `disable_active_reparents` deprecation](#unmanaged-flag)
- **[Query Compatibility](#query-compatibility)**
- [Vindex Hints](#vindex-hints)
- [Update with Limit Support](#update-limit)
Expand All @@ -31,6 +32,12 @@ This makes reparenting in Vitess resilient to client errors, and prevents Planne

In order to preserve the old behaviour, the users can set the flag back to `0 seconds` causing open transactions to never be shutdown, but in that case, they run the risk of PlannedReparentShard calls timing out.

#### <a id="unmanaged-tablet"/> New `unmanaged` Flag and `disable_active_reparents` deprecation

New flag `--unmanaged` has been introduced in this release to make it easier to flag unmanaged tablets. It also runs validations to make sure the unmanaged tablets are configured properly. `--disable_active_reparents` flag has been deprecated for `vttablet`, `vtcombo` and `vttestserver` binaries and will be removed in future releases. Specifying the `--unmanaged` flag will also block replication commands and replication repairs.

Starting this release, all unmanaged tablets should specify this flag.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People can still not specify this, and just specify the existing set of flags, correct?
We should make this language stronger, and say that we recommend setting this flag so that we can validate all the related flags.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about this again, and I believe we should deprecate --disable_active_reparents along with adding this flag.
For backwards compatibility, we should deprecate --disable_active_reparents in v20, and delete it in v21.
The internal value of mysqlctl.DisableActiveReparents should be the OR of --unmanaged and --disable_active_reparents in v20, and just the value of --unmanaged in v21.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disable_active_reparents is used in vtctldclient as well to prevent that specific instance from running reparent commands. I don't think we can get rid of that. We could rename the flag on vtctld too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fine to leave that flag on vtctld. Having the same flag name on vttablet and vtctld was always confusing. I'm not sure if the two are sharing code which might make this separation more complicated, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 😎 I made the required changes and deprecated the flag only on the required binaries.


### <a id="query-compatibility"/>Query Compatibility

#### <a id="vindex-hints"/> Vindex Hints
Expand Down
2 changes: 1 addition & 1 deletion go/cmd/vttablet/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ See "Unmanaged Tablet" for the full guide.
Even if a MySQL is external, you can still make vttablet perform some management functions. They are as follows:

` +
"* `--disable_active_reparents`: If this flag is set, then any reparent or replica commands will not be allowed. These are InitShardPrimary, PlannedReparentShard, EmergencyReparentShard, and ReparentTablet. In this mode, you should use the TabletExternallyReparented command to inform vitess of the current primary.\n" +
"* `--unmanaged`: This flag indicates that this tablet is running in unmanaged mode. In this mode, any reparent or replica commands are not allowed. These are InitShardPrimary, PlannedReparentShard, EmergencyReparentShard, and ReparentTablet. You should use the TabletExternallyReparented command to inform vitess of the current primary.\n" +
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
"* `--replication_connect_retry`: This value is give to mysql when it connects a replica to the primary as the retry duration parameter.\n" +
"* `--heartbeat_enable` and `--heartbeat_interval duration`: cause vttablet to write heartbeats to the sidecar database. This information is also used by the replication reporter to assess replica lag.\n",
Example: `
Expand Down
2 changes: 1 addition & 1 deletion go/flags/endtoend/vtcombo.txt
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ Flags:
--ddl_strategy string Set default strategy for DDL statements. Override with @@ddl_strategy session variable (default "direct")
--default_tablet_type topodatapb.TabletType The default tablet type to set for queries, when one is not explicitly selected. (default PRIMARY)
--degraded_threshold duration replication lag after which a replica is considered degraded (default 30s)
--disable_active_reparents if set, do not allow active reparents. Use this to protect a cluster using external reparents.
--emit_stats If set, emit stats to push-based monitoring and stats backends
--enable-consolidator Synonym to -enable_consolidator (default true)
--enable-consolidator-replicas Synonym to -enable_consolidator_replicas
Expand Down Expand Up @@ -397,6 +396,7 @@ Flags:
--tx_throttler_config string The configuration of the transaction throttler as a text-formatted throttlerdata.Configuration protocol buffer message. (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_healthcheck_cells strings A comma-separated list of cells. Only tabletservers running in these cells will be monitored for replication lag by the transaction throttler.
--unhealthy_threshold duration replication lag after which a replica is considered unhealthy (default 2h0m0s)
--unmanaged Indicates an unmanaged tablet, i.e. using an external mysql-compatible database
--v Level log level for V logs
-v, --version print binary version
--vmodule vModuleFlag comma-separated list of pattern=N settings for file-filtered logging
Expand Down
4 changes: 2 additions & 2 deletions go/flags/endtoend/vttablet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ See "Unmanaged Tablet" for the full guide.

Even if a MySQL is external, you can still make vttablet perform some management functions. They are as follows:

* `--disable_active_reparents`: If this flag is set, then any reparent or replica commands will not be allowed. These are InitShardPrimary, PlannedReparentShard, EmergencyReparentShard, and ReparentTablet. In this mode, you should use the TabletExternallyReparented command to inform vitess of the current primary.
* `--unmanaged`: This flag indicates that this tablet is running in unmanaged mode. In this mode, any reparent or replica commands are not allowed. These are InitShardPrimary, PlannedReparentShard, EmergencyReparentShard, and ReparentTablet. You should use the TabletExternallyReparented command to inform vitess of the current primary.
* `--replication_connect_retry`: This value is give to mysql when it connects a replica to the primary as the retry duration parameter.
* `--heartbeat_enable` and `--heartbeat_interval duration`: cause vttablet to write heartbeats to the sidecar database. This information is also used by the replication reporter to assess replica lag.

Expand Down Expand Up @@ -139,7 +139,6 @@ Flags:
--dba_idle_timeout duration Idle timeout for dba connections (default 1m0s)
--dba_pool_size int Size of the connection pool for dba connections (default 20)
--degraded_threshold duration replication lag after which a replica is considered degraded (default 30s)
--disable_active_reparents if set, do not allow active reparents. Use this to protect a cluster using external reparents.
--emit_stats If set, emit stats to push-based monitoring and stats backends
--enable-consolidator Synonym to -enable_consolidator (default true)
--enable-consolidator-replicas Synonym to -enable_consolidator_replicas
Expand Down Expand Up @@ -398,6 +397,7 @@ Flags:
--tx_throttler_config string The configuration of the transaction throttler as a text-formatted throttlerdata.Configuration protocol buffer message. (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_healthcheck_cells strings A comma-separated list of cells. Only tabletservers running in these cells will be monitored for replication lag by the transaction throttler.
--unhealthy_threshold duration replication lag after which a replica is considered unhealthy (default 2h0m0s)
--unmanaged Indicates an unmanaged tablet, i.e. using an external mysql-compatible database
--v Level log level for V logs
-v, --version print binary version
--vmodule vModuleFlag comma-separated list of pattern=N settings for file-filtered logging
Expand Down
1 change: 0 additions & 1 deletion go/flags/endtoend/vttestserver.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ Flags:
--dba_idle_timeout duration Idle timeout for dba connections (default 1m0s)
--dba_pool_size int Size of the connection pool for dba connections (default 20)
--default_schema_dir string Default directory for initial schema files. If no schema is found in schema_dir, default to this location.
--disable_active_reparents if set, do not allow active reparents. Use this to protect a cluster using external reparents.
--enable_direct_ddl Allow users to submit direct DDL statements (default true)
--enable_online_ddl Allow users to submit, review and control Online DDL (default true)
--enable_system_settings This will enable the system settings to be changed per session at the database connection level (default true)
Expand Down
10 changes: 9 additions & 1 deletion go/vt/mysqlctl/mysqld.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,12 @@ func init() {
for _, cmd := range []string{"mysqlctl", "mysqlctld", "vtcombo", "vttablet", "vttestserver"} {
servenv.OnParseFor(cmd, registerMySQLDFlags)
}
for _, cmd := range []string{"vtcombo", "vttablet", "vttestserver", "vtctld", "vtctldclient"} {
for _, cmd := range []string{"vtctld", "vtctldclient"} {
servenv.OnParseFor(cmd, registerReparentFlags)
}
for _, cmd := range []string{"vtcombo", "vttablet", "vttestserver"} {
servenv.OnParseFor(cmd, registerDeprecatedReparentFlags)
}
for _, cmd := range []string{"mysqlctl", "mysqlctld", "vtcombo", "vttablet", "vttestserver"} {
servenv.OnParseFor(cmd, registerPoolFlags)
}
Expand All @@ -141,6 +144,11 @@ func registerReparentFlags(fs *pflag.FlagSet) {
fs.BoolVar(&DisableActiveReparents, "disable_active_reparents", DisableActiveReparents, "if set, do not allow active reparents. Use this to protect a cluster using external reparents.")
}

func registerDeprecatedReparentFlags(fs *pflag.FlagSet) {
fs.BoolVar(&DisableActiveReparents, "disable_active_reparents", DisableActiveReparents, "if set, do not allow active reparents. Use this to protect a cluster using external reparents.")
fs.MarkDeprecated("disable_active_reparents", "Use --unmanaged flag instead for unmanaged tablets.")
}

func registerPoolFlags(fs *pflag.FlagSet) {
fs.IntVar(&dbaPoolSize, "dba_pool_size", dbaPoolSize, "Size of the connection pool for dba connections")
fs.DurationVar(&DbaIdleTimeout, "dba_idle_timeout", DbaIdleTimeout, "Idle timeout for dba connections")
Expand Down
54 changes: 54 additions & 0 deletions go/vt/vttablet/tabletserver/tabletenv/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package tabletenv

import (
"context"
"encoding/json"
"errors"
"fmt"
Expand All @@ -27,9 +28,11 @@ import (
"google.golang.org/protobuf/encoding/prototext"

"vitess.io/vitess/go/flagutil"
"vitess.io/vitess/go/mysql"
"vitess.io/vitess/go/streamlog"
"vitess.io/vitess/go/vt/dbconfigs"
"vitess.io/vitess/go/vt/log"
"vitess.io/vitess/go/vt/mysqlctl"
"vitess.io/vitess/go/vt/servenv"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/throttler"
Expand Down Expand Up @@ -213,6 +216,8 @@ func registerTabletEnvFlags(fs *pflag.FlagSet) {
fs.BoolVar(&currentConfig.EnableViews, "queryserver-enable-views", false, "Enable views support in vttablet.")

fs.BoolVar(&currentConfig.EnablePerWorkloadTableMetrics, "enable-per-workload-table-metrics", defaultConfig.EnablePerWorkloadTableMetrics, "If true, query counts and query error metrics include a label that identifies the workload")

fs.BoolVar(&currentConfig.Unmanaged, "unmanaged", false, "Indicates an unmanaged tablet, i.e. using an external mysql-compatible database")
}

var (
Expand Down Expand Up @@ -298,6 +303,8 @@ func Init() {
type TabletConfig struct {
DB *dbconfigs.DBConfigs `json:"db,omitempty"`

Unmanaged bool `json:"unmanaged,omitempty"`

OltpReadPool ConnPoolConfig `json:"oltpReadPool,omitempty"`
OlapReadPool ConnPoolConfig `json:"olapReadPool,omitempty"`
TxPool ConnPoolConfig `json:"txPool,omitempty"`
Expand Down Expand Up @@ -861,6 +868,9 @@ func (c *TabletConfig) TxTimeoutForWorkload(workload querypb.ExecuteOptions_Work

// Verify checks for contradicting flags.
func (c *TabletConfig) Verify() error {
if err := c.verifyUnmanagedTabletConfig(); err != nil {
return err
}
if err := c.verifyTransactionLimitConfig(); err != nil {
return err
}
Expand All @@ -882,6 +892,50 @@ func (c *TabletConfig) Verify() error {
return nil
}

// verifyUnmanagedTabletConfig checks unmanaged tablet related config for sanity
func (c *TabletConfig) verifyUnmanagedTabletConfig() error {
// Skip checks if tablet is not unmanaged
if !c.Unmanaged {
return nil
}

// Throw error if both host and socket are null
if !c.DB.HasGlobalSettings() {
return errors.New("no connection parameters specified but unmanaged mode specified")
}
if c.DB.App.User == "" {
return errors.New("database app user not specified")
}
if c.DB.App.Password == "" {
return errors.New("database app user password not specified")
}
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
// Replication fixes should be disabled for Unmanaged tablets.
mysqlctl.DisableActiveReparents = true

return c.checkConnectionForExternalMysql()
}

// Test connectivity of external mysql
func (c *TabletConfig) checkConnectionForExternalMysql() error {
params := mysql.ConnParams{
Host: c.DB.Host,
Port: c.DB.Port,
DbName: c.DB.DBName,
Uname: c.DB.App.User,
Pass: c.DB.App.Password,
UnixSocket: c.DB.Socket,
}

conn, err := mysql.Connect(context.Background(), &params)
if err != nil {
return err
}
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved

defer conn.Close()

return conn.Ping()
}

// verifyTransactionLimitConfig checks TransactionLimitConfig for sanity
func (c *TabletConfig) verifyTransactionLimitConfig() error {
actual, dryRun := c.EnableTransactionLimit, c.EnableTransactionLimitDryRun
Expand Down
37 changes: 37 additions & 0 deletions go/vt/vttablet/tabletserver/tabletenv/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"vitess.io/vitess/go/mysql/fakesqldb"
"vitess.io/vitess/go/test/utils"
"vitess.io/vitess/go/vt/dbconfigs"
"vitess.io/vitess/go/vt/mysqlctl"
"vitess.io/vitess/go/vt/throttler"
"vitess.io/vitess/go/vt/topo/topoproto"
"vitess.io/vitess/go/vt/vterrors"
Expand Down Expand Up @@ -444,3 +446,38 @@ func TestVerifyTxThrottlerConfig(t *testing.T) {
})
}
}

func TestVerifyUnmanagedTabletConfig(t *testing.T) {
oldDisableActiveReparents := mysqlctl.DisableActiveReparents
defer func() {
mysqlctl.DisableActiveReparents = oldDisableActiveReparents
}()

config := defaultConfig

db := fakesqldb.New(t)
defer db.Close()

params := db.ConnParams()
config.DB = dbconfigs.NewTestDBConfigs(*params, *params, "")

// By default, unmanaged mode should be false
err := config.verifyUnmanagedTabletConfig()
assert.Nil(t, err)

config.Unmanaged = true
err = config.verifyUnmanagedTabletConfig()
assert.EqualError(t, err, "no connection parameters specified but unmanaged mode specified")

config.DB.Socket = db.ConnParams().UnixSocket
err = config.verifyUnmanagedTabletConfig()
assert.EqualError(t, err, "database app user not specified")

config.DB.App.User = "testUser"
err = config.verifyUnmanagedTabletConfig()
assert.EqualError(t, err, "database app user password not specified")

config.DB.App.Password = "testPassword"
err = config.verifyUnmanagedTabletConfig()
assert.Nil(t, err)
}
Loading