XiaoHongbo-Hope commented on code in PR #9821:
URL: https://github.com/apache/paimon/pull/9821#discussion_r4013705510
##########
paimon-python/pypaimon/filesystem/pyarrow_file_io.py:
##########
@@ -465,6 +497,34 @@ def delete(self, path: str, recursive: bool = False) ->
bool:
self.filesystem.delete_file(path_str)
return True
+ def _delete_s3_compatible_directory(self, path_str: str) -> bool:
+ selector = pafs.FileSelector(
+ path_str, recursive=True, allow_not_found=True)
+ file_infos = self.filesystem.get_file_info(selector)
Review Comment:
> **Scalability concern: this fallback is substantially more expensive and
applies to every custom S3 endpoint.**
>
> With PyArrow 23, each `delete_file()` performs HEAD + DELETE + a PUT to
preserve the parent directory. In a local HTTP request-count comparison for
1,000 files under one directory, the PyArrow 21 native path issued 10 requests,
while this path issued 3,010: 1,003 HEADs, 4 GETs, 1,002 DELETEs, and 1,001
PUTs. These are request counts from a local test server, not production
throughput measurements.
>
> Also, `get_file_info(selector)` materializes the entire tree, and
`executor.map()` eagerly submits the files on the tested Python version.
Limiting workers to 16 does not bound the number of queued futures.
>
> Could we provide a way to retain native batch deletion for endpoints that
support it, and bound listing/submission memory for the fallback?
`_uses_s3_compatibility()` currently includes all explicit S3 endpoints, even
services that support the newer requests. This is separate from the two
correctness issues above, but the cost should be considered before enabling it
broadly.
Thanks, fixed
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
