Updating default value of max list items per call (#856)

* Updating default value of max list items per call * fixed integration tests * fixed integration tests * addressed review comments * fixed formatting * addressed review comments * Update CHANGES.md Co-authored-by: Igor Dvorzhak <[email protected]>
GoogleCloudDataproc · Aug 20, 2022 · 3826f97 · 3826f97
1 parent de53e52
commit 3826f97
Show file tree

Hide file tree

Showing 8 changed files with 128 additions and 166 deletions.
diff --git a/gcs/CHANGES.md b/gcs/CHANGES.md
@@ -72,6 +72,8 @@
     fs.gs.outputstream.type
     ```
 
+1.  Set default value for `fs.gs.list.max.items.per.call` property to `5000`.
+
 ### 2.2.2 - 2021-06-25
 
 1.  Support footer prefetch in gRPC read channel.

diff --git a/gcs/CONFIGURATION.md b/gcs/CONFIGURATION.md
@@ -102,7 +102,7 @@
 
     Maximum number of threads used to execute batch requests in parallel.
 
-*   `fs.gs.list.max.items.per.call` (default: `1024`)
+*   `fs.gs.list.max.items.per.call` (default: `5000`)
 
     Maximum number of items to return in response for list Cloud Storage
     requests.

diff --git a/...src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfigurationTest.java b/...src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfigurationTest.java
@@ -102,7 +102,7 @@ public class GoogleHadoopFileSystemConfigurationTest {
           put("fs.gs.inputstream.support.gzip.encoding.enable", false);
           put("fs.gs.io.buffersize.write", 64 * 1024 * 1024);
           put("fs.gs.lazy.init.enable", false);
-          put("fs.gs.list.max.items.per.call", 1024L);
+          put("fs.gs.list.max.items.per.call", 5000L);
           put("fs.gs.marker.file.pattern", null);
           put("fs.gs.max.requests.per.batch", 15L);
           put("fs.gs.max.wait.for.empty.object.creation.ms", 3_000);

diff --git a/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageOptions.java b/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageOptions.java
@@ -66,7 +66,7 @@ public abstract class GoogleCloudStorageOptions {
   public static final int MAX_WAIT_MILLIS_FOR_EMPTY_OBJECT_CREATION = 3_000;
 
   /** Default number of items to return per call to the list* GCS RPCs. */
-  public static final long MAX_LIST_ITEMS_PER_CALL_DEFAULT = 1024;
+  public static final long MAX_LIST_ITEMS_PER_CALL_DEFAULT = 5000;
 
   /** Default setting for maximum number of requests per GCS batch. */
   public static final long MAX_REQUESTS_PER_BATCH_DEFAULT = 15;

diff --git a/...st/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageFileSystemNewIntegrationTest.java b/...st/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageFileSystemNewIntegrationTest.java
@@ -590,8 +590,7 @@ public void listFileInfo_file() throws Exception {
     assertThat(gcsRequestsTracker.getAllRequestStrings())
         .containsExactly(
             getRequestString(bucketName, fileObject),
-            listRequestWithTrailingDelimiter(
-                bucketName, fileObject + "/", /* maxResults= */ 1024, /* pageToken= */ null));
+            listRequestWithTrailingDelimiter(bucketName, fileObject + "/", /* pageToken= */ null));
 
     assertThat(fileInfos).hasSize(1);
     FileInfo fileInfo = fileInfos.get(0);
@@ -616,8 +615,7 @@ public void listFileInfo_file_directoryPath() throws Exception {
 
     assertThat(gcsRequestsTracker.getAllRequestStrings())
         .containsExactly(
-            listRequestWithTrailingDelimiter(
-                bucketName, fileObject + "/", /* maxResults= */ 1024, /* pageToken= */ null));
+            listRequestWithTrailingDelimiter(bucketName, fileObject + "/", /* pageToken= */ null));
   }
 
   @Test
@@ -637,8 +635,7 @@ public void listFileInfo_directory() throws Exception {
     assertThat(gcsRequestsTracker.getAllRequestStrings())
         .containsExactly(
             getRequestString(bucketName, dirObject),
-            listRequestWithTrailingDelimiter(
-                bucketName, dirObject + "/", /* maxResults= */ 1024, /* pageToken= */ null));
+            listRequestWithTrailingDelimiter(bucketName, dirObject + "/", /* pageToken= */ null));
 
     assertThat(fileInfos.stream().map(FileInfo::exists).collect(toList()))
         .containsExactly(true, true);
@@ -686,8 +683,7 @@ public void listFileInfo_directory_directoryPath() throws Exception {
 
     assertThat(gcsRequestsTracker.getAllRequestStrings())
         .containsExactly(
-            listRequestWithTrailingDelimiter(
-                bucketName, dirObject + "/", /* maxResults= */ 1024, /* pageToken= */ null));
+            listRequestWithTrailingDelimiter(bucketName, dirObject + "/", /* pageToken= */ null));
 
     assertThat(fileInfos.stream().map(FileInfo::exists).collect(toList()))
         .containsExactly(true, true);
@@ -713,8 +709,7 @@ public void listFileInfo_implicitDirectory() throws Exception {
     assertThat(gcsRequestsTracker.getAllRequestStrings())
         .containsExactly(
             getRequestString(bucketName, dirObject),
-            listRequestWithTrailingDelimiter(
-                bucketName, dirObject + "/", /* maxResults= */ 1024, /* pageToken= */ null));
+            listRequestWithTrailingDelimiter(bucketName, dirObject + "/", /* pageToken= */ null));
 
     assertThat(fileInfos.stream().map(FileInfo::exists).collect(toList())).containsExactly(true);
     assertThat(fileInfos.stream().map(FileInfo::getPath).collect(toList()))
@@ -738,8 +733,7 @@ public void listFileInfo_emptyDirectoryObject() throws Exception {
     assertThat(gcsRequestsTracker.getAllRequestStrings())
         .containsExactly(
             getRequestString(bucketName, dirObject),
-            listRequestWithTrailingDelimiter(
-                bucketName, dirObject + "/", /* maxResults= */ 1024, /* pageToken= */ null));
+            listRequestWithTrailingDelimiter(bucketName, dirObject + "/", /* pageToken= */ null));
 
     assertThat(fileInfos).isEmpty();
   }
@@ -760,8 +754,7 @@ public void listFileInfo_notFound() throws Exception {
     assertThat(gcsRequestsTracker.getAllRequestStrings())
         .containsExactly(
             getRequestString(bucketName, dirObject),
-            listRequestWithTrailingDelimiter(
-                bucketName, dirObject + "/", /* maxResults= */ 1024, /* pageToken= */ null));
+            listRequestWithTrailingDelimiter(bucketName, dirObject + "/", /* pageToken= */ null));
   }
 
   @Test
@@ -805,7 +798,7 @@ public void listFileInfo_customFields_required() throws Exception {
                 bucketName,
                 dirObject + "/",
                 /* objectFields= */ "bucket,name",
-                /* maxResults= */ 1024,
+
                 /* pageToken= */ null));
   }
 
@@ -850,7 +843,7 @@ public void listFileInfo_customFields_some() throws Exception {
                 bucketName,
                 dirObject + "/",
                 /* objectFields= */ "bucket,name,contentType",
-                /* maxResults= */ 1024,
+
                 /* pageToken= */ null));
   }
 
@@ -880,11 +873,7 @@ public void listFileInfo_customFields_fails() throws Exception {
         .containsExactly(
             getRequestString(bucketName, dirObject),
             listRequestWithTrailingDelimiter(
-                bucketName,
-                dirObject + "/",
-                /* objectFields= */ "bucket",
-                /* maxResults= */ 1024,
-                /* pageToken= */ null));
+                bucketName, dirObject + "/", /* objectFields= */ "bucket", /* pageToken= */ null));
   }
 
   @Test
@@ -975,13 +964,7 @@ private void delete_directory(boolean parallelStatus) throws Exception {
             listRequestWithTrailingDelimiter(
                 bucketName, dirObject + "/d1/", /* maxResults= */ 1, /* pageToken= */ null),
             listRequestString(
-                bucketName,
-                /* flatList= */ true,
-                /* includeTrailingDelimiter= */ null,
-                dirObject + "/d1/",
-                "bucket,name,generation",
-                /* maxResults= */ 1024,
-                /* pageToken= */ null),
+                bucketName, true, null, dirObject + "/d1/", "bucket,name,generation", null),
             deleteRequestString(bucketName, dirObject + "/d1/", /* generationId= */ 1));
 
     assertThat(gcsFs.exists(bucketUri.resolve(dirObject + "/d1"))).isFalse();
@@ -1015,7 +998,7 @@ public void delete_inferredDirectory() throws Exception {
                 /* includeTrailingDelimiter= */ null,
                 dirObject + "/d1/",
                 "bucket,name,generation",
-                /* maxResults= */ 1024,
+
                 /* pageToken= */ null),
             deleteRequestString(bucketName, dirObject + "/d1/f1", /* generationId= */ 1),
             getRequestString(bucketName, dirObject + "/"));
@@ -1190,7 +1173,7 @@ public void rename_directory_sequential() throws Exception {
                 /* includeTrailingDelimiter= */ null,
                 dirObject + "/srcParent/srcDir/",
                 "bucket,name,generation",
-                /* maxResults= */ 1024,
+
                 /* pageToken= */ null),
             // Copy file
             copyRequestString(
@@ -1259,7 +1242,7 @@ public void rename_directory_parallel() throws Exception {
                 /* includeTrailingDelimiter= */ null,
                 dirObject + "/srcParent/srcDir/",
                 "bucket,name,generation",
-                /* maxResults= */ 1024,
+
                 /* pageToken= */ null),
             // Copy file
             copyRequestString(