Timeout Fixes and VTOrc Improvement #11881

GuptaManan100 · 2022-12-05T12:13:17Z

Description

This PR implements the ideas discussed in #11662 (comment) and #11662 (comment).

As discussed, this PR makes the following changes -

Remove the LockShardTimeout from VTOrc as it doesn't provide any value.
Add parallelism to VTOrc refresh phase so that we wait at most RemoteOperationTimeout seconds before proceeding.
Introduce a new flag lock-timeout which defaults to 45 seconds instead of using RemoteOperationTimeout for the shard lock duration. The default for RemoteOperationTimeout has also been changed to 15 seconds.
Fix the usage of WaitReplicasTimeout in ERS code-path. This makes it so that we are able to handle multiple failures with the set default values.

Related Issue(s)

Fixes Bug Report: VTOrc repairs fail when vttablet is unreachable #11662

Checklist

"Backport to:" labels have been added if this change should be back-ported
Tests were added or are not required
Documentation was added or is not required

Deployment Notes

…de-downgrade tests too Signed-off-by: Manan Gupta <manan@planetscale.com>

…th default values of flags Signed-off-by: Manan Gupta <manan@planetscale.com>

…ation-timeout Signed-off-by: Manan Gupta <manan@planetscale.com>

…ndling multiple failures Signed-off-by: Manan Gupta <manan@planetscale.com>

…elism to refresh of tablets Signed-off-by: Manan Gupta <manan@planetscale.com>

Signed-off-by: Manan Gupta <manan@planetscale.com>

vitess-bot · 2022-12-05T12:13:19Z

Signed-off-by: Manan Gupta <manan@planetscale.com>

GuptaManan100 · 2022-12-05T15:19:50Z

Even with these changes, VTOrc is unable to fix the failure of a single vttablet. i.e. if the primary's vttablet crashes, VTOrc doesn't run an ERS.
The issues still pending are -

vttablets in their tm.Close() function, delete the MySQL hostname and some more information from the vttablet record. This causes VTOrc to think that the vttablet isn't part of the topology anymore. This assumption is however incorrect. Until the tablet's record is removed from the topo-server, it should be considered part of the topology.
The above only happens sometimes on the vttablets. The reason is that the onclose_timeout defaults to 1ns, so the tm.Close() function runs successfully only sometimes. This makes the VTOrc behavior unpredictable.
When a vttablet fails, VTOrc thinks it is unreachable, but doesn't think it's dead. The reason is that for MySQL failures, VTOrc tries to take in information from the replicas as well to see if they are able to reach the primary MySQL server. If the replication is working fine for the replicas, then VTOrc assumes that it is network partitioned from the MySQL server and doesn't run an ERS.
For vttablet failures, this however becomes a problem! Since vttablets don't contact each other, if the MySQL servers are working fine, VTOrc doesn't run an ERS. We would need some way to know if the vttablet is actually down, or just network partitioned from VTOrc. We could potentially use the vtgate for it 🤷‍♂️, but this is just an idea. It needs more thought on how we want to handle this.

Irrespective of the problems listed above, this PR is still an improvement for both VTOrc and ERS, since it does allow ERS to handle multiple failures with the default values and it allows VTOrc to handle the situations where both vttablet and MySQL server is down. A test for this situation has been added to verify that this indeed does work after the changes.

…ltiple times Signed-off-by: Manan Gupta <manan@planetscale.com>

Signed-off-by: Manan Gupta <manan@planetscale.com>

GuptaManan100 · 2022-12-13T11:03:35Z

I have now fixed the default timeout of onclose_timeout flag. Do we need to include this in the release notes summary too?
I have also patched VTOrc not to forget vttablet instances with an empty hostname as long as they are present in the topo server. A test has been added for the same.

…ents to allow vttablets to setup replication when restarted Signed-off-by: Manan Gupta <manan@planetscale.com>

Signed-off-by: Manan Gupta <manan@planetscale.com>

deepthi · 2022-12-14T15:35:20Z

I have now fixed the default timeout of onclose_timeout flag. Do we need to include this in the release notes summary too?

I doubt anyone is actually using it, so this should be fine without a release note.

deepthi

Nice work cleaning up a whole bunch of incorrect timeout handling in this code.
Please address all feedback before merging.

deepthi · 2022-12-14T18:07:45Z

doc/releasenotes/16_0_0_summary.md

+During upgrades, if the users want to preserve the same behaviour as previous releases, then they should provide the `remote_operation_timeout` flag explicitly before upgrading.
+After the upgrade, they should then alter their configuration to also specify `lock-timeout` explicitly.


Specifying both remote_operation_timeout and lock-timeout to 30s will expose users to the bug that is being fixed in this PR.
Instead we should recommend that if (and only if) they get timeouts during PRS or ERS, they may need to increase remote_operation_timeout and lock-timeout at the same time while making sure that lock-timeout is at least 15 seconds more than remote_operation_timeout.

Actually, the defaults are 15 and 45 seconds, and we don't want lock-timeout to be at least 15 seconds more than remote_operation_timeout, we want it to be atleaset 2 times it and a littlle more.

Well in the previous release this bug exists, and only after upgrading and specifying both the flags (or specifying neither) can they really get rid of the bug. So the recommended process is to specify the remote-operation_timeout first so that while upgrading their behaviour remains the same. Once ugpraded they can choose whatever values they want.

deepthi · 2022-12-14T18:10:01Z

go/flags/endtoend/vtctld.txt

@@ -1,5 +1,5 @@
 Usage of vtctld:
-      --action_timeout duration                                          time to wait for an action before resorting to force (default 2m0s)
+      --action_timeout duration                                          time to wait for an action before resorting to force (default 1m0s)


Why is this changing? Can you also check the blame so that we can see whether it changed recently?
There's an action_timeout that defaults to 1h on the vtctlclient and vtctldclient binaries. How does this relate to those?

This is a effect of

vitess/go/vt/wrangler/wrangler.go

Line 39 in 8b8b135

DefaultActionTimeout = topo.RemoteOperationTimeout * 4

and

vitess/go/vt/vtctld/action_repository.go

Line 38 in 8b8b135

actionTimeout = wrangler.DefaultActionTimeout

Apparently there are two action_timeouts 🤣. That action_timeout is a parameter for vtctldclient and vtctlclient. This one is taken by vtctld and applies only to ApplyKeyspaceAction, ApplyShardAction and ApplyTabletAction. This is the commit that introduced it - facdb18

deepthi · 2022-12-14T18:19:21Z

go/flags/endtoend/vtorc.txt

@@ -23,14 +23,14 @@ Usage of vtorc:
      --keep_logs duration                           keep logs for this long (using ctime) (zero to keep forever)
      --keep_logs_by_mtime duration                  keep logs for this long (using mtime) (zero to keep forever)
      --lameduck-period duration                     keep running at least this long after SIGTERM before stopping (default 50ms)
-      --lock-shard-timeout duration                  Duration for which a shard lock is held when running a recovery (default 30s)


do we need to deprecate this flag?

I have deprecated it -

fs.Duration("lock-shard-timeout", 30*time.Second, "Duration for which a shard lock is held when running a recovery") _ = fs.MarkDeprecated("lock-shard-timeout", "Please use lock-timeout instead.")

Deprecated flags don't show up in the flags output either.

Added this to the summary too.

deepthi · 2022-12-14T18:21:26Z

go/internal/flag/flag.go

@@ -70,6 +70,17 @@ func Parse(fs *flag.FlagSet) {
 	flag.Parse()
 }

+// IsFlagPassed returns if the given flag has been provided by the user explicitly or not


suggestion: IsFlagProvided
WDYT? also @ajm188

I don't have strong opinions. Whatever we feel is best. I'll change it to IsFlagProvided for now.

go/test/endtoend/reparent/plannedreparent/reparent_test.go

deepthi · 2022-12-14T18:24:24Z

go/vt/topo/locks.go

 	ctx = trace.CopySpan(context.TODO(), ctx)
-	ctx, cancel := context.WithTimeout(ctx, defaultLockTimeout)
+	ctx, cancel := context.WithTimeout(ctx, RemoteOperationTimeout)


maybe I have missed something why we are not using getLockedTimeout during unlock?

Unlock is just a single remote operation. It doesn't need more time than a RemoteOperationTimeout. getLockTimeout is the time for which we want to lock the shard for.

deepthi · 2022-12-14T18:24:36Z

go/vt/topo/locks.go

 	ctx = trace.CopySpan(context.TODO(), ctx)
-	ctx, cancel := context.WithTimeout(ctx, defaultLockTimeout)
+	ctx, cancel := context.WithTimeout(ctx, RemoteOperationTimeout)


go/vt/topo/locks_test.go

go/vt/vttablet/tabletmanager/tm_init.go

Signed-off-by: Manan Gupta <manan@planetscale.com>

go/test/endtoend/reparent/newfeaturetest/reparent_test.go

rsajwani

overall LGTM

rsajwani · 2022-12-15T22:57:38Z

go/test/endtoend/reparent/utils/utils.go

@@ -129,7 +129,7 @@ func setupCluster(ctx context.Context, t *testing.T, shardName string, cells []s
 		// the replication manager to silently fix the replication in case ERS or PRS mess up. All the
 		// tests in this test suite should work irrespective of this flag. Each run of ERS, PRS should be
 		// setting up the replication correctly.
-		"--disable_active_reparents")
+		"--disable-replication-manager")


what is the context of this change?

This change is required because in the tests we test that a tablet that is restarted starts replicating from the primary again. So we want the replication initialization code to run. disable_active_reparents blocks both the replication manager and the replication initialization, but disable-replication-manager only blocks the first.

We required this change after increasing the onclose timeout. Earlier, because onclose timeout was 1ns, the vttablets when shutting down wouldn't have the time to remove the hostname, port from the vttablet records. This caused a call to ERS to have a hanging SetReplicationSource. This call would succeed when the tablet was up again. But this is incorrect and not want the test really wanted to test.

After increasing the onclose timeout, the vttablets now remove their hostname, port information from the tablet records. This causes the SetReplicationSource call to be directed at a tablet with address :0. So that call doesn't setup the replication. Instead, we require the replication initialization to do it properly. This is the reason for this change.

These changes resolve flakiness in the ERS tests which showed itself as vttablet timed out after 10s failing to reach the Serving state, in the few cases that onclose function did complete before timing out.

rsajwani · 2022-12-15T23:02:56Z

go/vt/servenv/servenv_test.go

-	// we deliberatly test the flag to make sure it's not accidently set to a
-	// high value.
-	if finished, want := fireOnCloseHooks(onCloseTimeout), false; finished != want {
+	if finished, want := fireOnCloseHooks(1*time.Nanosecond), false; finished != want {


1ns is here to check backward compact. given the default value is change now ?

I made this change, because otherwise the test would need to run for more than 10 seconds. The intent of the test is to verify that fireOnCloseHooks respects the timeout provided to it and that it returns after that much time. If we give the default here, we would need a sleep for like 11 seconds or something in the Onclose function and that would just slow down the test.

rsajwani · 2022-12-15T23:06:23Z

go/vt/topo/locks.go

 	ctx = trace.CopySpan(context.TODO(), ctx)
-	ctx, cancel := context.WithTimeout(ctx, defaultLockTimeout)
+	ctx, cancel := context.WithTimeout(ctx, RemoteOperationTimeout)


maybe I have missed something why we are not using getLockedTimeout during unlock?

go/vt/topo/locks_test.go

Signed-off-by: Manan Gupta <manan@planetscale.com>

* refactor: move tests out of newfeaturestest so that they run on upgrade-downgrade tests too Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add failing ers test for handling multiple vttablet failures with default values of flags Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add a new lock-timeout flag and use that instead of remote-operation-timeout Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: augment DownPrimary test to reproduce the issue of VTOrc not handling multiple failures Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove LockShardTimeout configuration from VTOrc and add parallelism to refresh of tablets Signed-off-by: Manan Gupta <manan@planetscale.com> * log: add more logging lines around ers in vtorc Signed-off-by: Manan Gupta <manan@planetscale.com> * test: get the test to work Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix usage of wait for replicas timeout Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flags expected output Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix race in test now that the function is called in parallel multiple times Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix default of onCloseTimeout to 1 second Signed-off-by: Manan Gupta <manan@planetscale.com> * test: add failing unit test to refreshTabletsInKeyspaceShard Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix vtorc to not forget a tablet which has been deleted Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix backward compatibility, add tests and release notes docs Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flags output Signed-off-by: Manan Gupta <manan@planetscale.com> * test: use disable-replication-manager instead of disable-active-reparents to allow vttablets to setup replication when restarted Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flaky test by not checking for an error Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: handle the case of empty hostname in tablet initialization Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: update onclose timeout to 10 seconds Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix unit test Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: address review comments Signed-off-by: Manan Gupta <manan@planetscale.com> * docs: add comments explaining the test functions Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add summary docs for 'lock-shard-timeout' deprecation Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Max Englander <max@planetscale.com>

* refactor: move tests out of newfeaturestest so that they run on upgrade-downgrade tests too Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add failing ers test for handling multiple vttablet failures with default values of flags Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add a new lock-timeout flag and use that instead of remote-operation-timeout Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: augment DownPrimary test to reproduce the issue of VTOrc not handling multiple failures Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove LockShardTimeout configuration from VTOrc and add parallelism to refresh of tablets Signed-off-by: Manan Gupta <manan@planetscale.com> * log: add more logging lines around ers in vtorc Signed-off-by: Manan Gupta <manan@planetscale.com> * test: get the test to work Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix usage of wait for replicas timeout Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flags expected output Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix race in test now that the function is called in parallel multiple times Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix default of onCloseTimeout to 1 second Signed-off-by: Manan Gupta <manan@planetscale.com> * test: add failing unit test to refreshTabletsInKeyspaceShard Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix vtorc to not forget a tablet which has been deleted Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix backward compatibility, add tests and release notes docs Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flags output Signed-off-by: Manan Gupta <manan@planetscale.com> * test: use disable-replication-manager instead of disable-active-reparents to allow vttablets to setup replication when restarted Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flaky test by not checking for an error Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: handle the case of empty hostname in tablet initialization Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: update onclose timeout to 10 seconds Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix unit test Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: address review comments Signed-off-by: Manan Gupta <manan@planetscale.com> * docs: add comments explaining the test functions Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add summary docs for 'lock-shard-timeout' deprecation Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com>

* VTOrc running PRS when database_instance empty bug fix. (vitessio#12019) * feat: convert join with database_instance to a left join and prevent fixes from running if the information from database_instance is unavailable Signed-off-by: Manan Gupta <manan@planetscale.com> * test: add tests to verify the fix works Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> * Timeout Fixes and VTOrc Improvement (vitessio#11881) * refactor: move tests out of newfeaturestest so that they run on upgrade-downgrade tests too Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add failing ers test for handling multiple vttablet failures with default values of flags Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add a new lock-timeout flag and use that instead of remote-operation-timeout Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: augment DownPrimary test to reproduce the issue of VTOrc not handling multiple failures Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove LockShardTimeout configuration from VTOrc and add parallelism to refresh of tablets Signed-off-by: Manan Gupta <manan@planetscale.com> * log: add more logging lines around ers in vtorc Signed-off-by: Manan Gupta <manan@planetscale.com> * test: get the test to work Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix usage of wait for replicas timeout Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flags expected output Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix race in test now that the function is called in parallel multiple times Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix default of onCloseTimeout to 1 second Signed-off-by: Manan Gupta <manan@planetscale.com> * test: add failing unit test to refreshTabletsInKeyspaceShard Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix vtorc to not forget a tablet which has been deleted Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix backward compatibility, add tests and release notes docs Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flags output Signed-off-by: Manan Gupta <manan@planetscale.com> * test: use disable-replication-manager instead of disable-active-reparents to allow vttablets to setup replication when restarted Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flaky test by not checking for an error Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: handle the case of empty hostname in tablet initialization Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: update onclose timeout to 10 seconds Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix unit test Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: address review comments Signed-off-by: Manan Gupta <manan@planetscale.com> * docs: add comments explaining the test functions Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add summary docs for 'lock-shard-timeout' deprecation Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> * log: also log error in DiscoverInstance when force discovery is specified (vitessio#11936) Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> * VTOrc Code Cleanup - generate_base, replace cluster_name with keyspace and shard. (vitessio#12012) * feat: refactor generate commands of VTOrc to be in a single file Signed-off-by: Manan Gupta <manan@planetscale.com> * refactor: cleanup create table formatting Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: cleanup the usage of IsSQLite and IsMySQL Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove unused minimal instance Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove unused table cluster_domain_name Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix vtorc database to store keyspace and shard instead of cluster Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove unused attributes Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove unused cluster domain Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: change GetClusterName to GetKeyspaceAndShardName Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix insertion into database_instance Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix SnapshotTopologies Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove inject unseen primary and inject seed Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove ClusterName from Instance Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix Audit operations Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add Keyspace and Shard to cluster information to replace ClusterName Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix attempt failure detection registeration Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix blocked topology recoveries Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix topology recovery Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: reading recovery instances Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix get replication and analysis Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix bug in query Signed-off-by: Manan Gupta <manan@planetscale.com> * test: add tests to check that filtering by keyspace works for APIs Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: remove remaining usages of ClusterName Signed-off-by: Manan Gupta <manan@planetscale.com> * refactor: fix comment explaining sleep in the test Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add code to prevent filtering just by shard and add tests for it Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> * Fix insert query of blocked_recovery table in VTOrc (vitessio#12091) * feat: add failing test and fix the query of insertion Signed-off-by: Manan Gupta <manan@planetscale.com> * empty-commit Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> * Fix: VTOrc forgetting old instances (vitessio#12089) * test: add a failing test for the case where the port changes for a tablet Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix the issue by adding alias as a unique field Signed-off-by: Manan Gupta <manan@planetscale.com> * empty-commit Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> * Move vtorc from go-sqlite3 to modernc.org/sqlite (vitessio#12214) * Move vtorc from go-sqlite3 to modernc.org/sqlite This moves vtorc from the go-sqlite3 library that uses CGO, to use modernc.org/sqlite which is a pure Go implementation. vtorc is the only component we have to build with CGO but it's causing pain for releases since we need to build it against an old Linux for linking against glibc. Using modernc.org/sqlite allows for using Go only again and makes all Vitess components buildable without CGO. In https://datastation.multiprocess.io/blog/2022-05-12-sqlite-in-go-with-and-without-cgo.html someone ran some basic benchmarks. It shows that the pure Go version can be twice as slow, but the usage of vtorc is very limited and we operate on small datasets, so I think the performance impact purely of a somewhat slower sqlite implementation is negligable. None of this is in a hot query serving path or anything like that, so I have little concern performance wise. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix error handling in RowToArray Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * see if CI passes on v14.0.5 as previous release Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Revert "see if CI passes on v14.0.5 as previous release" This reverts commit 53a0e0c. --------- Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Manan Gupta <35839558+GuptaManan100@users.noreply.github.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com>

GuptaManan100 added 6 commits December 2, 2022 14:15

refactor: move tests out of newfeaturestest so that they run on upgra…

31ba4d4

…de-downgrade tests too Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: add failing ers test for handling multiple vttablet failures wi…

b9417b8

…th default values of flags Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: add a new lock-timeout flag and use that instead of remote-oper…

def9aec

…ation-timeout Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: augment DownPrimary test to reproduce the issue of VTOrc not ha…

0273581

…ndling multiple failures Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: remove LockShardTimeout configuration from VTOrc and add parall…

6c449c0

…elism to refresh of tablets Signed-off-by: Manan Gupta <manan@planetscale.com>

log: add more logging lines around ers in vtorc

d288571

Signed-off-by: Manan Gupta <manan@planetscale.com>

GuptaManan100 added 3 commits December 5, 2022 20:10

test: get the test to work

e1b23da

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: fix usage of wait for replicas timeout

c2b6a0d

Signed-off-by: Manan Gupta <manan@planetscale.com>

test: fix flags expected output

033083c

Signed-off-by: Manan Gupta <manan@planetscale.com>

GuptaManan100 force-pushed the timeout-fixes branch from 2b15dd0 to 033083c Compare December 5, 2022 14:49

GuptaManan100 added 6 commits December 5, 2022 20:53

test: fix race in test now that the function is called in parallel mu…

5f03e90

…ltiple times Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: fix default of onCloseTimeout to 1 second

c2aa9fd

Signed-off-by: Manan Gupta <manan@planetscale.com>

test: add failing unit test to refreshTabletsInKeyspaceShard

2097817

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: fix vtorc to not forget a tablet which has been deleted

27bf296

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: fix backward compatibility, add tests and release notes docs

e3790c9

Signed-off-by: Manan Gupta <manan@planetscale.com>

test: fix flags output

97c7eef

Signed-off-by: Manan Gupta <manan@planetscale.com>

GuptaManan100 removed the release notes (needs details) This PR needs to be listed in the release notes in a dedicated section (deprecation notice, etc...) label Dec 13, 2022

GuptaManan100 marked this pull request as ready for review December 13, 2022 11:03

GuptaManan100 requested review from deepthi, shlomi-noach and rsajwani as code owners December 13, 2022 11:03

GuptaManan100 added 5 commits December 14, 2022 11:59

test: use disable-replication-manager instead of disable-active-repar…

e31e7c5

…ents to allow vttablets to setup replication when restarted Signed-off-by: Manan Gupta <manan@planetscale.com>

test: fix flaky test by not checking for an error

ac83323

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: handle the case of empty hostname in tablet initialization

6b0db02

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: update onclose timeout to 10 seconds

1f1e8ac

Signed-off-by: Manan Gupta <manan@planetscale.com>

test: fix unit test

2411cea

Signed-off-by: Manan Gupta <manan@planetscale.com>

deepthi approved these changes Dec 14, 2022

View reviewed changes

feat: address review comments

ea18935

Signed-off-by: Manan Gupta <manan@planetscale.com>

rsajwani reviewed Dec 15, 2022

View reviewed changes

go/test/endtoend/reparent/newfeaturetest/reparent_test.go Show resolved Hide resolved

rsajwani approved these changes Dec 15, 2022

View reviewed changes

GuptaManan100 added 3 commits December 16, 2022 10:54

docs: add comments explaining the test functions

740f2fa

Signed-off-by: Manan Gupta <manan@planetscale.com>

Merge remote-tracking branch 'upstream/main' into timeout-fixes

24a4f40

Signed-off-by: Manan Gupta <manan@planetscale.com>

feat: add summary docs for 'lock-shard-timeout' deprecation

d827acc

Signed-off-by: Manan Gupta <manan@planetscale.com>

GuptaManan100 merged commit 46d63e9 into vitessio:main Dec 16, 2022

GuptaManan100 deleted the timeout-fixes branch December 16, 2022 09:02

GuptaManan100 mentioned this pull request Jan 5, 2023

Bug Report: data race in IsFlagProvided function. #12041

Closed

deepthi restored the timeout-fixes branch January 11, 2023 23:15

deepthi mentioned this pull request Jan 11, 2023

[15.0] CI: remove pitrtls test #12064

Merged

3 tasks

deepthi deleted the timeout-fixes branch January 11, 2023 23:34

deepthi restored the timeout-fixes branch January 12, 2023 00:46

This was referenced Jan 13, 2023

Bug Report: VTOrc doesn't forget old instances #12088

Closed

Fix: VTOrc forgetting old instances #12089

Merged

timvaillancourt mentioned this pull request Sep 11, 2023

slack-vitess-r15.0.4: backport v16 vtorc fixes + go1.20 slackhq/vitess#126

Closed

4 tasks

timvaillancourt mentioned this pull request Mar 15, 2024

slack-vitess-r15.0.5: backport v16 vtorc fixes slackhq/vitess#258

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Timeout Fixes and VTOrc Improvement #11881

Timeout Fixes and VTOrc Improvement #11881

GuptaManan100 commented Dec 5, 2022 •

edited

Loading

vitess-bot bot commented Dec 5, 2022 •

edited by deepthi

Loading

GuptaManan100 commented Dec 5, 2022 •

edited

Loading

GuptaManan100 commented Dec 13, 2022

deepthi commented Dec 14, 2022

deepthi left a comment

deepthi Dec 14, 2022

GuptaManan100 Dec 15, 2022

deepthi Dec 14, 2022

GuptaManan100 Dec 15, 2022 •

edited

Loading

deepthi Dec 14, 2022

GuptaManan100 Dec 15, 2022

GuptaManan100 Dec 16, 2022

deepthi Dec 14, 2022

GuptaManan100 Dec 15, 2022

deepthi Dec 14, 2022

rsajwani Dec 15, 2022

GuptaManan100 Dec 16, 2022

deepthi Dec 14, 2022

rsajwani left a comment

rsajwani Dec 15, 2022

GuptaManan100 Dec 16, 2022

rsajwani Dec 15, 2022

GuptaManan100 Dec 16, 2022

rsajwani Dec 15, 2022

		During upgrades, if the users want to preserve the same behaviour as previous releases, then they should provide the `remote_operation_timeout` flag explicitly before upgrading.
		After the upgrade, they should then alter their configuration to also specify `lock-timeout` explicitly.

Timeout Fixes and VTOrc Improvement #11881

Timeout Fixes and VTOrc Improvement #11881

Conversation

GuptaManan100 commented Dec 5, 2022 • edited Loading

Description

Related Issue(s)

Checklist

Deployment Notes

vitess-bot bot commented Dec 5, 2022 • edited by deepthi Loading

Review Checklist

General

If a new flag is being introduced:

If a workflow is added or modified:

Bug fixes

Non-trivial changes

New/Existing features

Backward compatibility

GuptaManan100 commented Dec 5, 2022 • edited Loading

GuptaManan100 commented Dec 13, 2022

deepthi commented Dec 14, 2022

deepthi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuptaManan100 Dec 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rsajwani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuptaManan100 commented Dec 5, 2022 •

edited

Loading

vitess-bot bot commented Dec 5, 2022 •

edited by deepthi

Loading

GuptaManan100 commented Dec 5, 2022 •

edited

Loading

GuptaManan100 Dec 15, 2022 •

edited

Loading