gaborgsomogyi opened a new pull request, #29129:
URL: https://github.com/apache/flink/pull/29129
## What is the purpose of the change
`NativeS3FileSystem.delete(Path, boolean recursive)` deleted a directory by
listing its immediate children and recursing into `delete()` for each one,
issuing one `DeleteObject` S3 API call per file. This mirrors the performance
problem that Hadoop's S3A connector solved years ago with multi-object delete
(`fs.s3a.multiobjectdelete.enable`): every object under a prefix costs its own
network round trip.
This pull request replaces the recursive single-object delete loop with a
flat listing of all keys under the prefix followed by batched `DeleteObjects`
calls (up to 1000 keys per call, the S3 API limit), cutting the number of S3
requests needed to delete a directory with many objects.
A global config flag and a matching per-bucket override are added so
batching can be disabled for S3-compatible stores that do not support the
multi-object delete API, or for buckets/policies that reject it, matching the
escape hatch S3A already provides.
## Brief change log
- `NativeS3FileSystem.delete()` no longer recurses into itself per child;
it now calls a new `deleteRecursively()` helper
- `deleteRecursively()` does a single flat (non-delimited) `ListObjectsV2`
paginated walk to collect every key under the prefix, then either:
- issues one `DeleteObjects` batch request per 1000 keys when batching
is enabled (default), or
- falls back to one `DeleteObject` request per key when disabled
- Added `s3.delete.batch.enabled` (default `true`) to
`NativeS3FileSystemFactory`
- Added a matching per-bucket override
`s3.bucket.<bucket-name>.delete.batch.enabled` via `S3BucketConfig` /
`BucketConfigProvider`
- Added INFO-level logging of the resolved batching setting at filesystem
creation, and DEBUG-level logging of which delete strategy and how many objects
are used per call
- Added `log4j2-test.properties` to `flink-s3-fs-native` (previously
missing), matching the template already used by `flink-s3-fs-hadoop`
## Verifying this change
This change added tests and can be verified as follows:
- `NativeS3FileSystemFactoryTest`: default value, global override, and
per-bucket-overrides-global tests for `s3.delete.batch.enabled`
- `BucketConfigProviderTest`: parsing and validation tests for the new
per-bucket `delete.batch.enabled` property, including an invalid-value
rejection test
- `NativeS3FileSystemITCase`:
`testRecursiveDeleteManyFilesWithBatchingEnabled` and
`testRecursiveDeleteManyFilesWithBatchingDisabled` write 25 files under a
prefix and assert they are all removed by a single recursive `delete()` call,
run against a real SeaweedFS S3-compatible container with both flag settings
- Manually verified via DEBUG logs against the SeaweedFS test container
that the enabled path issues a single `DeleteObjects` request for all objects
under a prefix, while the disabled path falls back to one `DeleteObject` call
per key
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: yes
## Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? JavaDocs and `ConfigOption`
descriptions (`s3.delete.batch.enabled`, per-bucket `delete.batch.enabled`)
--
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]
