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.

Sign up for GitHub

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

Jump to bottom

storageccl: lots of cgo calls in Backup #18884

Closed
maddyblue opened this issue Sep 28, 2017 · 2 comments
Closed

storageccl: lots of cgo calls in Backup #18884

maddyblue opened this issue Sep 28, 2017 · 2 comments
Labels
A-disaster-recovery C-performance Perf of queries or internals. Solution not expected to change functional behavior.

Comments

@maddyblue
Copy link
Contributor

cockroach/pkg/ccl/storageccl/engineccl/mvcc.go

Line 44 in 14c5215

// TODO(dan): Move all this logic into c++ and make this a thin wrapper.

cockroach/pkg/ccl/storageccl/export.go

Line 144 in 14c5215

// TODO(dan): Move all this iteration into cpp to avoid the cgo calls.

This should reduce p99 by a lot.
@maddyblue maddyblue added this to the 1.2 milestone Sep 28, 2017
@dt
Copy link
Member

dt commented Oct 16, 2017

likely cause of #16276

@dt dt modified the milestones: 2.0, 2.1 Jan 18, 2018
@maddyblue maddyblue added the C-performance Perf of queries or internals. Solution not expected to change functional behavior. label Apr 26, 2018
@petermattis petermattis removed this from the 2.1 milestone Oct 5, 2018
@adityamaru27 adityamaru27 mentioned this issue May 13, 2019
Merged
adityamaru27 added a commit to adityamaru27/cockroach that referenced this issue May 15, 2019
@adityamaru27
engine: Add ExportToSst method with all logic moved to C++
d3de2b0 
Export was previously executed using an MVCCIncrementalIterator
where the logic handling iteration over the diff of the key
range [startKey, endKey) and time range (startTime, endTime]
was written in Go. This iteration involved making a cgo call
to find every key, along with another cgo call for writing
each key to the sstable.
This migration resolves the aforementioned performance
bottleneck (cockroachdb#18884), by pushing all the required logic to C++
and exposing a single export method.

Release note: Performance improvement to the BACKUP process.
adityamaru27 added a commit to adityamaru27/cockroach that referenced this issue May 16, 2019
@adityamaru27
engine: Add ExportToSst method with all logic moved to C++
d8df6cd 
Export was previously executed using an MVCCIncrementalIterator
where the logic handling iteration over the diff of the key
range [startKey, endKey) and time range (startTime, endTime]
was written in Go. This iteration involved making a cgo call
to find every key, along with another cgo call for writing
each key to the sstable.
This migration resolves the aforementioned performance
bottleneck (cockroachdb#18884), by pushing all the required logic to C++
and exposing a single export method.

Release note: Performance improvement to the BACKUP process.
adityamaru27 added a commit to adityamaru27/cockroach that referenced this issue May 17, 2019
@adityamaru27
engine: Add ExportToSst method with all logic moved to C++
93b1fdf 
Export was previously executed using an MVCCIncrementalIterator
where the logic handling iteration over the diff of the key
range [startKey, endKey) and time range (startTime, endTime]
was written in Go. This iteration involved making a cgo call
to find every key, along with another cgo call for writing
each key to the sstable.
This migration resolves the aforementioned performance
bottleneck (cockroachdb#18884), by pushing all the required logic to C++
and exposing a single export method.

Release note: Performance improvement to the BACKUP process.
adityamaru27 added a commit to adityamaru27/cockroach that referenced this issue May 22, 2019
@adityamaru27
engine: Add ExportToSst method with all logic moved to C++
825d630 
Export was previously executed using an MVCCIncrementalIterator
where the logic handling iteration over the diff of the key
range [startKey, endKey) and time range (startTime, endTime]
was written in Go. This iteration involved making a cgo call
to find every key, along with another cgo call for writing
each key to the sstable.
This migration resolves the aforementioned performance
bottleneck (cockroachdb#18884), by pushing all the required logic to C++
and exposing a single export method.

Release note: Performance improvement to the BACKUP process.
adityamaru27 added a commit to adityamaru27/cockroach that referenced this issue May 22, 2019
@adityamaru27
engine: Add ExportToSst method with all logic moved to C++
8729a20 
Export was previously executed using an MVCCIncrementalIterator
where the logic handling iteration over the diff of the key
range [startKey, endKey) and time range (startTime, endTime]
was written in Go. This iteration involved making a cgo call
to find every key, along with another cgo call for writing
each key to the sstable.
This migration resolves the aforementioned performance
bottleneck (cockroachdb#18884), by pushing all the required logic to C++
and exposing a single export method.

Release note: Performance improvement to the BACKUP process.
adityamaru27 added a commit to adityamaru27/cockroach that referenced this issue Jun 4, 2019
@adityamaru27
engine: Add ExportToSst method with all logic moved to C++
75f1314 
Export was previously executed using an MVCCIncrementalIterator
where the logic handling iteration over the diff of the key
range [startKey, endKey) and time range (startTime, endTime]
was written in Go. This iteration involved making a cgo call
to find every key, along with another cgo call for writing
each key to the sstable.
This migration resolves the aforementioned performance
bottleneck (cockroachdb#18884), by pushing all the required logic to C++
and exposing a single export method.

Release note: Performance improvement to the BACKUP process.
craig bot pushed a commit that referenced this issue Jun 4, 2019
@adityamaru27
Merge #37496
9521979 
37496: libroach: Migrate IncrementalIterator logic to C++ r=dt a=adityamaru27

Export was previously executed using an `MVCCIncrementalIterator`
where the logic handling iteration over the diff of the key
range `[startKey, endKey)` and time range `(startTime, endTime]`
was written in Go. This iteration involved making a cgo call
to find every key, along with another cgo call for writing
each key to the sstable.
This migration resolves the aforementioned performance
bottleneck (#18884), by pushing all the required logic to C++
and exposing a single export method.

Based on a performance benchmark by running BACKUP on a tpcc database with 1000 warehouses we observe the following:

- Over an average of 3 runs we see a 1.1x improvement in time performance. While the original binary took ~32m04s the changed implementation took ~28m55s. This is due to the elimination of a cgo call per key.

Co-authored-by: Aditya Maru <[email protected]>
@rolandcrosby
Copy link

Fixed by #37496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-disaster-recovery C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dt @maddyblue @rolandcrosby @petermattis