backupccl: merge files returned to the processor during backup #65576
Conversation
|
This change is |
|
(tried to break this one up for easier commit-by-commit review) |
dt
force-pushed
the
stitch-returned
branch
4 times, most recently
from
f6d98e3 to
3ffbab9
Compare
dt
force-pushed
the
stitch-returned
branch
3 times, most recently
from
c71a76a to
0970220
Compare
|
|
||
| // If this span starts before the last buffered span ended, we need to flush | ||
| // since it overlaps but SSTWriter demands writes in-order. | ||
| // TODO(dt): consider buffering resp until _next_ `write` to allow minimal |
There was a problem hiding this comment.
It seems like the concurrency in the producer could make us miss many merging opportunities in the sstSink. This is also a problem that RESTORE faces with the introduced concurrency. I think this is different than what this TODO is saying; but I may have misunderstood.
I wonder if we could give the channel a bit more buffer to give a bit of a lead to the producer, then the sstWriter could pull off the SSTs and buffer them in a sorted data structure. This would certainly increase the memory overhead of this buffering, but I worry that we won't see much improvement in the merging otherwise.
Alternatively, maybe we can sort the buffered files by span in the sstSink during flush (they should all be non-overlapping) and merge them there rather than in write?
There was a problem hiding this comment.
That is what this TODO is about: right now if a later file gets to the sink before an earlier one, the sink is currently forced to flush the later, then add the earlier to its buffer. The todo is suggesting the sink buffer the received file before pushing its contents into the SST writer, so that if the next (or next n) files show up and are indeed earlier, we can flush them first instead of being forced to start a new file.
I just didn't want to do that in this change since it looks a little bit different after the direct-write-ssts change that is queued up next.
BACKUP writes row data to cloud storage in two ways:
1) the range holding the data can be instructed to export it into a
file which it writes to cloud storage directly or
2) the range holding the data can export it into a file which it
returns to the BACKUP process which then writes it to cloud storage.
When doing the second, we can be smarter than writing each file that is
returned to the SQL process immediately and individually, and instead
buffer and merge many of those files before writing the merged file to
the destination.
A future change could then build on this to opportunitically choose to
return files if they are smaller than some limit even in we were initially
operating in case (1), to make use of this merging and avoid creating
large numbers of small files.
Release note: none.
|
TFTR! |
|
bors r+ |
|
Build succeeded: |
backupccl: merge KV-returned SSTs during BACKUP
BACKUP writes row data to cloud storage in two ways:
1) the range holding the data can be instructed to export it into a
file which it writes to cloud storage directly or
2) the range holding the data can export it into a file which it
returns to the BACKUP process which then writes it to cloud storage.
When doing the second, we can be smarter than writing each file that is
returned to the SQL process immediately and individually, and instead
buffer and merge many of those files before writing the merged file to
the destination.
A future change could then build on this to opportunitically choose to
return files if they are smaller than some limit even in we were initially
operating in case (1), to make use of this merging and avoid creating
large numbers of small files.