parallel composite upload support #1067

singhravidutt · 2023-10-26T05:12:10Z

This PR adds the support for integrating with new write APIs introduced in java-storage to leverage ParallelCompositeUpload (PCU) feature.

singhravidutt · 2023-10-26T05:12:27Z

/gcbrun

codecov · 2023-10-26T05:20:20Z

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (48891bf) 81.71% compared to head (b21262b) 81.68%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1067      +/-   ##
============================================
- Coverage     81.71%   81.68%   -0.03%     
- Complexity     1669     1675       +6     
============================================
  Files            96       96              
  Lines          7275     7318      +43     
  Branches        890      890              
============================================
+ Hits           5945     5978      +33     
- Misses          969      975       +6     
- Partials        361      365       +4

Flag	Coverage Δ
integrationtest	`67.50% <82.35%> (+0.08%)`	⬆️
unittest	`73.23% <49.01%> (-0.15%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...op/fs/gcs/GoogleHadoopFileSystemConfiguration.java	`99.09% <100.00%> (+0.05%)`	⬆️
...le/cloud/hadoop/util/AsyncWriteChannelOptions.java	`86.36% <100.00%> (+3.50%)`	⬆️
...oud/hadoop/gcsio/GoogleCloudStorageClientImpl.java	`82.20% <65.38%> (-6.34%)`	⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfiguration.java

arunkumarchacko · 2023-10-26T05:19:51Z

gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfiguration.java

+   * alignment with configuration of java-storage client
+   * https://cloud.google.com/java/docs/reference/google-cloud-storage/latest/com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.BufferAllocationStrategy#com_google_cloud_storage_ParallelCompositeUploadBlobWriteSessionConfig_BufferAllocationStrategy_fixedPool_int_int_
+   */
+  public static final HadoopConfigurationProperty<Integer> GCS_PCU_BUFFER_COUNT =


update Configuration.md as well?

Once we have benchmarking numbers out then will be adding it to configurations file.

gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfiguration.java

arunkumarchacko · 2023-10-26T05:25:42Z

gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfiguration.java

@@ -588,6 +632,10 @@ private static AsyncWriteChannelOptions getWriteChannelOptions(Configuration con
        .setUploadType(GCS_CLIENT_UPLOAD_TYPE.get(config, config::getEnum))
        .setTemporaryPaths(
            ImmutableSet.copyOf(GCS_WRITE_TEMPORARY_FILES_PATH.getStringCollection(config)))
+        .setPCUBufferCapacity(GCS_PCU_BUFFER_COUNT.get(config, config::getInt))


Please add UTs.

gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfiguration.java

...a/com/google/cloud/hadoop/gcsio/integration/GoogleCloudStorageClientImplIntegrationTest.java

arunkumarchacko · 2023-10-26T05:52:04Z

...a/com/google/cloud/hadoop/gcsio/integration/GoogleCloudStorageClientImplIntegrationTest.java

+            .setWriteChannelOptions(
+                pcuDefaultOptions.toBuilder()
+                    .setPartFilePrefix(partFilePrefix)
+                    .setPartFileCleanupType(PartFileCleanupType.NEVER)


ON_SUCCESS is not tested?

add test for it.

gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfigurationTest.java

arunkumarchacko · 2023-10-26T05:55:10Z

...a/com/google/cloud/hadoop/gcsio/integration/GoogleCloudStorageClientImplIntegrationTest.java

+    writeChannel.write(ByteBuffer.wrap(bytesToWrite));
+    // part files are getting uploaded in async thread
+    // wait for it to complete before listing files
+    Thread.sleep(5000);


Please change this.

arunkumarchacko · 2023-10-26T12:39:49Z

/gcbrun

arunkumarchacko · 2023-10-26T12:36:10Z

gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientImpl.java

+        return PartCleanupStrategy.never();
+      case ON_SUCCESS:
+        return PartCleanupStrategy.onlyOnSuccess();
+      default:


https://google.github.io/styleguide/javaguide.html#s4.8.4-switch

It is recommended to explicitly list the values, rather than having default. This is more future proof.

arunkumarchacko · 2023-10-26T12:41:37Z

gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientImpl.java

@@ -251,12 +257,43 @@ private static BlobWriteSessionConfig getSessionConfig(AsyncWriteChannelOptions
            writeOptions.getTemporaryPaths().stream()
                .map(x -> Paths.get(x))
                .collect(ImmutableSet.toImmutableSet()));
+      case PARALLEL_COMPOSITE_UPLOAD:
+        ExecutorService pcuThreadPool =


this is a bit weird. If that is the case, why can't it create it?

arunkumarchacko · 2023-10-26T12:41:59Z

gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientImpl.java

@@ -251,12 +257,43 @@ private static BlobWriteSessionConfig getSessionConfig(AsyncWriteChannelOptions
            writeOptions.getTemporaryPaths().stream()
                .map(x -> Paths.get(x))
                .collect(ImmutableSet.toImmutableSet()));
+      case PARALLEL_COMPOSITE_UPLOAD:
+        ExecutorService pcuThreadPool =


Either case, add a comment. It is not obvious.

arunkumarchacko · 2023-10-26T12:42:31Z

gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientImpl.java

+                BufferAllocationStrategy.fixedPool(
+                    writeOptions.getPCUBufferCount(), writeOptions.getPCUBufferCapacity()))
+            .withPartCleanupStrategy(getPartCleanupStrategy(writeOptions.getPartFileCleanupType()))
+            .withExecutorSupplier(ExecutorSupplier.useExecutor(pcuThreadPool))


why not go with the default option?

arunkumarchacko · 2023-10-26T12:43:20Z

util/src/main/java/com/google/cloud/hadoop/util/AsyncWriteChannelOptions.java

  }

+  private static final int PARALLEL_COMPOSITE_UPLOAD_BUFFER_COUNT = 1;


Please add a comment with TODO.

arunkumarchacko · 2023-10-26T12:44:14Z

...a/com/google/cloud/hadoop/gcsio/integration/GoogleCloudStorageClientImplIntegrationTest.java

+                    .build())
+            .build();
+
+    gcs = getGCSImpl(storageOptions);


Old test does not close does not mean that we keep on repeating the mistake, I guess.

arunkumarchacko · 2023-10-27T03:49:57Z

...a/com/google/cloud/hadoop/gcsio/integration/GoogleCloudStorageClientImplIntegrationTest.java

  private static GoogleCloudStorage helperGcs;

  private GoogleCloudStorage gcs;

+  private int partFileCount = 2;


arunkumarchacko · 2023-10-27T03:50:04Z

...a/com/google/cloud/hadoop/gcsio/integration/GoogleCloudStorageClientImplIntegrationTest.java

  private static GoogleCloudStorage helperGcs;

  private GoogleCloudStorage gcs;

+  private int partFileCount = 2;
+  private int bufferCapacity = partFileCount * ONE_MiB;


parallel composite support

3cae3be

singhravidutt requested a review from arunkumarchacko October 26, 2023 05:12

singhravidutt marked this pull request as ready for review October 26, 2023 05:12

arunkumarchacko reviewed Oct 26, 2023

View reviewed changes

arunkumarchacko requested changes Oct 26, 2023

View reviewed changes

singhravidutt added 2 commits October 26, 2023 16:51

addressed review comment

cd7e815

fixed unit test

ba3a930

Merge branch 'master' into pcu

b87d08e

arunkumarchacko reviewed Oct 27, 2023

View reviewed changes

addressed review comment

b21262b

arunkumarchacko approved these changes Oct 27, 2023

View reviewed changes

singhravidutt merged commit e081924 into master Oct 27, 2023
3 of 4 checks passed

singhravidutt deleted the pcu branch October 27, 2023 07:45

singhravidutt added a commit that referenced this pull request Jul 8, 2024

parallel composite upload support (#1067)

e5fcac8

singhravidutt added a commit that referenced this pull request Jul 9, 2024

parallel composite upload support (#1067) (#1212)

143205f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parallel composite upload support #1067

parallel composite upload support #1067

singhravidutt commented Oct 26, 2023 •

edited

Loading

singhravidutt commented Oct 26, 2023

codecov bot commented Oct 26, 2023 •

edited

Loading

arunkumarchacko Oct 26, 2023

singhravidutt Oct 26, 2023

arunkumarchacko Oct 26, 2023

singhravidutt Oct 26, 2023

arunkumarchacko Oct 26, 2023

singhravidutt Oct 26, 2023

arunkumarchacko Oct 26, 2023

singhravidutt Oct 26, 2023

arunkumarchacko commented Oct 26, 2023

arunkumarchacko Oct 26, 2023

arunkumarchacko Oct 26, 2023

arunkumarchacko Oct 26, 2023

arunkumarchacko Oct 26, 2023

arunkumarchacko Oct 26, 2023

arunkumarchacko Oct 26, 2023

arunkumarchacko Oct 27, 2023

singhravidutt Oct 27, 2023

arunkumarchacko Oct 27, 2023

singhravidutt Oct 27, 2023

		}

		private static final int PARALLEL_COMPOSITE_UPLOAD_BUFFER_COUNT = 1;

parallel composite upload support #1067

parallel composite upload support #1067

Conversation

singhravidutt commented Oct 26, 2023 • edited Loading

singhravidutt commented Oct 26, 2023

codecov bot commented Oct 26, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arunkumarchacko commented Oct 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

singhravidutt commented Oct 26, 2023 •

edited

Loading

codecov bot commented Oct 26, 2023 •

edited

Loading