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.

Sign up for GitHub

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

Jump to bottom

VReplication Workflows and VStream API gets stuck in the copy phase if tablet type is set only as a replica #13175

Closed
rohit-nayak-ps opened this issue May 28, 2023 · 1 comment
Closed

VReplication Workflows and VStream API gets stuck in the copy phase if tablet type is set only as a replica #13175

rohit-nayak-ps opened this issue May 28, 2023 · 1 comment
Labels
Component: Cluster management Component: VReplication Type: Bug

Comments

@rohit-nayak-ps
Copy link
Contributor

rohit-nayak-ps commented May 28, 2023 •
edited
Loading

Overview of the Issue

VStreams do not stream any data while streaming using the vtgate VStream API If

The rowstreamer starts streaming, but it gets permanently throttled before a single row is streamed and the copy phase times out.

I am also able to reproduce this in the MoveTables step of the local example if tablet_type is set just to replica.

If we modify local_example.sh to enable the throttler, MoveTables never moves out of the copy phase.

./101_initial_cluster.sh

vtctlclient -- UpdateThrottlerConfig --enable --threshold 5.0 commerce

Reproduction Steps

  1. Run local_example upto ./201_customer_tablets.sh, so that the commerce keyspace is setup and populated
  2. Enable the throttler: vtctlclient -- UpdateThrottlerConfig --enable --threshold 5.0 commerce
  3. Modify examples/local/vstream_client.go to stream from commerce and from replica
    reader, err := conn.VStream(ctx, topodatapb.TabletType_REPLICA, vgtid, filter, flags)
  4. Stream using the VStream API: go run examples/local/vstream_client.go
  5. Nothing gets streamed
  6. In a different terminal disable the throttler: vtctlclient -- UpdateThrottlerConfig --disable commerce
  7. Data is streamed

Binary Version

`main` at `18ab80e26b71bc25c11ec11919c5b9361576a160`

Operating System and Environment details

-

Log Fragments

I0528 11:08:25.236309  611413 uvstreamer.go:405] Stream() called
I0528 11:08:25.236349  611413 uvstreamer.go:410] TablePKs is not nil: starting vs.copy()
I0528 11:08:25.236352  611413 copy.go:50] catchupAndCopy for customer
I0528 11:08:25.236354  611413 copy.go:58] catchupAndCopy: before copyTable customer
I0528 11:08:25.236358  611413 copy.go:214] Starting copyTable for customer, PK []
I0528 11:08:25.236363  611413 engine.go:279] Streaming rows for query select * from customer, lastpk: []
I0528 11:08:25.238083  611413 rowstreamer.go:291] Streaming query: select customer_id, email from customer order by customer_id
I0528 11:08:25.238300  611413 snapshot_conn.go:121] Locking table customer for copying
I0528 11:08:25.238636  611413 snapshot_conn.go:114] Tables unlocked: customer
I0528 11:12:17.104444  611413 rowstreamer.go:346] Stream ended because of ctx.Done
I0528 11:12:17.104667  611413 uvstreamer.go:412] uvstreamer.Stream() copy returned with err stream ended: context canceled
@rohit-nayak-ps rohit-nayak-ps changed the title VStreams do not start if tablet type is set to replicas VReplication Workflows and VStream API gets stuck in the copy phase if tablet type is set only as a replica May 28, 2023
This was referenced May 28, 2023
Closed
Closed
Merged
mattlord added a commit to planetscale/vitess that referenced this issue May 31, 2023
@mattlord
Use constant heartbeats in vrepl e2e tests
70570a5 
Until vitessio#13175 is
fixed.

Signed-off-by: Matt Lord <mattalord@gmail.com>
@shlomi-noach
Copy link
Contributor

fixed by #13195

shlomi-noach added a commit to planetscale/vitess that referenced this issue Jun 1, 2023
@shlomi-noach
vitessio#13175 is fixed, therefore re-instating on-deman heartbeats
11ca17a 
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@mattlord mattlord mentioned this issue Jun 1, 2023
Merged
4 tasks
shlomi-noach added a commit that referenced this issue Jun 5, 2023
@shlomi-noach @mattlord
Tablet throttler: throttler-config-via-topo defaults 'true', deprecat…
8d3a1fb 
…ion message for old flags (#13130)

* Table throttler: --throttler-config-via-topo now defaults to 'true'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* add deprecation message

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* endtoend tests: remove '--enable-lag-throttler' and use 'UpdateThrottlerConfig' everywhere

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* always use vtctldclient

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* use cluster.VtctldClientProcess

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* disable --throttler-config-via-topo in old throttler tests

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Remove --throttler-config-via-topo where used, since it now defaults 'true'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fix vreplication cluster setup, waiting for throttler config to apply

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* changelog

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* extend throttler threshold

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* a bit more verbose

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fixed CLI test

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* remove old '--enable-lag-throttler' flag, introduce '--heartbeat_on_demand_duration'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* more log info in throttler.Open()

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* more logging

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Revert to --heartbeat_enable

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Protect throttler config change application with initMutex

And in e2e test update the throttler config on the keyspace
when it's created. Only wait for the new tablets in a shard
to have the throttler enabled when adding a Shard.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* More CI testing

Signed-off-by: Matt Lord <mattalord@gmail.com>

* CI testing cont

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Yes...

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Somebody doesn't like force pushes so msg here

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Increase on-demand heartbeat duration from 10s to 1m

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use only on-demand heartbeats everywhere

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use same throttler config everywhere

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Update all keyspaces and don't fail test on missing JSON keys

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use constant heartbeats in vrepl e2e tests

Until #13175 is
fixed.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Increase workflow command timeout

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Don't wait for throttler on non-serving primaries

Signed-off-by: Matt Lord <mattalord@gmail.com>

* #13175 is fixed, therefore re-instating on-deman heartbeats

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Added ToC

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Tweak comment and kick CI

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Treat isOpen as the ready/running signal.

Also align all initMutex usage.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Re-adjust comment

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Adjust CheckIsReady() to match OnlineDDL's expectation/usage

This was only using IsReady() before, now it's using
IsOpen() and IsReady().

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Get rid of log messages from SrvKeyspaceWatcher when no node/key

Signed-off-by: Matt Lord <mattalord@gmail.com>

* More corrections/tweaks

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use more convenient/clear new IsRunning function

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Revert "Use more convenient/clear new IsRunning function"

This reverts commit 9aef276 as this
change was not correct.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Further fix correct use of IsOpen(), IsRunning(), IsEnabled()

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* throttler.throttledApps cannot be nil

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* minor refactory/beautify for test

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fix flakiness of tabletmanager_throttler_topo test by: (1) proper wait-for functions, and (2) issue different queries per goroutine

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Fix typo in release notes

Signed-off-by: Matt Lord <mattalord@gmail.com>

---------

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Co-authored-by: Matt Lord <mattalord@gmail.com>
shlomi-noach added a commit to planetscale/vitess that referenced this issue Jun 5, 2023
@shlomi-noach @mattlord
Tablet throttler: throttler-config-via-topo defaults 'true', deprecat…
19e2021 
…ion message for old flags (vitessio#13130)

* Table throttler: --throttler-config-via-topo now defaults to 'true'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* add deprecation message

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* endtoend tests: remove '--enable-lag-throttler' and use 'UpdateThrottlerConfig' everywhere

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* always use vtctldclient

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* use cluster.VtctldClientProcess

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* disable --throttler-config-via-topo in old throttler tests

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Remove --throttler-config-via-topo where used, since it now defaults 'true'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fix vreplication cluster setup, waiting for throttler config to apply

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* changelog

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* extend throttler threshold

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* a bit more verbose

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fixed CLI test

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* remove old '--enable-lag-throttler' flag, introduce '--heartbeat_on_demand_duration'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* more log info in throttler.Open()

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* more logging

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Revert to --heartbeat_enable

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Protect throttler config change application with initMutex

And in e2e test update the throttler config on the keyspace
when it's created. Only wait for the new tablets in a shard
to have the throttler enabled when adding a Shard.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* More CI testing

Signed-off-by: Matt Lord <mattalord@gmail.com>

* CI testing cont

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Yes...

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Somebody doesn't like force pushes so msg here

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Increase on-demand heartbeat duration from 10s to 1m

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use only on-demand heartbeats everywhere

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use same throttler config everywhere

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Update all keyspaces and don't fail test on missing JSON keys

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use constant heartbeats in vrepl e2e tests

Until vitessio#13175 is
fixed.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Increase workflow command timeout

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Don't wait for throttler on non-serving primaries

Signed-off-by: Matt Lord <mattalord@gmail.com>

* vitessio#13175 is fixed, therefore re-instating on-deman heartbeats

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Added ToC

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Tweak comment and kick CI

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Treat isOpen as the ready/running signal.

Also align all initMutex usage.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Re-adjust comment

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Adjust CheckIsReady() to match OnlineDDL's expectation/usage

This was only using IsReady() before, now it's using
IsOpen() and IsReady().

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Get rid of log messages from SrvKeyspaceWatcher when no node/key

Signed-off-by: Matt Lord <mattalord@gmail.com>

* More corrections/tweaks

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use more convenient/clear new IsRunning function

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Revert "Use more convenient/clear new IsRunning function"

This reverts commit 9aef276 as this
change was not correct.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Further fix correct use of IsOpen(), IsRunning(), IsEnabled()

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* throttler.throttledApps cannot be nil

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* minor refactory/beautify for test

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fix flakiness of tabletmanager_throttler_topo test by: (1) proper wait-for functions, and (2) issue different queries per goroutine

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Fix typo in release notes

Signed-off-by: Matt Lord <mattalord@gmail.com>

---------

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Co-authored-by: Matt Lord <mattalord@gmail.com>
mattlord added a commit that referenced this issue Jun 5, 2023
@shlomi-noach @mattlord
Tablet throttler: throttler-config-via-topo defaults 'true', deprecat…
172dcfe 
…ion message for old flags (#13130) (#13237)

* Table throttler: --throttler-config-via-topo now defaults to 'true'



* add deprecation message



* endtoend tests: remove '--enable-lag-throttler' and use 'UpdateThrottlerConfig' everywhere



* always use vtctldclient



* use cluster.VtctldClientProcess



* disable --throttler-config-via-topo in old throttler tests



* Remove --throttler-config-via-topo where used, since it now defaults 'true'



* fix vreplication cluster setup, waiting for throttler config to apply



* changelog



* extend throttler threshold



* a bit more verbose



* fixed CLI test



* remove old '--enable-lag-throttler' flag, introduce '--heartbeat_on_demand_duration'



* more log info in throttler.Open()



* more logging



* Revert to --heartbeat_enable



* Protect throttler config change application with initMutex

And in e2e test update the throttler config on the keyspace
when it's created. Only wait for the new tablets in a shard
to have the throttler enabled when adding a Shard.



* More CI testing



* CI testing cont



* Yes...



* Somebody doesn't like force pushes so msg here



* Increase on-demand heartbeat duration from 10s to 1m



* Use only on-demand heartbeats everywhere



* Use same throttler config everywhere



* Update all keyspaces and don't fail test on missing JSON keys



* Use constant heartbeats in vrepl e2e tests

Until #13175 is
fixed.



* Increase workflow command timeout



* Don't wait for throttler on non-serving primaries



* #13175 is fixed, therefore re-instating on-deman heartbeats



* Added ToC



* Tweak comment and kick CI



* Treat isOpen as the ready/running signal.

Also align all initMutex usage.



* Re-adjust comment



* Adjust CheckIsReady() to match OnlineDDL's expectation/usage

This was only using IsReady() before, now it's using
IsOpen() and IsReady().



* Get rid of log messages from SrvKeyspaceWatcher when no node/key



* More corrections/tweaks



* Use more convenient/clear new IsRunning function



* Revert "Use more convenient/clear new IsRunning function"

This reverts commit 9aef276 as this
change was not correct.



* Further fix correct use of IsOpen(), IsRunning(), IsEnabled()



* throttler.throttledApps cannot be nil



* minor refactory/beautify for test



* fix flakiness of tabletmanager_throttler_topo test by: (1) proper wait-for functions, and (2) issue different queries per goroutine



* Fix typo in release notes



---------

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Co-authored-by: Matt Lord <mattalord@gmail.com>
arvind-murty pushed a commit to arvind-murty/vitess that referenced this issue Jun 7, 2023
@shlomi-noach @mattlord @arvind-murty
Tablet throttler: throttler-config-via-topo defaults 'true', deprecat…
44d9c53 
…ion message for old flags (vitessio#13130)

* Table throttler: --throttler-config-via-topo now defaults to 'true'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* add deprecation message

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* endtoend tests: remove '--enable-lag-throttler' and use 'UpdateThrottlerConfig' everywhere

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* always use vtctldclient

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* use cluster.VtctldClientProcess

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* disable --throttler-config-via-topo in old throttler tests

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Remove --throttler-config-via-topo where used, since it now defaults 'true'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fix vreplication cluster setup, waiting for throttler config to apply

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* changelog

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* extend throttler threshold

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* a bit more verbose

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fixed CLI test

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* remove old '--enable-lag-throttler' flag, introduce '--heartbeat_on_demand_duration'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* more log info in throttler.Open()

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* more logging

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Revert to --heartbeat_enable

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Protect throttler config change application with initMutex

And in e2e test update the throttler config on the keyspace
when it's created. Only wait for the new tablets in a shard
to have the throttler enabled when adding a Shard.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* More CI testing

Signed-off-by: Matt Lord <mattalord@gmail.com>

* CI testing cont

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Yes...

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Somebody doesn't like force pushes so msg here

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Increase on-demand heartbeat duration from 10s to 1m

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use only on-demand heartbeats everywhere

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use same throttler config everywhere

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Update all keyspaces and don't fail test on missing JSON keys

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use constant heartbeats in vrepl e2e tests

Until vitessio#13175 is
fixed.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Increase workflow command timeout

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Don't wait for throttler on non-serving primaries

Signed-off-by: Matt Lord <mattalord@gmail.com>

* vitessio#13175 is fixed, therefore re-instating on-deman heartbeats

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Added ToC

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Tweak comment and kick CI

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Treat isOpen as the ready/running signal.

Also align all initMutex usage.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Re-adjust comment

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Adjust CheckIsReady() to match OnlineDDL's expectation/usage

This was only using IsReady() before, now it's using
IsOpen() and IsReady().

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Get rid of log messages from SrvKeyspaceWatcher when no node/key

Signed-off-by: Matt Lord <mattalord@gmail.com>

* More corrections/tweaks

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use more convenient/clear new IsRunning function

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Revert "Use more convenient/clear new IsRunning function"

This reverts commit 9aef276 as this
change was not correct.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Further fix correct use of IsOpen(), IsRunning(), IsEnabled()

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* throttler.throttledApps cannot be nil

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* minor refactory/beautify for test

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fix flakiness of tabletmanager_throttler_topo test by: (1) proper wait-for functions, and (2) issue different queries per goroutine

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Fix typo in release notes

Signed-off-by: Matt Lord <mattalord@gmail.com>

---------

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Co-authored-by: Matt Lord <mattalord@gmail.com>
@brendar brendar mentioned this issue Jun 13, 2023
Merged
shlomi-noach added a commit that referenced this issue Jul 13, 2023
@shlomi-noach @mattlord
Deprecating and removing tablet throttler CLI flags and tests (#13246)
c71c26a 
* Table throttler: --throttler-config-via-topo now defaults to 'true'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* add deprecation message

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* endtoend tests: remove '--enable-lag-throttler' and use 'UpdateThrottlerConfig' everywhere

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* always use vtctldclient

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* use cluster.VtctldClientProcess

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* disable --throttler-config-via-topo in old throttler tests

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Remove --throttler-config-via-topo where used, since it now defaults 'true'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fix vreplication cluster setup, waiting for throttler config to apply

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* changelog

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* extend throttler threshold

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* a bit more verbose

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* fixed CLI test

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* remove old '--enable-lag-throttler' flag, introduce '--heartbeat_on_demand_duration'

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* more log info in throttler.Open()

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* more logging

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Revert to --heartbeat_enable

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Protect throttler config change application with initMutex

And in e2e test update the throttler config on the keyspace
when it's created. Only wait for the new tablets in a shard
to have the throttler enabled when adding a Shard.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* More CI testing

Signed-off-by: Matt Lord <mattalord@gmail.com>

* CI testing cont

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Yes...

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Somebody doesn't like force pushes so msg here

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Increase on-demand heartbeat duration from 10s to 1m

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use only on-demand heartbeats everywhere

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use same throttler config everywhere

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Update all keyspaces and don't fail test on missing JSON keys

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use constant heartbeats in vrepl e2e tests

Until #13175 is
fixed.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Increase workflow command timeout

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Don't wait for throttler on non-serving primaries

Signed-off-by: Matt Lord <mattalord@gmail.com>

* #13175 is fixed, therefore re-instating on-deman heartbeats

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Added ToC

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Tweak comment and kick CI

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Treat isOpen as the ready/running signal.

Also align all initMutex usage.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Re-adjust comment

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Adjust CheckIsReady() to match OnlineDDL's expectation/usage

This was only using IsReady() before, now it's using
IsOpen() and IsReady().

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Get rid of log messages from SrvKeyspaceWatcher when no node/key

Signed-off-by: Matt Lord <mattalord@gmail.com>

* More corrections/tweaks

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Use more convenient/clear new IsRunning function

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Revert "Use more convenient/clear new IsRunning function"

This reverts commit 9aef276 as this
change was not correct.

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Further fix correct use of IsOpen(), IsRunning(), IsEnabled()

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* throttler.throttledApps cannot be nil

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Remove --enable_lag_throttler flag

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Deprecate --throttler_config_via_topo

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* remove throttler mitigation code, as the problem was solved in #13195

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* deperecate throttler config flags

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Removed tabletmanager_throttler and tabletmanager_throttler_custom_config tests

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* changelog

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* remove EnableThrottler() call

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* restore default value

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* update threshold

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* update flags desc

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* using atomic.Bool

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* Update changelog/18.0/18.0.0/summary.md

Co-authored-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* use MarkDeprecated

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* do not expect flags in vttablet --help

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

* remove --throttler-config-via-topo from examples scripts

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

---------

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Co-authored-by: Matt Lord <mattalord@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component: Cluster management Component: VReplication Type: Bug
Projects
None yet
Milestone
No milestone
2 participants
@shlomi-noach @rohit-nayak-ps