Skip to content

Do Not Merge: VReplication Copy Phase: Parallelize bulk insert to check for performance improvements#7728

Closed
rohit-nayak-ps wants to merge 4 commits intovitessio:mainfrom
planetscale:rn-parallelize-bulk-insert
Closed

Do Not Merge: VReplication Copy Phase: Parallelize bulk insert to check for performance improvements#7728
rohit-nayak-ps wants to merge 4 commits intovitessio:mainfrom
planetscale:rn-parallelize-bulk-insert

Conversation

@rohit-nayak-ps
Copy link
Copy Markdown
Member

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

POC

Batches multiple bulk inserts in the copy phase to test if concurrent bulk inserts improve copy phase performance.

Description

During the copy phase we stream rows and for every PacketSize of rows (default 250K bytes) we do a bulk insert. This PR batches multiple of these sets.

We add a VReplicationTableCopyTimings stat to monitor when a table copy starts and when it ends for benchmarking purposes. This metric is per vreplication stream.

This feature is behind an experimental vttablet flag. To enable it use -vreplication_experimental_flags 2 on the target.

You can specify the number of bulk inserts using -vreplication_parallel_bulk_inserts 16 Default is 4

Approach

Instead of inserting one batch at a time we collect N batches and insert them in parallel. Commits are done in order.

Checklist

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

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

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

@rohit-nayak-ps rohit-nayak-ps force-pushed the rn-parallelize-bulk-insert branch from 78de2de to 02abe6f Compare March 23, 2021 16:53
@rohit-nayak-ps rohit-nayak-ps force-pushed the rn-parallelize-bulk-insert branch from ca7ae94 to f7a41c9 Compare April 19, 2021 20:35
…cs and flags.

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
@rohit-nayak-ps rohit-nayak-ps force-pushed the rn-parallelize-bulk-insert branch from 0664e10 to 5410ad6 Compare April 26, 2021 14:02
…metric fails in Prometheus and wondering if it failing Prometheus' naming convention

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
@github-actions
Copy link
Copy Markdown
Contributor

This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:

  • Push additional commits to the associated branch.
  • Remove the stale label.
  • Add a comment indicating why it is not stale.

If no action is taken within 7 days, this PR will be closed.

@github-actions github-actions bot added the Stale Marks PRs as stale after a period of inactivity, which are then closed after a grace period. label Jul 20, 2022
@maxenglander
Copy link
Copy Markdown
Collaborator

Hey @rohit-nayak-ps, at PlanetScale we're pretty keen to speed up MoveTables for customers with terrabytes-large tables. I came across this PR while seeing how much of a performance improvement we can get from parallelizing bulk inserts. Before I discovered this PR, I created this one, which takes a pretty similar approach to yours.

I was wondering: would you be interested in reviving this PR? Or if you too busy, letting me revive it? I created a branch based on yours that resolves those merge conflicts. I tested it out and got pretty good performance results. If those changes look good to you, I can open a PR against this one, or apply the commits directly.

@rohit-nayak-ps
Copy link
Copy Markdown
Member Author

Replaced by #10828 which is essentially the same changes over the latest main. Did this in preference to a merge since it is a year since I started this and this felt easier and safer to do rather than fix conflicts in this one.

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

Labels

Stale Marks PRs as stale after a period of inactivity, which are then closed after a grace period.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants