FrankChen021 commented on code in PR #20327:
URL: https://github.com/apache/druid/pull/20327#discussion_r4015589040
##########
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentPusher.java:
##########
@@ -148,6 +166,40 @@ private DataSegment pushNoZip(File indexFilesDir,
DataSegment baseSegment, Strin
.withBinaryVersion(binaryVersion);
}
+ /**
+ * Removes everything under {@code s3Path} that this push did not write.
+ * <p>
+ * A zipped push replaces the previous segment outright, because one {@code
index.zip} object overwrites another, and
+ * {@link #push} with {@code useUniquePath = false} is expected to replace a
previous push the same way. Uploading
+ * file by file only overwrites the names the new segment happens to share,
so without this a re-push could leave
+ * behind objects of whatever was there before: a stale {@code index.zip}
from a zipped push, or smoosh chunks from a
+ * larger prior v9 segment.
+ * <p>
+ * Note that this makes an unzipped push require permission to delete
objects under the segment path, which a zipped
+ * push does not.
+ */
+ private void deleteStaleObjects(final String s3Path, final Set<String>
pushedKeys) throws IOException
+ {
+ try {
+ S3Utils.deleteObjectsInPath(
+ s3Client,
+ config.getMaxListingLength(),
Review Comment:
[P1] S3 cleanup uses an invalid default batch size
**Finding:** The new stale-object cleanup passes S3DataSegmentPusherConfig's
maxListingLength directly to S3Utils.deleteObjectsInPath. That config defaults
to 1024 and only enforces a lower bound, while this helper uses the value for
both ListObjectsV2 maxKeys and DeleteObjects batches; AWS S3 permits at most
1000 keys for either request. Consequently, a default-configured
druid.storage.zip=false push can upload the files and then fail cleanup before
returning its DataSegment.
**Suggestion:** Cap the cleanup/listing size at 1000 or apply the same
upper-bound validation and default used by S3InputDataConfig, and add a
regression test with the default pusher configuration.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
