FrankChen021 commented on code in PR #20327:
URL: https://github.com/apache/druid/pull/20327#discussion_r3989160557
##########
extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentPuller.java:
##########
@@ -96,4 +96,40 @@ FileUtils.FileCopyResult getSegmentFiles(
throw new SegmentLoadingException(e, e.getMessage());
}
}
+
+ private FileUtils.FileCopyResult unzipSegmentFiles(
+ final String containerName,
+ final String blobPath,
+ final File outDir
+ ) throws IOException
+ {
+ final ByteSource byteSource = byteSourceFactory.create(containerName,
blobPath, azureStorage);
+ return CompressionUtils.unzip(
+ byteSource,
+ outDir,
+ AzureUtils.AZURE_RETRY,
+ false
+ );
+ }
+
+ private FileUtils.FileCopyResult getSegmentFilesFromDirectory(
+ final String containerName,
+ final String blobPathPrefix,
+ final File outDir
+ )
+ {
+ final int maxTries = azureAccountConfig.getMaxTries();
+ final List<String> blobPaths = azureStorage.listBlobs(containerName,
blobPathPrefix, null, maxTries);
+ final FileUtils.FileCopyResult copyResult = new FileUtils.FileCopyResult();
+
+ for (final String blobPath : blobPaths) {
+ final ByteSource byteSource = byteSourceFactory.create(containerName,
blobPath, azureStorage);
+ final File outFile = new File(outDir,
Paths.get(blobPath).getFileName().toString());
+ copyResult.addFiles(
+ FileUtils.retryCopy(byteSource, outFile, AzureUtils.AZURE_RETRY,
maxTries).getFiles()
Review Comment:
[P2] Preserve pull failure cleanup and error handling
`FileUtils.retryCopy` wraps its final checked I/O failure in
`RuntimeException`, while `getSegmentFiles` only catches `IOException` (lines
84-96). A failed retry for any blob in this new directory path therefore
escapes as a runtime exception and bypasses the cleanup that removes `outDir`;
after earlier blobs were copied, the caller can be left with a partial segment.
It also bypasses the `SegmentLoadingException` contract used by `AzureLoadSpec`
and prevents `SegmentLocalCacheManager` from trying another storage location.
Preserve or translate the checked failure here so the existing cleanup and
fallback path apply.
##########
extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentPusher.java:
##########
@@ -121,19 +148,62 @@ public DataSegment pushToPath(File indexFilesDir,
DataSegment segment, String st
}
}
+ /**
+ * Uploads the segment files as they are, one blob per file, under {@code
azureBasePath}. The resulting loadSpec
+ * blobPath is the directory itself, with a trailing slash to tell {@link
AzureDataSegmentPuller} and
+ * {@link AzureDataSegmentKiller} that it names a directory of files rather
than a single blob.
+ */
+ private DataSegment pushNoZip(
+ File indexFilesDir,
+ DataSegment segment,
+ int binaryVersion,
+ String azureBasePath
+ ) throws IOException
+ {
+ final File[] files = indexFilesDir.listFiles();
+ if (files == null) {
+ throw new IOE("Cannot list directory [%s]", indexFilesDir);
+ }
+
+ long size = 0;
+ for (final File file : files) {
Review Comment:
[P1] Replace stale blobs when overwriting an unzipped segment
When `druid.storage.zip=false`, this loop overwrites only files present in
the new segment and never removes objects already under `azureBasePath`
(including a prior `index.zip` or old smoosh files). `push` is explicitly
allowed to run with `useUniquePath=false`—`BatchAppenderator` does this—and the
`DataSegmentPusher` contract requires that mode to replace a previous push. A
retry or replica with a different file set therefore leaves stale blobs;
overlapping pushes can also interleave per-file overwrites. The directory
puller lists the whole prefix, so it can load a mixed or corrupt segment. Stage
to an isolated prefix and publish/replace safely, or otherwise guarantee stale
destination blobs cannot be visible.
--
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]
