Skip to content

VRep JSON Columns: fix bug where vreplication of update statements were failing#7640

Merged
deepthi merged 5 commits intovitessio:masterfrom
planetscale:rn-json-collation-bug
Mar 9, 2021
Merged

VRep JSON Columns: fix bug where vreplication of update statements were failing#7640
deepthi merged 5 commits intovitessio:masterfrom
planetscale:rn-json-collation-bug

Conversation

@rohit-nayak-ps
Copy link
Member

@rohit-nayak-ps rohit-nayak-ps commented Mar 8, 2021

Description

During vreplication we set the MySQL connection to use binary charsets. Due to unexpected MySQL behavior, with collations and json columns, like: https://bugs.mysql.com/bug.php?id=93052, we need to cast json columns to utf8mb4.. We had done this as part of #6829. It turns out however that the casting was only done for inserts on the target and not for updates.

The copy phase results only in (bulk) inserts, so any updates done before we do the copy will not face this issue. This bug surfaces only if the updates are done while we stream binlog events: during the catchup, fastforward or replicate phases.

This PR adds the casting convert(<value> as utf8mb4) while building update plans as well. The only non-test code change is here: https://github.com/vitessio/vitess/pull/7640/files#diff-944642971422bb00ed68466f784f8b586f5e800f57e0c46002d6194c517cb1a0

Signed-off-by: Rohit Nayak rohit@planetscale.com

Related Issue(s)

https://github.com/planetscale/vitess-private/issues/101

Checklist

  • Should this PR be backported?
  • Tests were added or are not required
  • Documentation was added or is not required

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build/CI
  • VTAdmin

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
@rohit-nayak-ps rohit-nayak-ps requested a review from sougou March 8, 2021 20:50
@rohit-nayak-ps rohit-nayak-ps marked this pull request as ready for review March 8, 2021 20:50
log.Errorf("TestPlayerTypes: flavor is %s", env.Flavor)

enableJSONColumnTesting := false
if strings.EqualFold(env.Flavor, "mysql57") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should be testing mysql80 as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is something that doesn't work only on percona56, we should exclude just that one flavor.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added mysql80 in addition to mysql57. The Percona flavor is mysql56 and other platforms start with mariadb.

… in CI unit tests

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
…on't have json support or have not yet been enabled

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
… in CI

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Copy link
Collaborator

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

LGTM

@deepthi deepthi merged commit 2726c84 into vitessio:master Mar 9, 2021
@deepthi deepthi deleted the rn-json-collation-bug branch March 9, 2021 16:06
@askdba askdba added this to the v10.0 milestone Mar 10, 2021
setassociative pushed a commit to tinyspeck/vitess that referenced this pull request Mar 11, 2021
VRep JSON Columns: fix bug where vreplication of update statements were failing

Signed-off-by: Richard Bailey <rbailey@slack-corp.com>
@williamsjasonj
Copy link

I believe I am still hitting this same issue running this version (Git revision 873d89a branch 'master') built on Thu Mar 11 09:16:17 UTC 2021
I have a few tables that have a json data type. When running vreplication, the table itself replicates fine, but any updates to that table throw this error "Cannot create a JSON value from a string with CHARACTER SET 'binary'" and vreplication breaks.

@rohit-nayak-ps
Copy link
Member Author

That commit seems to have been made one minute before the merge of this PR. Can you please try again with the latest code?

rohit@rohit-ubuntu:~/vitess-ps/go/test/endtoend/vreplication$ git checkout 2726c84e0b9c1eb10fe76701bf447348a01562ad
Previous HEAD position was 873d89a57 Merge pull request #7639 from AdamKorcz/fuzz1
HEAD is now at 2726c84e0 Merge pull request #7640 from planetscale/rn-json-collation-bug
rohit@rohit-ubuntu:~/vitess-ps/go/test/endtoend/vreplication$ git log
commit 2726c84e0b9c1eb10fe76701bf447348a01562ad (HEAD)
Merge: 873d89a57 a0f7933ed
Author: Deepthi Sigireddi <deepthi@planetscale.com>
Date:   Tue Mar 9 08:06:05 2021 -0800

    Merge pull request #7640 from planetscale/rn-json-collation-bug
    
    VRep JSON Columns: fix bug where vreplication of update statements were failing

commit 873d89a57839c7e1ea1ec12544a8c04add6271b7
Merge: 2d82b0066 00910c657
Author: Andres Taylor <andres@planetscale.com>
Date:   Tue Mar 9 17:05:03 2021 +0100

    Merge pull request #7639 from AdamKorcz/fuzz1
    
    Fuzzing: Add 3 fuzzers for mysql endpoints

@williamsjasonj
Copy link

Thanks! I am now running Version: 10.0.0-SNAPSHOT (Git revision de902e5 branch 'master') built on Mon Mar 15 19:02:59 UTC 2021 by vitess@d7b8a3c04182 using go1.15.6 linux/amd64
I had to restart the vreplication process, so it will take a day or two before I know if this fix works.

@williamsjasonj
Copy link

Fix is tested - replication of updates to JSON data types is confirmed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants