Do not parallelize GCS list requests, because it leads to too high QP…

…S (HTTP 429 reponse). Fixes #151
GoogleCloudDataproc · Feb 21, 2019 · f4b5ec9 · f4b5ec9
1 parent 88cb250
commit f4b5ec9
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 19 deletions.
diff --git a/gcs/CHANGES.txt b/gcs/CHANGES.txt
@@ -4,6 +4,8 @@
 
   2. Do not convert path to directory path for inferred implicit directories.
 
+  3. Do not parallelize GCS list requests, because it leads to too high QPS.
+
 
 1.9.14 - 2019-02-13
 

diff --git a/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageFileSystem.java b/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageFileSystem.java
@@ -1045,17 +1045,10 @@ public List<FileInfo> listFileInfo(URI path, boolean enableAutoRepair)
         pathCodec.validatePathAndGetId(FileInfo.convertToDirectoryPath(pathCodec, path), true);
 
     // To improve performance start to list directory items right away.
-    ExecutorService dirExecutor = Executors.newFixedThreadPool(2, DAEMON_THREAD_FACTORY);
+    ExecutorService dirExecutor = Executors.newSingleThreadExecutor(DAEMON_THREAD_FACTORY);
     try {
       Future<GoogleCloudStorageItemInfo> dirFuture =
           dirExecutor.submit(() -> gcs.getItemInfo(dirId));
-      Future<List<GoogleCloudStorageItemInfo>> dirChildrenFutures =
-          dirExecutor.submit(
-              () ->
-                  dirId.isRoot()
-                      ? gcs.listBucketInfo()
-                      : gcs.listObjectInfo(
-                          dirId.getBucketName(), dirId.getObjectName(), PATH_DELIMITER));
       dirExecutor.shutdown();
 
       if (!pathId.isDirectory()) {
@@ -1069,7 +1062,10 @@ public List<FileInfo> listFileInfo(URI path, boolean enableAutoRepair)
 
       try {
         GoogleCloudStorageItemInfo dirInfo = dirFuture.get();
-        List<GoogleCloudStorageItemInfo> dirItemInfos = dirChildrenFutures.get();
+        List<GoogleCloudStorageItemInfo> dirItemInfos =
+            dirId.isRoot()
+                ? gcs.listBucketInfo()
+                : gcs.listObjectInfo(dirId.getBucketName(), dirId.getObjectName(), PATH_DELIMITER);
         if (!dirInfo.exists() && dirItemInfos.isEmpty()) {
           throw new FileNotFoundException("Item not found: " + path);
         }
@@ -1123,16 +1119,8 @@ private GoogleCloudStorageItemInfo getFileInfoInternal(StorageResourceId resourc
     }
     StorageResourceId dirId = FileInfo.convertToDirectoryPath(resourceId);
     // To improve performance get directory and its child right away.
-    ExecutorService dirExecutor =
-        resourceId.isDirectory()
-            ? Executors.newSingleThreadExecutor(DAEMON_THREAD_FACTORY)
-            : Executors.newFixedThreadPool(2, DAEMON_THREAD_FACTORY);
+    ExecutorService dirExecutor = Executors.newSingleThreadExecutor(DAEMON_THREAD_FACTORY);
     try {
-      Future<List<String>> dirChildFuture =
-          dirExecutor.submit(
-              () ->
-                  gcs.listObjectNames(
-                      dirId.getBucketName(), dirId.getObjectName(), PATH_DELIMITER, 1));
       Future<GoogleCloudStorageItemInfo> dirFuture =
           resourceId.isDirectory()
               ? Futures.immediateFuture(gcs.getItemInfo(resourceId))
@@ -1152,7 +1140,9 @@ private GoogleCloudStorageItemInfo getFileInfoInternal(StorageResourceId resourc
           return dirInfo;
         }
 
-        if (dirChildFuture.get().isEmpty()) {
+        List<String> dirChild =
+            gcs.listObjectNames(dirId.getBucketName(), dirId.getObjectName(), PATH_DELIMITER, 1);
+        if (dirChild.isEmpty()) {
           return GoogleCloudStorageItemInfo.createNotFound(resourceId);
         }