Skip to content

Move towards MySQL 8.0 as the default template generation (#11153)#11619

Merged
harshit-gangal merged 32 commits intovitessio:release-13.0from
planetscale:v13-cherrypick11153
Nov 22, 2022
Merged

Move towards MySQL 8.0 as the default template generation (#11153)#11619
harshit-gangal merged 32 commits intovitessio:release-13.0from
planetscale:v13-cherrypick11153

Conversation

@rsajwani
Copy link
Contributor

@rsajwani rsajwani commented Nov 2, 2022

  • Move towards MySQL 8.0 as the default template generation

This upgrades the remaining things to Ubuntu 20.04 and makes MySQL 8.0 the default we run tests against. We still have tests for MySQL 5.7 but those are now explicitly opted into.

This should finish up the Ubuntu 20.04 upgrade and also makes things easier for the future when we need to upgrade again.

Signed-off-by: Dirkjan Bussink d.bussink@gmail.com

  • CI: rename shard vtorc_8.0 to vtorc_5.7, change expected test output for 8.0

Signed-off-by: deepthi deepthi@planetscale.com

  • CI: increase timeout for 8.0 tests on the actual test step from 30 to 45 mins

Signed-off-by: deepthi deepthi@planetscale.com

  • CI: increase timeout to 45 minutes for mysql57 tests too. We really only need this for vtorc, but I've made the change to the template so all tests get it.

Signed-off-by: deepthi deepthi@planetscale.com

  • CI: fix vtorc test to work with both 5.7 and 8.0

Signed-off-by: deepthi deepthi@planetscale.com

  • CI: missed docker flag in mysql57 template, one more fix to vtorc test

Signed-off-by: deepthi deepthi@planetscale.com

  • removing spaces from pb file

Signed-off-by: Rameez Sajwani rameezwazirali@hotmail.com

  • removing spaces in pb file part 2

Signed-off-by: Rameez Sajwani rameezwazirali@hotmail.com

Signed-off-by: Dirkjan Bussink d.bussink@gmail.com
Signed-off-by: deepthi deepthi@planetscale.com
Signed-off-by: Rameez Sajwani rameezwazirali@hotmail.com
Co-authored-by: deepthi deepthi@planetscale.com
Co-authored-by: Rameez Sajwani rameezwazirali@hotmail.com

Description

There are two main changes in CI workflow been done as part of this PR.

  • Move to ubuntu 20.04
  • Upgrade Mysql 5.7 to Mysql 8.0.25

This PR cherry-pick (#11153) the change from main where we have made Mysql8.0.x as default database (from Mysql 5.7). There were certain issues which were hard to cherry-pick from main as a result of moving Mysql8.0, especially failures in onlineddl_vrepl_suite which compelled us to pin Mysql version to 8.0.25. For example we no longer use Tengo whereas in v13 we used it for schemadiff. To remove Tango from v13 is a bigger work item.

Installing Deb files for 8.0.25 was giving a lot of pain during requisite deployment, therefore, we choose to un-tar the package with binaries and set VT_MYSQL_ROOT directory during run end-to-end step in workflow.

Apart from workflow changes, there were some compatibility issues between Mysql5.7 and Mysql8.0 which resulted in test failures. They are also addressed in this PR.

Related Issue(s)

#11511

Checklist

  • "Backport me!" label has been added if this change should be backported
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

…1153)

* Move towards MySQL 8.0 as the default template generation

This upgrades the remaining things to Ubuntu 20.04 and makes MySQL 8.0
the default we run tests against. We still have tests for MySQL 5.7 but
those are now explicitly opted into.

This should finish up the Ubuntu 20.04 upgrade and also makes things
easier for the future when we need to upgrade again.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

* CI: rename shard vtorc_8.0 to vtorc_5.7, change expected test output for 8.0

Signed-off-by: deepthi <deepthi@planetscale.com>

* CI: increase timeout for 8.0 tests on the actual test step from 30 to 45 mins

Signed-off-by: deepthi <deepthi@planetscale.com>

* CI: increase timeout to 45 minutes for mysql57 tests too. We really only need this for vtorc, but I've made the change to the template so all tests get it.

Signed-off-by: deepthi <deepthi@planetscale.com>

* CI: fix vtorc test to work with both 5.7 and 8.0

Signed-off-by: deepthi <deepthi@planetscale.com>

* CI: missed docker flag in mysql57 template, one more fix to vtorc test

Signed-off-by: deepthi <deepthi@planetscale.com>

* removing spaces from pb file

Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>

* removing spaces in pb file part 2

Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Co-authored-by: deepthi <deepthi@planetscale.com>
Co-authored-by: Rameez Sajwani <rameezwazirali@hotmail.com>
@vitess-bot
Copy link
Contributor

vitess-bot bot commented Nov 2, 2022

Review Checklist

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

General

  • Ensure that the Pull Request has a descriptive title.
  • If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.

If a new flag is being introduced:

  • Is it really necessary to add this flag?
  • Flag names should be clear and intuitive (as far as possible)
  • Help text should be descriptive.
  • Flag names should use dashes (-) as word separators rather than underscores (_).

If a workflow is added or modified:

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

Bug fixes

  • There should be at least one unit or end-to-end test.
  • The Pull Request description should include a link to an issue that describes the bug.

Non-trivial changes

  • There should be some code comments as to why things are implemented the way they are.

New/Existing features

  • Should be documented, either by modifying the existing documentation or creating new documentation.
  • New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

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

Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
@rsajwani rsajwani changed the title Move towards MySQL 8.0 as the default template generation (#11153) [Do not review]Move towards MySQL 8.0 as the default template generation (#11153) Nov 2, 2022
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
mattlord and others added 2 commits November 4, 2022 22:10
* Prevent deadlocks related to 0 receiver behavior

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

* Update test tables to use poller_idx

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

* Minor changes after mutex usage review in message manager + cache

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

* Use atomics for receiver count and messages pending

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

* Don't take exclusive lock when in fast path

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

* Update tests to use the new recommended message table structure

See: vitessio/website#1015

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

* Correct tests

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

* Update e2e test to use new recommended table structure

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

* Fix TestMessageStreamingPlan test

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

* Fix godriver/TestStreamMessaging test

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

* Split streamMu into streamProcessingMu and lastPollPositionMu

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

* Poller cannot take main lock w/o having X stream processing lock

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

* Improve the comments a bit

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

* Hold the main mutex during Add

This is for safe concurrency with the last receiver unsubscribing

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

* Changes after pair reviewing with Sugu

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

* Use my GitHub handle for the self reference

Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
…ick11153

Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
@rsajwani rsajwani self-assigned this Nov 8, 2022
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
…rectly

Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
Comment on lines 1401 to 1406
// truncateSQLAndBindVars calls TruncateForLog which:
// splits off trailing comments, truncates the query, re-adds the trailing comments,
// if sanitize is false appends quoted bindvar:value pairs in sorted order, and
// lastly it truncates the resulting string
//
// splits off trailing comments, truncates the query, re-adds the trailing comments,
// if sanitize is false appends quoted bindvar:value pairs in sorted order, and
// lastly it truncates the resulting string
func truncateSQLAndBindVars(sql string, bindVariables map[string]*querypb.BindVariable, sanitize bool) string {
Copy link
Member

Choose a reason for hiding this comment

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

not sure about this formatting change, what would have caused this.

Copy link
Member

@harshit-gangal harshit-gangal left a comment

Choose a reason for hiding this comment

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

LGTM for query serving side changes.

@rohit-nayak-ps
Copy link
Member

@mattlord, I noticed we have this commit (related to the messaging deadlock) in this issue: 6f0b569. Is that affected by switching to mysql 8.0 for testing?

Copy link
Member

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

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

vrep related changes look good.

@harshit-gangal harshit-gangal merged commit 8bc1fbc into vitessio:release-13.0 Nov 22, 2022
@harshit-gangal harshit-gangal deleted the v13-cherrypick11153 branch November 22, 2022 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants