Move vreplication to vitess json parser #12761

rohit-nayak-ps · 2023-03-29T14:05:32Z

Description

This PR moves away from using the ajson based binlog parser, switching instead to the new internal json parser created as part of the new evalengine work which was refactored in #12757 to be usable by other Vitess modules.

The primary motivation for this work was to resolve the problem we have today where decimals and numerics are vreplicated in a lossy manner.

Key Changes

VStreamer uses the functionality of the new parser which converts the binary representation in binlogs into a json document string.
VReplication (vplayer and vcopier) will map the json document string into a format using json_objects and json_arrays while creating the insert/update dmls in vreplication workflows.
Initially there were import cycles due to dependency b/w the go/mysql and sqlparser packages. Some code organization and a significant refactor of binlog json parsing code is part of this PR along with a new serialization of json data types using json_object() and json_array() for use in DMLs.

Related Issue(s)

Fixes: #8686

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

vitess-bot · 2023-03-29T14:05:36Z

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 test is added or modified, there should be a documentation on top of the test to explain what the expected behavior is what the test does.

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.

…at and replication constants required for json parser into sqltypes. Currently has import cycles due to use of CellAlias in json parser Signed-off-by: Rohit Nayak <rohit@planetscale.com>

…w vitess json parser Signed-off-by: Rohit Nayak <rohit@planetscale.com>

Signed-off-by: Vicent Marti <vmg@strn.cat>

…ore json values in a lossless manner using JSON_OBJECT/JSON_ARRAYs Signed-off-by: Rohit Nayak <rohit@planetscale.com>

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

…rings as part of all vrep e2e tests Signed-off-by: Rohit Nayak <rohit@planetscale.com>

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

…12799) * Give mysqlctld time to shutdown properly before restarting to account for delays in CI Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Address review comments Signed-off-by: Rohit Nayak <rohit@planetscale.com> --------- Signed-off-by: Rohit Nayak <rohit@planetscale.com>

…fix vdiff failure Signed-off-by: Rohit Nayak <rohit@planetscale.com>

mattlord

LGTM! Thank you @rohit-nayak-ps, @vmg, and @dbussink !

go/mysql/binlog/binlog_json_test.go

go/vt/vttablet/tabletmanager/vreplication/vplayer_flaky_test.go

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

… space b/w key and value added by the new serialization Signed-off-by: Rohit Nayak <rohit@planetscale.com>

rohit-nayak-ps added the Skip CI Skip CI actions from running label Mar 29, 2023

vitess-bot bot added NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsWebsiteDocsUpdate What it says labels Mar 29, 2023

vmg force-pushed the rn-vrep-move-to-vitess-json-parser branch from eee0706 to 2822a1f Compare March 29, 2023 16:33

vmg marked this pull request as ready for review March 29, 2023 16:33

vmg requested review from harshit-gangal, systay, shlomi-noach, frouioui, GuptaManan100, mattlord and deepthi as code owners March 29, 2023 16:33

dbussink force-pushed the rn-vrep-move-to-vitess-json-parser branch from c778177 to 5fe03bc Compare March 30, 2023 07:46

vmg added Type: Internal Cleanup Component: Query Serving and removed NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsWebsiteDocsUpdate What it says Skip CI Skip CI actions from running labels Mar 30, 2023

rohit-nayak-ps requested a review from vmg as a code owner March 30, 2023 20:18

rohit-nayak-ps added Component: VReplication Do Not Merge labels Mar 30, 2023

rohit-nayak-ps added 2 commits April 2, 2023 22:15

Initial commit. Added json support to tracked buffer. Moved FormatFlo…

b3d919c

…at and replication constants required for json parser into sqltypes. Currently has import cycles due to use of CellAlias in json parser Signed-off-by: Rohit Nayak <rohit@planetscale.com>

Also add code to move vstreamer-side json binlog parser to use the ne…

50b0ef7

…w vitess json parser Signed-off-by: Rohit Nayak <rohit@planetscale.com>

rohit-nayak-ps force-pushed the rn-vrep-move-to-vitess-json-parser branch from a9ad6a5 to 25c4589 Compare April 2, 2023 20:16

vmg and others added 5 commits April 2, 2023 22:17

mysql: extract binlog package

07fc909

Signed-off-by: Vicent Marti <vmg@strn.cat>

add missing license

b9b5edf

Signed-off-by: Vicent Marti <vmg@strn.cat>

Use Vitess json parser in VStreamer VEvents and VReplication DMLs. St…

c02ee2b

…ore json values in a lossless manner using JSON_OBJECT/JSON_ARRAYs Signed-off-by: Rohit Nayak <rohit@planetscale.com>

Remove unused code incorrectly added to TrackedBuffer

085e9c9

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

Add table with json columns and multiple rows with a bunch of json st…

25c4589

…rings as part of all vrep e2e tests Signed-off-by: Rohit Nayak <rohit@planetscale.com>

rohit-nayak-ps and others added 3 commits April 3, 2023 12:49

Add json_tbl to second customer vschema

3952928

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

Add e2e tests for geometry columns. Make geometry byte comparable to …

c2bae1a

…fix vdiff failure Signed-off-by: Rohit Nayak <rohit@planetscale.com>

mattlord approved these changes Apr 4, 2023

View reviewed changes

go/mysql/binlog/binlog_json_test.go Show resolved Hide resolved

go/vt/vttablet/tabletmanager/vreplication/vplayer_flaky_test.go Show resolved Hide resolved

rohit-nayak-ps changed the title ~~[WIP] Move vreplication to vitess json parser~~ Move vreplication to vitess json parser Apr 4, 2023

rohit-nayak-ps removed the Do Not Merge label Apr 4, 2023

Add tests with nested json docs. Remove GEOMETRY from TestConsistentL…

70962be

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

rohit-nayak-ps requested review from dbussink and removed request for systay, frouioui and GuptaManan100 April 4, 2023 20:43

rohit-nayak-ps requested a review from derekperkins as a code owner April 5, 2023 10:59

Fix expected json value for TestMessage to account for the additional…

2052a3e

… space b/w key and value added by the new serialization Signed-off-by: Rohit Nayak <rohit@planetscale.com>

This was referenced Apr 5, 2023

[Work In Progress] VReplicating JSON Columns: fix data loss with large NUMERIC and DECIMAL values #12731

Closed

WIP: VStreamer: improve representation of integers in json data types #9508

Closed

dbussink approved these changes Apr 6, 2023

View reviewed changes

dbussink mentioned this pull request Apr 6, 2023

vstream: json integers get converted into scientific notation #8686

Closed

rohit-nayak-ps merged commit e1be0c7 into vitessio:main Apr 6, 2023

rohit-nayak-ps deleted the rn-vrep-move-to-vitess-json-parser branch April 6, 2023 06:25

dbussink mentioned this pull request Apr 7, 2023

json: Fix JSON SQL serialization and ensure to run tests #12861

Merged

4 tasks

This was referenced Sep 9, 2023

VReplication: Handle SQL NULL and JSON 'null' correctly for JSON columns #13944

Merged

vreplication: support JSON columns without CONVERT #10084

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move vreplication to vitess json parser #12761

Move vreplication to vitess json parser #12761

rohit-nayak-ps commented Mar 29, 2023 •

edited

Loading

vitess-bot bot commented Mar 29, 2023

mattlord left a comment

Move vreplication to vitess json parser #12761

Move vreplication to vitess json parser #12761

Conversation

rohit-nayak-ps commented Mar 29, 2023 • edited Loading

Description

Key Changes

Related Issue(s)

Checklist

vitess-bot bot commented Mar 29, 2023

Review Checklist

General

If a new flag is being introduced:

If a workflow is added or modified:

Bug fixes

Non-trivial changes

New/Existing features

Backward compatibility

mattlord left a comment

Choose a reason for hiding this comment

rohit-nayak-ps commented Mar 29, 2023 •

edited

Loading