Avoid throttling: S3FileSystem::CreateDir should check the existence first rather than creating the parent path all the time

[HaochengLIU]

Describe the enhancement requested

Hi, I have a use case that thousands of jobs are writing hive partitioned parquet files daily to the same bucket via S3FS filesystem. Note each job may generate from single digit to a few thousand parquet files depending on the volume from its data source.
After abstraction, these jobs follow regex path patterns like s3fs://my-S3-bucket/<vendor-name>/<fruit-type>/<color>/<origination>/<creation_date>/<data-center-location>/date=YYYY-MM-DD/.... The gist here is a lot of keys are being created at the same time hense jobs hits AWS Error SLOW_DOWN. during Put Object operation: The object exceeded the rate limit for object mutation operations(create, update, and delete). Please reduce your rate request error. frequently throughout the day.

After investigation i realize they are creating too many objects in S3FileSystem::CreateDir(..) function one by one. My local experiments show that if my implementation avoids/checks the existence of the path first then call impl_->CreateEmptyDir(...) only when necessary, it addresses the issue in my production environment.

(I understand various cloud vendors have various IO limits on a single bucket, in order to completely fix the the issue is another story to my daily work)

I'm proposing a code change like below. Hi @pitrou I see you are the main author of s3fs.cc, can you pls share your insights when you have time?
Also even with a vanilla build S3FS test fails quite a few on my mac... can you guide how to make them run successfully..?

Many thanks.

diff --git a/cpp/src/arrow/filesystem/s3fs.cc b/cpp/src/arrow/filesystem/s3fs.cc
index 640888e1c..782d5f75d 100644
--- a/cpp/src/arrow/filesystem/s3fs.cc
+++ b/cpp/src/arrow/filesystem/s3fs.cc
@@ -2871,7 +2871,10 @@ Status S3FileSystem::CreateDir(const std::string& s, bool recursive) {
     for (const auto& part : path.key_parts) {
       parent_key += part;
       parent_key += kSep;
-      RETURN_NOT_OK(impl_->CreateEmptyDir(path.bucket, parent_key));
+      ARROW_ASSIGN_OR_RAISE(FileInfo parent_key_info, this->GetFileInfo(parent_key));
+      if (parent_key_info.type() == FileType::NotFound) {
+        RETURN_NOT_OK(impl_->CreateEmptyDir(path.bucket, parent_key));
+      }
     }
     return Status::OK();
   } else {

TestS3FS.CreateDir even fails with a Clean build :sigh

➜  build ninja && ./debug/arrow-s3fs-test --gtest_filter="TestS3FS.CreateDir"
ninja: no work to do.
Running main() from /Users/haochengliu/Documents/projects/Arrow/build/_deps/googletest-src/googletest/src/gtest_main.cc
Note: Google Test filter = TestS3FS.CreateDir
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from TestS3FS
[ RUN      ] TestS3FS.CreateDir
/Users/haochengliu/Documents/projects/Arrow/arrow/cpp/src/arrow/filesystem/s3fs_test.cc:934: Failure
Failed
Expected 'fs_->CreateDir("bucket/somefile")' to fail with IOError, but got OK
[  FAILED  ] TestS3FS.CreateDir (219 ms)
[----------] 1 test from TestS3FS (219 ms total)

Component(s)

C++

[MattShrider]

We're running into a similar issue in R. arrow::write_dataset(data, path = s3bucket$path('/foo/bar/baz')) attempts to create /foo and /foo/bar even if those directories already exist. In our setup we only allow writing directly into /bar or /baz, so we get an unnecessary permission error when it tries to write /foo.

[HaochengLIU]

Hi @westonpace @pitrou when you have time can you guide how to make S3FS tests pass in a vanilla test build? Once I'm unblocked happy to propose a fix and ask for review.

[westonpace]

Hmm...I think these tests use minio? Have you setup minio? I wonder if the issue might have something to do with minio version?

[HaochengLIU]

Hi Weston!
I just used brew to install minio (stable 20240510014138 (bottled), HEAD) and launched it via brew services start minio but same deal with the above test failure... Humm, any other ideas?
My personal laptop is a mac, one thing I can try is to use my work linux host with that test tomorrow

[HaochengLIU]

Another thing i could do this weekend is to create the PR first and see how it goes on github

[pitrou]

My personal laptop is a mac, one thing I can try is to use my work linux host with that test tomorrow

Yes, and you can also use docker to easily reproduce the CI configurations:
https://arrow.apache.org/docs/developers/continuous_integration/docker.html

Another thing i could do this weekend is to create the PR first and see how it goes on github

You can do that as well, though iterating will be more painful for you than if you can recreate the test setup locally.

[pitrou]

As to the issue itself: the reason CreateDir was implemented like this is that we're assuming that, usually, the directory doesn't exist already. So it's an optimization to try directory creation and catch the error, rather than issue two dependent IOs.

If, OTOH, there are situations where the directory exists most of the time, then this becomes a pessimization (at least for S3 with its accounting rules).

Perhaps CreateDir can take an optional argument, or perhaps this can be an extra option inside S3Options.

[HaochengLIU]

I tried few things and here is my latest finding:

• A VM running Ubuntu 22.04 on my mac.
• My working desktop which runs a modified version of Fedora 36.
  Both "passes" when minio is not installed

[3/3] Completed 'absl_ep'
Running main() from /home/lbting/software/arrow/build/_deps/googletest-src/googletest/src/gtest_main.cc
Note: Google Test filter = TestS3FS.CreateDir
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from TestS3FS
[ RUN      ] TestS3FS.CreateDir
/home/lbting/software/arrow/arrow/cpp/src/arrow/filesystem/s3fs_test.cc:186: Failure
Failed
'InitServerAndClient()' failed with IOError: Failed to find minio executable ('minio') in PATH
/home/lbting/software/arrow/arrow/cpp/src/arrow/filesystem/s3_test_util.cc:168  server->Start()
/home/lbting/software/arrow/arrow/cpp/src/arrow/filesystem/s3fs_test.cc:203  GetMinioEnv()->GetOneServer()
[1]    41608 segmentation fault (core dumped)  ./debug/arrow-s3fs-test --gtest_filter="TestS3FS.CreateDir"

Both failed with the presence of minio...

➜  build ninja && ./debug/arrow-s3fs-test --gtest_filter="TestS3FS.CreateDir"                                           
[3/3] Completed 'absl_ep'
Running main() from /home/lbting/software/arrow/build/_deps/googletest-src/googletest/src/gtest_main.cc
Note: Google Test filter = TestS3FS.CreateDir
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from TestS3FS
[ RUN      ] TestS3FS.CreateDir
/home/lbting/software/arrow/arrow/cpp/src/arrow/filesystem/s3fs_test.cc:934: Failure
Failed
Expected 'fs_->CreateDir("bucket/somefile")' to fail with IOError, but got OK
[  FAILED  ] TestS3FS.CreateDir (497 ms)
[----------] 1 test from TestS3FS (497 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (524 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] TestS3FS.CreateDir

 1 FAILED TEST

Which makes me wonder if minio is ever used in Arrow testing pipelines?

Anyway, I will try to create a PR with an extra options inside S3Options very soon