Repository navigation
Parallelize per-file rewrites in GcsUtilV2 copy and move - #40376
Conversation
|
cc'ed @chamikaramj @damccorm This addressed the performance regression of rename op in GcsUtilV2 by parallelizing the rewrites. |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
assign set of reviewers |
|
Assigning reviewers: R: @ahmedabu98 for label java. This pull request likely touches a core component ("core" label). Please review with scrutiny. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
| private static final long MEGABYTES_COPIED_PER_CHUNK = 2048L; | ||
|
|
||
| /** Maximum number of files rewritten concurrently by {@link #copy} and {@link #move}. */ | ||
| @VisibleForTesting static final int MAX_CONCURRENT_REWRITES = 32; |
There was a problem hiding this comment.
maybe configurable via GcsOptions?
| if (failure == null) { | ||
| failure = cause; | ||
| for (Future<Void> other : futures) { | ||
| other.cancel(false); |
There was a problem hiding this comment.
We need to wait for the inflight rewrite to finish to limit the scope of partial results. With the current code, every file ends up either fully rewritten or never started, with none in an unknown state.
If we call cancel(true) to an on-going RewriteOne call, there is no guarantee that whether the file has already been copied, or in the move case, whether the original file has been removed or not.
There was a problem hiding this comment.
Revised the code to better handling shutdown on executor. Waiting to push the change but github remote server is not healthy right now.
- Make the number of concurrent copy/move rewrites in GcsUtilV2 configurable via GcsOptions (defaults to 32). - Deprecate gcsRewriteDataOpBatchLimit, which only applies to GcsUtilV1.
When one rewrite fails, files not yet started are cancelled, but the executor's shutdownNow() also interrupted rewrites already in flight, and the method could return while they were still running. Wait up to 5 minutes.
1b7b80b to
2614804
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40376 +/- ##
============================================
+ Coverage 58.65% 58.73% +0.07%
- Complexity 13843 13916 +73
============================================
Files 2579 2582 +3
Lines 270366 270550 +184
Branches 11212 11243 +31
============================================
+ Hits 158587 158900 +313
+ Misses 105747 105643 -104
+ Partials 6032 6007 -25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
GcsUtilV2.rewriteHelperpreviously processed files sequentially, making up to three dependent RPCs per file (target lookup, rewrite, and source delete). Unlike V1, which batches rewrites and runs batches concurrently, V2'sjava-storageclient does not support rewrite inStorageBatch, causingrenameto be ~6x slower than V1 in 64-shardParquetIOLTbenchmarks.GcsUtilV2: extract per-file logic intorewriteOneand execute multi-filecopy/movecalls concurrently on a bounded thread pool (up to 32 threads). Propagate the caller'sMetricsContainerto worker threads, cancel unstarted tasks on the first failure, and rethrow the original exception type (FileNotFoundException,FileAlreadyExistsException,AccessDeniedException, etc.) with subsequent failures attached as suppressed.GcsUtilV2Test: addtestV2RenameRewritesFilesConcurrentlyandtestV2RenameFailuresAreReportedPerFile.In the 64-shard
ParquetIOLT(f1_s1k) write benchmark on Dataflow, V2gcs_op_rename_msecdropped from ~20.6s to ~1.0s (vs ~2.3s–2.9s on V1) after this change.