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

Manual backport of upstream https://github.com/vitessio/vitess/pull/17300 #17306

Conversation

ejortegau
Copy link
Contributor

This PR is a manual backport of upstream #17300
The information below is a copy of what the description of the upstream PR

Description

This PR addresses fixes issues in PRS & ERS preference for tablets that are not taking backups. It does away with redundant field definitions in some of the proto messages and addresses segfault that could take place during ERS.

Related Issue(s)

#17299

Checklist

  • "Backport to:" labels have been added if this change should be back-ported to release branches
  • If this change is to be back-ported to previous releases, a justification is included in the PR description
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on CI?
  • Documentation was added or is not required

Deployment Notes

N/A

harshit-gangal and others added 30 commits October 14, 2022 09:51
Signed-off-by: Harshit Gangal <[email protected]>
* Plan order by Count()

Signed-off-by: Florent Poinsard <[email protected]>

* Clean up the new aggregation E2E test

Signed-off-by: Florent Poinsard <[email protected]>

* Push more order by needs to the select list

Signed-off-by: Florent Poinsard <[email protected]>

* Remove unrequired code in TestOrderByCount

Signed-off-by: Florent Poinsard <[email protected]>

* remove unwanted directory

Signed-off-by: Florent Poinsard <[email protected]>

Signed-off-by: Florent Poinsard <[email protected]>

Signed-off-by: Florent Poinsard <[email protected]>
…k overflow (vitessio#11499)

Signed-off-by: Andres Taylor <[email protected]>

Signed-off-by: Andres Taylor <[email protected]>
* test: added failing e2e test

Signed-off-by: Harshit Gangal <[email protected]>

* log txID and reserveID in stream execute

Signed-off-by: Harshit Gangal <[email protected]>

* fix: maintain list of qd per key on the map and check for current connection id while removing

Signed-off-by: Harshit Gangal <[email protected]>

* added additional comments

Signed-off-by: Harshit Gangal <[email protected]>

Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Harshit Gangal <[email protected]>
…ease-15.0

[release-15.0] fix: reserve connection to follow query timeout when outside of transaction (vitessio#11490)
* feat: added test for vtorc not being able to handle mutliple failures and fix it

Signed-off-by: Manan Gupta <[email protected]>

* test: fix code to delete rdonly tablet from the correct list

Signed-off-by: Manan Gupta <[email protected]>

Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>

Signed-off-by: Florent Poinsard <[email protected]>
* Adding deprecate message to backup hooks

Signed-off-by: Rameez Sajwani <[email protected]>

* adding markdeprecated

Signed-off-by: Rameez Sajwani <[email protected]>

* fixing deprecation message

Signed-off-by: Rameez Sajwani <[email protected]>

* fixing messaging

Signed-off-by: Rameez Sajwani <[email protected]>

* fix flag name

Signed-off-by: Rameez Sajwani <[email protected]>

Signed-off-by: Rameez Sajwani <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Co-authored-by: Manan Gupta <[email protected]>
* removing unncessary flags across binaries

Signed-off-by: Rameez Sajwani <[email protected]>

* code review feedback

Signed-off-by: Rameez Sajwani <[email protected]>

* fixing blank space

Signed-off-by: Rameez Sajwani <[email protected]>

Signed-off-by: Rameez Sajwani <[email protected]>
[15.0] Fix query list override issue on mysql restart (vitessio#11309)
* remove excessive logging

Signed-off-by: Rameez Sajwani <[email protected]>

* code feeback

Signed-off-by: Rameez Sajwani <[email protected]>

* fixing typo

Signed-off-by: Rameez Sajwani <[email protected]>

Signed-off-by: Rameez Sajwani <[email protected]>
…io#11493)

* Move CI to mysql80

Signed-off-by: Rameez Sajwani <[email protected]>

* fix typo

Signed-off-by: Rameez Sajwani <[email protected]>

* fixing typo

Signed-off-by: Rameez Sajwani <[email protected]>

* moving to apt-config 8.2

Signed-off-by: Rameez Sajwani <[email protected]>

Signed-off-by: Rameez Sajwani <[email protected]>
I accidentally stumbled over the behavior outlined in
`flag.PrintDefaults` [1], which `pflag` replicates, specifically:

> The listed type, here int, can be changed by placing a back-quoted
> name in the flag's usage string; the first such item in the message is
> taken to be a parameter name to show in the message and the back quotes
> are stripped from the message when displayed.

[1]: https://pkg.go.dev/flag#PrintDefaults.

Signed-off-by: Andrew Mason <[email protected]>

Signed-off-by: Andrew Mason <[email protected]>
Co-authored-by: Andrew Mason <[email protected]>
…th transaction (vitessio#11527)

* fix: stream exec once in case of transactional connection

Signed-off-by: Harshit Gangal <[email protected]>

* test: added e2e test

Signed-off-by: Harshit Gangal <[email protected]>

* generate ci worflow

Signed-off-by: Harshit Gangal <[email protected]>

Signed-off-by: Harshit Gangal <[email protected]>
* Add web side of topology components

Signed-off-by: notfelineit <[email protected]>

* Add GetTopologyPath

Signed-off-by: notfelineit <[email protected]>

* Add protos

Signed-off-by: notfelineit <[email protected]>

* Add comment explaining why name is not needed at toplevel topo path

Signed-off-by: notfelineit <[email protected]>

* Add GetTopologyPath to vtadmin

Signed-off-by: notfelineit <[email protected]>

* Modify node code for non recursive

Signed-off-by: notfelineit <[email protected]>

* Update logic for GetTopologyPath

Signed-off-by: notfelineit <[email protected]>

* Render correct depth for nodes

Signed-off-by: notfelineit <[email protected]>

* Add tests TestGetTopologyPath

Signed-off-by: notfelineit <[email protected]>

* Add GetTopologyPath command

Signed-off-by: notfelineit <[email protected]>

* Run lint prettier fix

Signed-off-by: notfelineit <[email protected]>

* Consolidate DecodeContent to vt/topo package - can't put in vt/topo/topoproto because of import cycle

Signed-off-by: notfelineit <[email protected]>

* Add rbac check for TopologyResource

Signed-off-by: notfelineit <[email protected]>

* Update DecodeContent of zkcmd.go

Signed-off-by: notfelineit <[email protected]>

* Update help text

Signed-off-by: notfelineit <[email protected]>

* Run make proto again

Signed-off-by: notfelineit <[email protected]>

* Run lint prettier fix

Signed-off-by: notfelineit <[email protected]>

Signed-off-by: notfelineit <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Co-authored-by: Florent Poinsard <[email protected]>
…itessio#11528)

* Allow version to be accessible via the -v shorthand

Signed-off-by: Andrew Mason <[email protected]>

* update help text

Signed-off-by: Andrew Mason <[email protected]>

* fix nonidempotence for tests

Signed-off-by: Andrew Mason <[email protected]>

Signed-off-by: Andrew Mason <[email protected]>

Signed-off-by: Andrew Mason <[email protected]>
* feat: fix default for cell flag in vtgate

Signed-off-by: Manan Gupta <[email protected]>

* feat: deprecate --enable-query-plan-field-caching and --enable_query_plan_field_caching properly

Signed-off-by: Manan Gupta <[email protected]>

* feat: properly deprecate prefill flags

Signed-off-by: Manan Gupta <[email protected]>

* feat: properly deprecate filtered_replication_wait_time flag in switch writes

Signed-off-by: Manan Gupta <[email protected]>

Signed-off-by: Manan Gupta <[email protected]>
…gainst unsharded tables (vitessio#11461) (vitessio#11530)

* Only perform vindex logic for deletes against sharded keyspaces.

Signed-off-by: Arthur Schreiber <[email protected]>

* Add a testcase covering deletes on an unsharded reference table.

Signed-off-by: Arthur Schreiber <[email protected]>

* Check if target keyspace is unsharded instead of checking opcode.

Signed-off-by: Arthur Schreiber <[email protected]>

Signed-off-by: Arthur Schreiber <[email protected]>

Signed-off-by: Arthur Schreiber <[email protected]>
Co-authored-by: Arthur Schreiber <[email protected]>
* test: added failing e2e test

Signed-off-by: Harshit Gangal <[email protected]>

* fix: sequence execution of concate engine when in transaction

Signed-off-by: Harshit Gangal <[email protected]>

* fix: concatenate fix for olap transaction

Signed-off-by: Harshit Gangal <[email protected]>

* fix: test race

Signed-off-by: Harshit Gangal <[email protected]>

Signed-off-by: Harshit Gangal <[email protected]>
…sio#11526)

* don't rewrite HAVING predicates that use table columns

Signed-off-by: Andres Taylor <[email protected]>

* Revert the changes made in vitessio#11306

Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>

* Fix early rewriter test

Signed-off-by: Florent Poinsard <[email protected]>

Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Co-authored-by: Andres Taylor <[email protected]>

Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Co-authored-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>

Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>

Signed-off-by: Manan Gupta <[email protected]>
* feat: add hyperlink using fqdn in vtgates

Signed-off-by: Manan Gupta <[email protected]>

* feat: provide correct vtgate web address in examples

Signed-off-by: Manan Gupta <[email protected]>

* refactor: fix prettier errors

Signed-off-by: Manan Gupta <[email protected]>

Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>

Signed-off-by: Florent Poinsard <[email protected]>
* removing redundant flags across binaries

Signed-off-by: Rameez Sajwani <[email protected]>

* adding app pool flag back for some binaries

Signed-off-by: Rameez Sajwani <[email protected]>

* running generate_ci on prscomplex.yml

Signed-off-by: Rameez Sajwani <[email protected]>

* code review

Signed-off-by: Rameez Sajwani <[email protected]>

* fixing markdeprecated error

Signed-off-by: Rameez Sajwani <[email protected]>

* code feedback

Signed-off-by: Rameez Sajwani <[email protected]>

* doing some minor changes

Signed-off-by: Rameez Sajwani <[email protected]>

* fixing typo

Signed-off-by: Rameez Sajwani <[email protected]>

Signed-off-by: Rameez Sajwani <[email protected]>
…hin GoLand and to parse test flags in vtgate to allow running unit tests (vitessio#11551)

* Update logic for parsing test flags to run unit tests within GoLand. Parse test flags in vtgate to allow running unit tests

Signed-off-by: Rohit Nayak <[email protected]>

* Minor changes after self-review

Signed-off-by: Rohit Nayak <[email protected]>

Signed-off-by: Rohit Nayak <[email protected]>
* feat: deprecate initshardprimary

Signed-off-by: Manan Gupta <[email protected]>

* feat: add summary docs

Signed-off-by: Manan Gupta <[email protected]>

* feat: deprecate the correct command

Signed-off-by: Manan Gupta <[email protected]>

* test: fix expected output

Signed-off-by: Manan Gupta <[email protected]>

Signed-off-by: Manan Gupta <[email protected]>
Instead of relying on a specific version check, detect if the new redo
log location is present or not.

This makes the backup logic independent from the version check against
MySQL. With this change, 8.0.30 can be backed up just as well as any
other version.

Solves the backup part of
vitessio#11554 by removing the
dependency on the version check. There might be still other places where
that issue can crop up, but it solves the immediate breaking issue with
backups.

Signed-off-by: Dirkjan Bussink <[email protected]>

Signed-off-by: Dirkjan Bussink <[email protected]>
Co-authored-by: Dirkjan Bussink <[email protected]>
Signed-off-by: Rameez Sajwani <[email protected]>

Signed-off-by: Rameez Sajwani <[email protected]>
venkatraju and others added 23 commits June 28, 2024 16:42
implement flow based tablet load balancer

Signed-off-by: Michael Demmer <[email protected]>
Signed-off-by: Venkatraju V <[email protected]>
Co-authored-by: Michael Demmer <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
…on` (#442)

* `slack-15.0`: reset semi-sync in `setReplicationSourceRepairReplication`

Signed-off-by: Tim Vaillancourt <[email protected]>

* reorder shard lock

Signed-off-by: Tim Vaillancourt <[email protected]>

* try 100% percona-xtrabackup-80

Signed-off-by: Tim Vaillancourt <[email protected]>

* Revert "try 100% percona-xtrabackup-80"

This reverts commit e67be76.

* percona-xtrabackup-24 problem fix attempt 2

Signed-off-by: Tim Vaillancourt <[email protected]>

* remove currentPrimary

Signed-off-by: Tim Vaillancourt <[email protected]>

* empty commit to fix CI

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
* Allow setting grpc send and recv message sizes independently

Signed-off-by: Henry Robinson <[email protected]>

* Add comments

Signed-off-by: Henry Robinson <[email protected]>

* Update .txt files

Signed-off-by: Henry Robinson <[email protected]>

* update help txt ordering

Signed-off-by: Priya Bibra <[email protected]>

---------

Signed-off-by: Henry Robinson <[email protected]>
Signed-off-by: Priya Bibra <[email protected]>
Co-authored-by: Priya Bibra <[email protected]>
Signed-off-by: Henry Robinson <[email protected]>
…492)

* WIP

Signed-off-by: Tim Vaillancourt <[email protected]>

* update all flags

Signed-off-by: Tim Vaillancourt <[email protected]>

* update e2e flag test

Signed-off-by: Tim Vaillancourt <[email protected]>

* 2 missing flags

Signed-off-by: Tim Vaillancourt <[email protected]>

* rename Type()

Signed-off-by: Tim Vaillancourt <[email protected]>

* empty commit to test ci

Signed-off-by: Tim Vaillancourt <[email protected]>

* fakesqldb: Guard query log usage with lock (vitessio#12813)

* fakesqldb: Guard query log usage with lock

This lock is used around adding to the query log, but it means we also
need to use the lock when reading from it or when resetting it.

Signed-off-by: Dirkjan Bussink <[email protected]>

* grpcvtctldserver: Fix alias for loop reuse

Signed-off-by: Dirkjan Bussink <[email protected]>

* unit_race: Increase runtime

We've been adding a lot of tests for the evalengine and we're bumping up
to the timeout.

Signed-off-by: Dirkjan Bussink <[email protected]>

---------

Signed-off-by: Dirkjan Bussink <[email protected]>

* remove t.Parallel() like upstream did

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Dirkjan Bussink <[email protected]>
Co-authored-by: Dirkjan Bussink <[email protected]>
* `txthrottler`: move `ThrottlerInterface` to `go/vt/throttler`, use `slices` pkg, add stats (vitessio#16248)

Signed-off-by: Tim Vaillancourt <[email protected]>

* revert to `reflect`

Signed-off-by: Tim Vaillancourt <[email protected]>

* Support passing filters to `discovery.NewHealthCheck(...)`

Signed-off-by: Tim Vaillancourt <[email protected]>

* Update go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

Co-authored-by: Matt Lord <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>

* Address some PR suggestions

Signed-off-by: Tim Vaillancourt <[email protected]>

* PR ctx suggestion

Signed-off-by: Tim Vaillancourt <[email protected]>

* fix test

Signed-off-by: Tim Vaillancourt <[email protected]>

* simplify updateHealthCheckCells signature

Signed-off-by: Tim Vaillancourt <[email protected]>

* Fix race in `replicationLagModule` of `go/vt/throttle`

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
Co-authored-by: Matt Lord <[email protected]>
…tessio#13582) (#497)

* VReplication: Make Source Tablet Selection More Robust (vitessio#13582)

* update sprintf args.

* `slack-15.0`: pre-backport `txthrottler` crash fixes (#480)

* `txthrottler`: move `ThrottlerInterface` to `go/vt/throttler`, use `slices` pkg, add stats (vitessio#16248)

Signed-off-by: Tim Vaillancourt <[email protected]>

* revert to `reflect`

Signed-off-by: Tim Vaillancourt <[email protected]>

* Support passing filters to `discovery.NewHealthCheck(...)`

Signed-off-by: Tim Vaillancourt <[email protected]>

* Update go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go

Co-authored-by: Matt Lord <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>

* Address some PR suggestions

Signed-off-by: Tim Vaillancourt <[email protected]>

* PR ctx suggestion

Signed-off-by: Tim Vaillancourt <[email protected]>

* fix test

Signed-off-by: Tim Vaillancourt <[email protected]>

* simplify updateHealthCheckCells signature

Signed-off-by: Tim Vaillancourt <[email protected]>

* Fix race in `replicationLagModule` of `go/vt/throttle`

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
Co-authored-by: Matt Lord <[email protected]>

* ignore unused wrangler weighted semaphore

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
Co-authored-by: Tim Vaillancourt <[email protected]>
Co-authored-by: Matt Lord <[email protected]>
* Backport Use GetTabletsByCell in healthcheck

This backports upstram PR vitessio#14693, with a few minor changes to make it work with the Go
version we are using and a small change to topology_watcher.go so that test cases reflect
and test for the same behavior as the upstream code. The description of the original PR
follows:

VTGate's healthcheck module currently calls GetTablet for each tablet alias that it discovers
in a cell. Instead we can use GetTabletsForCell to fetch all tablets for a cell at once.

This PR does a few more things:

* GetTabletsForCell now handles the case where the response size violates gRPC limits by
  falling back to one tablet at a time in case of error.
* Previously, the one tablet at a time method had unlimited concurrency. In this PR we
  introduce a configuration option for concurrency.
* We pass topoReadConcurrency from healthcheck into GetTabletsForCell.
* The behavior of --refresh_known_tablets flag is different now. Previously we would not
  read those tablets at all, now we do read them, but ignore any changes if they are
  already known.

The basic fix has already been tried in production and shown to reduce the number of Get
calls from vtgate -> topo from O(n) to O(1).

We can consider deprecating and deleting --refresh_known_tablets in a future release.
The concerns that originally motivated adding that flag in vitessio#3965 are alleviated by fetching
all tablets in one call to the topo.
…et` (vitessio#15897) (#525)

* `slack-15.0`: Add sql text counts stats to `vtcombo`,`vtgate`+`vttablet` (vitessio#15897)

Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Harshit Gangal <[email protected]>
Co-authored-by: Harshit Gangal <[email protected]>
Co-authored-by: Deepthi Sigireddi <[email protected]>

* Update help as well

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Harshit Gangal <[email protected]>
Co-authored-by: Harshit Gangal <[email protected]>
Co-authored-by: Deepthi Sigireddi <[email protected]>
#510)

* fixing issue with xtrabackup and long gtids (vitessio#16304)

Signed-off-by: Renan Rangel <[email protected]>

* fix file format

* fix format

---------

Signed-off-by: Renan Rangel <[email protected]>
Co-authored-by: Renan Rangel <[email protected]>
Co-authored-by: Tim Vaillancourt <[email protected]>
* `slack-15.0`: backport PR vitessio#16852

Signed-off-by: Tim Vaillancourt <[email protected]>

* make vtadmin_web_proto_types

Signed-off-by: Tim Vaillancourt <[email protected]>

* fix v15 tests

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
* Fix healthcheck filters

vitessio#16871 added filtering in the health check,
but later #514 undid that. So aiming at
fixing it here.

With this change, essentially, the filters passed in NewHealthCheck() get actually
applied to the topology watchers, instead of it using a newly instantiated, empty
filter as it's currently happening.

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* Improve tests by getting them from vitessio#16871

Signed-off-by: Eduardo J. Ortega U <[email protected]>

---------

Signed-off-by: Eduardo J. Ortega U <[email protected]>
Co-authored-by: Tim Vaillancourt <[email protected]>
* `slack-15.0`: support consul topo stale reads

Signed-off-by: Tim Vaillancourt <[email protected]>

* update e2e tests

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Eduardo J. Ortega U. <[email protected]>
Signed-off-by: Eduardo J. Ortega U. <[email protected]>
Copy link
Contributor

vitess-bot bot commented Dec 2, 2024

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

  • Ensure that the Pull Request has a descriptive title.
  • Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

  • Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

  • Apply the release notes (needs details) label if users need to know about this change.
  • New features should be documented.
  • There should be some code comments as to why things are implemented the way they are.
  • There should be a comment at the top of each new or modified test to explain what the test does.

New flags

  • Is this flag really necessary?
  • Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

  • Each item in Jobs should be named in order to mark it as required.
  • If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

  • Protobuf changes should be wire-compatible.
  • Changes to _vt tables and RPCs need to be backward compatible.
  • RPC changes should be compatible with vitess-operator
  • If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
  • vtctl command output order should be stable and awk-able.

@vitess-bot vitess-bot bot added NeedsBackportReason If backport labels have been applied to a PR, a justification is required NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsIssue A linked issue is missing for this Pull Request NeedsWebsiteDocsUpdate What it says labels Dec 2, 2024
@ejortegau ejortegau closed this Dec 2, 2024
@github-actions github-actions bot added this to the v22.0.0 milestone Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsBackportReason If backport labels have been applied to a PR, a justification is required NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsIssue A linked issue is missing for this Pull Request NeedsWebsiteDocsUpdate What it says
Projects
None yet
Development

Successfully merging this pull request may close these issues.