Skip to content

slack-vitess-r15.0.5: backport required Transaction Throttler PRs, pt. 3 + ci fixes#351

Merged
timvaillancourt merged 16 commits intoslack-vitess-r15.0.5from
bp-txthrottler-pt3-slack-vitess-r15.0.5
May 21, 2024
Merged

slack-vitess-r15.0.5: backport required Transaction Throttler PRs, pt. 3 + ci fixes#351
timvaillancourt merged 16 commits intoslack-vitess-r15.0.5from
bp-txthrottler-pt3-slack-vitess-r15.0.5

Conversation

@timvaillancourt
Copy link

@timvaillancourt timvaillancourt commented May 16, 2024 •
edited
Loading

Description

Backports:

  1. txthrottler: add metrics for topoWatcher and healthCheckStreamer vitessio/vitess#13153
  2. Replace deprecated github.com/golang/mock with go.uber.org/mock vitessio/vitess#13512
  3. Per workload TxThrottler metrics vitessio/vitess#13526
  4. tx throttler: healthcheck all cells if --tx-throttler-healthcheck-cells is undefined vitessio/vitess#12477
  5. Add dry-run/monitoring-only mode for TxThrottler vitessio/vitess#13604
  6. txthrottler: remove txThrottlerConfig struct, rely on tabletenv vitessio/vitess#13624
  7. tx throttler: remove unused topology watchers vitessio/vitess#14412
  8. tx_throttler: delete topo watcher metric instead of deprecating vitessio/vitess#14445
  9. TxThrottler: dont throttle unless lag vitessio/vitess#14789

Flaky test fix: 2b0a28c - this fixes an unrecognized field error because v15 dropped the io_thread_running and sql_thread_running fields - causing it to error unmarshalling v14 results. This commit restores the 2 x bools so unmarshalling passes

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on the CI
  • Documentation was added or is not required

Deployment Notes

timvaillancourt and others added 9 commits May 16, 2024 20:38
@timvaillancourt
txthrottler: add metrics for topoWatcher and healthCheckStreamer (vit…
bc70603 
…essio#13153)

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@Juneezee @shlomi-noach @timvaillancourt
Replace deprecated github.com/golang/mock with go.uber.org/mock (v…
010239a 
…itessio#13512)

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@ejortegau @timvaillancourt
Per workload TxThrottler metrics (vitessio#13526)
81350c5 
Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com>
@timvaillancourt @shlomi-noach
tx throttler: healthcheck all cells if `--tx-throttler-healthcheck-ce…
bd9da81 
…lls` is undefined (vitessio#12477)

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Co-authored-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@ejortegau @timvaillancourt
Add dry-run/monitoring-only mode for TxThrottler (vitessio#13604)
e1ac65f 
Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com>
Signed-off-by: Eduardo J. Ortega U. <5791035+ejortegau@users.noreply.github.com>
@timvaillancourt
txthrottler: remove txThrottlerConfig struct, rely on tabletenv (…
098db61 
…vitessio#13624)

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@deepthi @timvaillancourt
tx throttler: remove unused topology watchers (vitessio#14412)
65333f6 
Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi @timvaillancourt
tx_throttler: delete topo watcher metric instead of deprecating (vite…
97d455e 
…ssio#14445)

Signed-off-by: deepthi <deepthi@planetscale.com>
@ejortegau @timvaillancourt
TxThrottler: dont throttle unless lag (vitessio#14789)
bb2c0e1 
Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com>
@salesforce-cla
Copy link

salesforce-cla bot commented May 16, 2024

Thanks for the contribution! Before we can merge this, we need @Juneezee @deepthi to sign the Salesforce Inc. Contributor License Agreement.

@github-actions github-actions bot added this to the v15.0.5 milestone May 16, 2024
@salesforce-cla
Copy link

salesforce-cla bot commented May 16, 2024

Thanks for the contribution! Before we can merge this, we need @deepthi to sign the Salesforce Inc. Contributor License Agreement.

@timvaillancourt timvaillancourt marked this pull request as ready for review May 16, 2024 18:55
@timvaillancourt timvaillancourt requested a review from a team as a code owner May 16, 2024 18:55
@timvaillancourt
go test -v
c243a2c 
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt
add mutex to flaky parseFlags()
3ce0e3f 
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt
Merge branch 'slack-vitess-r15.0.5' into bp-txthrottler-pt3-slack-vit…
95a976b 
…ess-r15.0.5
@timvaillancourt
revert tweaks for flaky tests
dd86486 
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt
Copy link
Author

timvaillancourt commented May 20, 2024

This PR should fix the problems with the Upgrade Downgrade Testing Reparent Old Vtctl CI

@timvaillancourt
fix protojson err
2b0a28c 
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt
make vtadmin_web_proto_types
d443a7e 
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt
Copy link
Author

timvaillancourt commented May 21, 2024

This commit should fix the incompatibilities due to replicationdata.proto differences causing protojson to fail un-marshalling an unrecognised/deprecated field

timvaillancourt
timvaillancourt commented May 21, 2024
View reviewed changes
proto/replicationdata.proto
// 2 used to be io_thread_running. Instead io_state is used.
// 3 used to be sql_thread_running. Instead sql_state is used.
reserved 2, 3;
bool io_thread_running = 2;
Copy link
Author

@timvaillancourt timvaillancourt May 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The deprecation of field 2 + 3 (v16) was reverted just to stop protojson returning unmarshalling errors. Nothing actually uses this value

@timvaillancourt timvaillancourt changed the title slack-vitess-r15.0.5: backport required Transaction Throttler PRs, pt. 3 slack-vitess-r15.0.5: backport required Transaction Throttler PRs, pt. 3 + ci fixes May 21, 2024
@timvaillancourt
remove debug t.Logf(...)
f53d6b9 
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt timvaillancourt merged commit 4a92ae7 into slack-vitess-r15.0.5 May 21, 2024
@timvaillancourt timvaillancourt deleted the bp-txthrottler-pt3-slack-vitess-r15.0.5 branch May 21, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vmogilev vmogilev vmogilev approved these changes

Assignees

No one assigned

Labels

cla:missing txthrottler upstream-backport An upstream backport v15 v18-backport v19-backport

Projects

None yet

Milestone

v15.0.5

Development

Successfully merging this pull request may close these issues.

5 participants

@timvaillancourt @vmogilev @Juneezee @ejortegau @deepthi