Improve VTOrc config handling to support dynamic variables

changelog/22.0/22.0.0/summary.md

@@ -4,6 +4,7 @@
 
 - **[Major Changes](#major-changes)**
   - **[RPC Changes](#rpc-changes)**
+  - **[VTOrc Config File Changes](#vtorc-config-file-changes)**
 
 
 ## <a id="major-changes"/>Major Changes</a>
@@ -13,3 +14,22 @@
 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="vtorc-config-file-changes"/>VTOrc Config File Changes</a>
+
+The configuration file for VTOrc has been updated to now support dynamic fields. The old `--config` parameter has been removed. The alternative is to use the `--config-file` parameter. The configuration can now be provided in both json, yaml or any other format that [viper](https://github.com/spf13/viper) supports.
+
+The following fields can be dynamically changed - 
+1. `InstancePollTime`
+2. `PreventCrossCellFailover`
+3. `SnapshotTopologyInterval`
+4. `ReasonableReplicationLag`
+5. `AuditToBackend`
+6. `AuditToSyslog`
+7. `AuditPurgeDuration`
+8. `WaitReplicasTimeout`
+9. `TolerableReplicationLag`
+10. `TopoInformationRefreshDuration`
+11. `RecoveryPollDuration`
+
+To upgrade to the newer version of the configuration file, the users can first change to using the flags in the previous release before upgrading. They can then revert to using the configuration file in the newer release.

examples/common/scripts/vtorc-up.sh

@@ -11,7 +11,7 @@ vtorc \
   $TOPOLOGY_FLAGS \
   --logtostderr \
   --alsologtostderr \
-  --config="${script_dir}/../vtorc/config.json" \
+  --config-path="${script_dir}/../vtorc/config.json" \
   --port $port \
   > "${log_dir}/vtorc.out" 2>&1 &
 

examples/common/vtorc/config.json

@@ -1,4 +1,3 @@
 {
-  "RecoveryPeriodBlockSeconds": 1,
-  "InstancePollSeconds": 1
+  "InstancePollTime": "1s"
 }

go/cmd/vtorc/cli/cli.go

@@ -29,8 +29,7 @@ import (
 )
 
 var (
-	configFile string
-	Main       = &cobra.Command{
+	Main = &cobra.Command{
 		Use:   "vtorc",
 		Short: "VTOrc is the automated fault detection and repair tool in Vitess.",
 		Example: `vtorc \
@@ -51,16 +50,10 @@ var (
 
 func run(cmd *cobra.Command, args []string) {
 	servenv.Init()
-	config.UpdateConfigValuesFromFlags()
 	inst.RegisterStats()
 
 	log.Info("starting vtorc")
-	if len(configFile) > 0 {
-		config.ForceRead(configFile)
-	} else {
-		config.Read("/etc/vtorc.conf.json", "conf/vtorc.conf.json", "vtorc.conf.json")
-	}
-	if config.Config.AuditToSyslog {
+	if config.GetAuditToSyslog() {
 		inst.EnableAuditSyslog()
 	}
 	config.MarkConfigurationLoaded()
@@ -96,7 +89,5 @@ func init() {
 	servenv.MoveFlagsToCobraCommand(Main)
 
 	logic.RegisterFlags(Main.Flags())
-	config.RegisterFlags(Main.Flags())
 	acl.RegisterFlags(Main.Flags())
-	Main.Flags().StringVar(&configFile, "config", "", "config file name")
 }

go/flags/endtoend/vtorc.txt

@@ -25,7 +25,6 @@ Flags:
       --catch-sigpipe                                               catch and ignore SIGPIPE on stdout and stderr if specified
       --change-tablets-with-errant-gtid-to-drained                  Whether VTOrc should be changing the type of tablets with errant GTIDs to DRAINED
       --clusters_to_watch strings                                   Comma-separated list of keyspaces or keyspace/shards that this instance will monitor and repair. Defaults to all clusters in the topology. Example: "ks1,ks2/-80"
-      --config string                                               config file name
       --config-file string                                          Full path of the config file (with extension) to use. If set, --config-path, --config-type, and --config-name are ignored.
       --config-file-not-found-handling ConfigFileNotFoundHandling   Behavior when a config file is not found. (Options: error, exit, ignore, warn) (default warn)
       --config-name string                                          Name of the config file (without extension) to search for. (default "vtconfig")

go/test/endtoend/cluster/vtorc_process.go

@@ -49,14 +49,12 @@ type VTOrcProcess struct {
 }
 
 type VTOrcConfiguration struct {
-	Debug                                 bool
-	ListenAddress                         string
-	RecoveryPeriodBlockSeconds            int
-	TopologyRefreshSeconds                int    `json:",omitempty"`
-	PreventCrossDataCenterPrimaryFailover bool   `json:",omitempty"`
-	LockShardTimeoutSeconds               int    `json:",omitempty"`
-	ReplicationLagQuery                   string `json:",omitempty"`
-	FailPrimaryPromotionOnLagMinutes      int    `json:",omitempty"`
+	InstancePollTime                 string `json:",omitempty"`
+	SnapshotTopologyInterval         string `json:",omitempty"`
+	PreventCrossCellFailover         bool   `json:",omitempty"`
+	LockShardTimeoutSeconds          int    `json:",omitempty"`
+	ReplicationLagQuery              string `json:",omitempty"`
+	FailPrimaryPromotionOnLagMinutes int    `json:",omitempty"`
 }
 
 // ToJSONString will marshal this configuration as JSON
@@ -66,11 +64,11 @@ func (config *VTOrcConfiguration) ToJSONString() string {
 }
 
 func (config *VTOrcConfiguration) AddDefaults(webPort int) {
-	config.Debug = true
-	if config.RecoveryPeriodBlockSeconds == 0 {
-		config.RecoveryPeriodBlockSeconds = 1
-	}
-	config.ListenAddress = fmt.Sprintf(":%d", webPort)
+	config.InstancePollTime = "10h"
+}
+
+func (orc *VTOrcProcess) RewriteConfiguration() error {
+	return os.WriteFile(orc.ConfigPath, []byte(orc.Config.ToJSONString()), 0644)
 }
 
 // Setup starts orc process with required arguements
@@ -111,12 +109,11 @@ func (orc *VTOrcProcess) Setup() (err error) {
 		"--topo_implementation", orc.TopoImplementation,
 		"--topo_global_server_address", orc.TopoGlobalAddress,
 		"--topo_global_root", orc.TopoGlobalRoot,
-		"--config", orc.ConfigPath,
+		"--config-file", orc.ConfigPath,
 		"--port", fmt.Sprintf("%d", orc.Port),
-		// This parameter is overriden from the config file, added here to just verify that we indeed use the config file paramter over the flag
-		"--recovery-period-block-duration", "10h",
+		// This parameter is overriden from the config file. This verifies that we indeed use the flag value over the config file.
 		"--instance-poll-time", "1s",
-		// Faster topo information refresh speeds up the tests. This doesn't add any significant load either
+		// Faster topo information refresh speeds up the tests. This doesn't add any significant load either.
 		"--topo-information-refresh-duration", "3s",
 		"--bind-address", "127.0.0.1",
 	)

go/test/endtoend/vtorc/api/api_test.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"math"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -35,8 +36,7 @@ import (
 func TestAPIEndpoints(t *testing.T) {
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 2, 1, nil, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
-		RecoveryPeriodBlockSeconds:            5,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -91,6 +91,22 @@ func TestAPIEndpoints(t *testing.T) {
 		return response != "null"
 	})
 
+	t.Run("Dynamic Configuration", func(t *testing.T) {
+		// Get configuration and verify the output.
+		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/config")
+		require.NoError(t, err)
+		assert.Equal(t, 200, status)
+		assert.Contains(t, resp, `"snapshottopologyinterval": 0`)
+		// Update configuration and verify the output.
+		vtorc.Config.SnapshotTopologyInterval = "10h"
+		err = vtorc.RewriteConfiguration()
+		assert.NoError(t, err)
+		// Wait until the config has been updated and seen.
+		utils.MakeAPICallRetry(t, vtorc, "/api/config", func(_ int, response string) bool {
+			return !strings.Contains(response, `"snapshottopologyinterval": "10h"`)
+		})
+	})
+
 	t.Run("Database State", func(t *testing.T) {
 		// Get database state
 		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/database-state")

go/test/endtoend/vtorc/general/vtorc_test.go

@@ -42,7 +42,7 @@ func TestPrimaryElection(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 2, 1, nil, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 2, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -70,7 +70,7 @@ func TestSingleKeyspace(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 1, 1, []string{"--clusters_to_watch", "ks"}, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -89,7 +89,7 @@ func TestKeyspaceShard(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 1, 1, []string{"--clusters_to_watch", "ks/0"}, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -111,7 +111,7 @@ func TestVTOrcRepairs(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 3, 0, []string{"--change-tablets-with-errant-gtid-to-drained"}, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -290,7 +290,7 @@ func TestRepairAfterTER(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 2, 0, nil, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -326,7 +326,7 @@ func TestSemiSync(t *testing.T) {
 	newCluster := utils.SetupNewClusterSemiSync(t)
 	defer utils.PrintVTOrcLogsOnFailure(t, newCluster.ClusterInstance)
 	utils.StartVTOrcs(t, newCluster, nil, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1)
 	defer func() {
 		utils.StopVTOrcs(t, newCluster)
@@ -424,7 +424,7 @@ func TestVTOrcWithPrs(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 4, 0, nil, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -456,8 +456,6 @@ func TestVTOrcWithPrs(t *testing.T) {
 		"--new-primary", replica.Alias)
 	require.NoError(t, err, "error in PlannedReparentShard output - %s", output)
 
-	time.Sleep(40 * time.Second)
-
 	// check that the replica gets promoted
 	utils.CheckPrimaryTablet(t, clusterInfo, replica, true)
 	// Verify that VTOrc didn't run any other recovery
@@ -560,7 +558,7 @@ func TestDurabilityPolicySetLater(t *testing.T) {
 
 	// Now start the vtorc instances
 	utils.StartVTOrcs(t, newCluster, nil, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1)
 	defer func() {
 		utils.StopVTOrcs(t, newCluster)
@@ -587,7 +585,7 @@ func TestFullStatusConnectionPooling(t *testing.T) {
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 4, 0, []string{
 		"--tablet_manager_grpc_concurrency=1",
 	}, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]

go/test/endtoend/vtorc/primaryfailure/primary_failure_test.go

@@ -44,7 +44,7 @@ func TestDownPrimary(t *testing.T) {
 	// If that replica is more advanced than the same-cell-replica, then we try to promote the cross-cell replica as an intermediate source.
 	// If we don't specify a small value of --wait-replicas-timeout, then we would end up waiting for 30 seconds for the dead-primary to respond, failing this test.
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 2, 1, []string{"--remote_operation_timeout=10s", "--wait-replicas-timeout=5s"}, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "semi_sync")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -150,7 +150,7 @@ func TestDownPrimaryBeforeVTOrc(t *testing.T) {
 
 	// Start a VTOrc instance
 	utils.StartVTOrcs(t, clusterInfo, []string{"--remote_operation_timeout=10s"}, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1)
 
 	vtOrcProcess := clusterInfo.ClusterInstance.VTOrcProcesses[0]
@@ -244,7 +244,7 @@ func TestDeadPrimaryRecoversImmediately(t *testing.T) {
 	// If that replica is more advanced than the same-cell-replica, then we try to promote the cross-cell replica as an intermediate source.
 	// If we don't specify a small value of --wait-replicas-timeout, then we would end up waiting for 30 seconds for the dead-primary to respond, failing this test.
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 2, 1, []string{"--remote_operation_timeout=10s", "--wait-replicas-timeout=5s"}, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "semi_sync")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -324,7 +324,7 @@ func TestCrossDataCenterFailure(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 2, 1, nil, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -370,7 +370,7 @@ func TestCrossDataCenterFailureError(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 1, 1, nil, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -417,7 +417,7 @@ func TestLostRdonlyOnPrimaryFailure(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 2, 2, nil, cluster.VTOrcConfiguration{
-		PreventCrossDataCenterPrimaryFailover: true,
+		PreventCrossCellFailover: true,
 	}, 1, "")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]
@@ -728,8 +728,8 @@ func TestDownPrimaryPromotionRuleWithLagCrossCenter(t *testing.T) {
 	defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance)
 	defer cluster.PanicHandler(t)
 	utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 2, 1, nil, cluster.VTOrcConfiguration{
-		LockShardTimeoutSeconds:               5,
-		PreventCrossDataCenterPrimaryFailover: true,
+		LockShardTimeoutSeconds:  5,
+		PreventCrossCellFailover: true,
 	}, 1, "test")
 	keyspace := &clusterInfo.ClusterInstance.Keyspaces[0]
 	shard0 := &keyspace.Shards[0]

go/test/endtoend/vtorc/readtopologyinstance/main_test.go

@@ -55,8 +55,7 @@ func TestReadTopologyInstanceBufferable(t *testing.T) {
 		"--topo_global_root", clusterInfo.ClusterInstance.VtctlProcess.TopoGlobalRoot,
 	}
 	servenv.ParseFlags("vtorc")
-	config.Config.RecoveryPeriodBlockSeconds = 1
-	config.Config.InstancePollSeconds = 1
+	config.SetInstancePollTime(1 * time.Second)
 	config.MarkConfigurationLoaded()
 	server.StartVTOrcDiscovery()
 

go/test/endtoend/vtorc/utils/utils.go

@@ -376,7 +376,6 @@ func CheckPrimaryTablet(t *testing.T, clusterInfo *VTOrcClusterInfo, tablet *clu
 	for {
 		now := time.Now()
 		if now.Sub(start) > time.Second*60 {
-			//log.Exitf("error")
 			assert.FailNow(t, "failed to elect primary before timeout")
 		}
 		tabletInfo, err := clusterInfo.ClusterInstance.VtctldClientProcess.GetTablet(tablet.Alias)

go/viperutil/debug/debug.go

@@ -25,3 +25,13 @@ import (
 func Debug() {
 	registry.Combined().Debug()
 }
+
+// WriteConfigAs writes the config into the given filename.
+func WriteConfigAs(filename string) error {
+	return registry.Combined().WriteConfigAs(filename)
+}
+
+// AllSettings gets all the settings in the configuration.
+func AllSettings() map[string]any {
+	return registry.Combined().AllSettings()
+}