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

VReplication: ability to compress gtid when stored in _vt.vreplication's pos column #7877

Merged
merged 5 commits into from
May 8, 2021

Conversation

rohit-nayak-ps
Copy link
Contributor

@rohit-nayak-ps rohit-nayak-ps commented Apr 15, 2021

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

Description

During the replication phase of a workflow the gtid of every transaction is recorded in the pos column of the _vt.vreplication table so that the target can keep track of their current position with respect to the source. We also have a heartbeat (default 1 second, configurable to a maximum of one minute) that updates the time_updated column for a proof-of-life of the stream.

For long running workflows this can cause a significant increase in the size of binlogs especially in cases where there is a high write QPS or if there have been a lot of reparenting operations increasing the size of each gtid. This problem is accentuated because in the full mode of RBR (which Vitess requires) the gtids are present both in the before and after images.

This PR adds an option to vttablet to compress gtids before storing it into the pos column. We use the same (zlib) algorithm as implemented by the compress() function. This makes it easier to inspect the column in sql using select uncompress(pos) from _vt.vreplication. This alss means we can use the (presumably efficient) mysql compress() function to compress and only need to write go code for the decompression. The compression achieved by this should be between 60-80% since the gtid is a hex string.

We put this functionality behind the -vreplication_store_compressed_gtid boolean flag (default: false), since users may prefer to have clear text positions for readability.

Independent of this option, we support both compressed and clear-text gtids while reading the pos column. So if a vttablet is first run with the option on and later it is turned off we will still be able to read the compressed gtid and future updates will store the pos in clear text. (and vice-versa).

Note that movetables or reshard workflows tend to be short-lived. Most of the data migration is done using in the copy phase which does not generate a lot of pos updates. So if you don't expect to use long-running Materialize flows it may not be very beneficial to enable this feature.

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

@rohit-nayak-ps rohit-nayak-ps added Component: VReplication Type: Enhancement Logical improvement (somewhere between a bug and feature) Type: Performance labels May 3, 2021
@rohit-nayak-ps rohit-nayak-ps marked this pull request as ready for review May 3, 2021 14:58
@rohit-nayak-ps rohit-nayak-ps requested review from shlomi-noach, ajm188 and a team May 3, 2021 14:58
Copy link
Contributor

@ajm188 ajm188 left a comment

Choose a reason for hiding this comment

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

I have a meta-question about the implementation details, but the approach of allowing forward/backward compatibility around using compression is really smart!

dataLength = binary.LittleEndian.Uint32(dataLengthBytes)

// uncompress using zlib
inputData := inputBytes[4:]
Copy link
Contributor

Choose a reason for hiding this comment

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

I got curious why this bit math was required and not provided by compress/zlib so I started to play around with this.

From my read and brief experimentation to write some benchmarks, that golang's zlib library expects this header to be present, so chopping off the first four bytes will always result in a zlib.ErrHeader result from zlib.NewReader: https://play.golang.org/p/p6XWE12Um89

So, I commented out the header truncation, and was able to exercise the rest of the decompression code as written, but it would still fail on the length check (line 740), which I haven't looked into yet. In any case, the results between doing this vs just calling into NewReader and then checking if the error is zlib.ErrHeader (which would indicate the data was not compressed with zlib) seem negligible. Benchmark repo.

Note: I did not check this against gtids compressed by mysql yet, so we should verify if the behavior of mysql's compress is actually different from golang's zlib compression. From the docs, it seems (but I am very unfamiliar, so happy to be told I'm wrong here), that mysql doesn't guarantee to be using zlib (due to "a compression library such as zlib" vs "this compression is always done with specifically zlib")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The four bytes are additionally added by MySQL. They are not related the header of zlib-generated data. Some references here:

https://bugs.mysql.com/bug.php?id=79400
https://www.bennadel.com/blog/3152-running-mysql-compress-and-uncompress-compatible-methods-in-coldfusion.htm

https://dev.mysql.com/doc/refman/8.0/en/innodb-compression-internals.html says zlib is used. Maybe there is a way to use a different library while compiling MySQL but I haven't seen references to anyone using a different library. Also I saw references to tools and drivers that have assumed zlib.

So if it is possible to link in other compression libraries and someone does it, we will have to update this implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification!! This makes sense to me now

Copy link
Contributor

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

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

looks good!

…et option

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
@rohit-nayak-ps rohit-nayak-ps merged commit 4578a9b into vitessio:master May 8, 2021
@rohit-nayak-ps rohit-nayak-ps deleted the rn-gtid-compression branch May 8, 2021 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: VReplication Type: Enhancement Logical improvement (somewhere between a bug and feature) Type: Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants