[GitHub] [arrow] pitrou commented on a change in pull request #11890: ARROW-14916: [C++] GcsFileSystem can delete directories

[GitBox <gi...@apache.org>]

pitrou commented on a change in pull request #11890:
URL: https://github.com/apache/arrow/pull/11890#discussion_r765034206



##########
File path: cpp/src/arrow/filesystem/gcsfs.cc
##########
@@ -342,6 +345,76 @@ class GcsFileSystem::Impl {
     return internal::ToArrowStatus(CreateDirMarkerRecursive(p.bucket, p.object));
   }
 
+  Status DeleteDir(const GcsPath& p) {
+    RETURN_NOT_OK(DeleteDirContents(p));
+    if (!p.object.empty()) {
+      return internal::ToArrowStatus(client_.DeleteObject(p.bucket, p.object));
+    }
+    return internal::ToArrowStatus(client_.DeleteBucket(p.bucket));
+  }
+
+  Status DeleteDirContents(const GcsPath& p) {
+    // Deleting large directories can be fairly slow, we need to parallelize the
+    // operation. This uses `std::async()` to run multiple delete operations in parallel.

Review comment:
       Instead of using `std::async`, we should the filesystem's IOContext, which has an executor (the default being a global IO thread pool):
   https://github.com/apache/arrow/blob/master/cpp/src/arrow/io/interfaces.h#L52-L60
   
   The executor in turn allows you to spawn tasks and wait on them with futures (not `std::future`, but our own `Future` which supports more operations, such as waiting for multiple futures at once). You can see examples in the thread pool tests: https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/thread_pool_test.cc#L536
   
   See here for waiting on multiple futures:
   https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/future.h#L835-L876
   
   I think this could actually simplify this code quite a bit.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org