capistrant commented on code in PR #20338:
URL: https://github.com/apache/druid/pull/20338#discussion_r4108343802
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -370,6 +389,101 @@ private void addFilesToCachedSegments(
}
}
+ /**
+ * Rewrite a complete on-disk cache layout for {@code segment} as a partial
(bundle) layout, returning whether it
+ * did. Called from {@link #getCachedSegments}, which runs before anything
is reserved, mounted, or serving, so
+ * there is no live entry to unmap and no cache key to swap. This method
should never be called on a 'live' entry.
+ * <p>
+ * The conversion reads the local complete file as if it were deep storage,
via a
+ * {@link DirectoryBackedRangeReader}, and materializes the bundle layout
from it. Nothing is fetched remotely.
+ * Deep storage is still probed for range-readability, because a partial
entry that cannot range-read deep storage
+ * could never fetch a bundle it is missing later, and {@link
#reservePartialForBootstrap} reclaims such a layout
+ * on the next boot anyway.
+ * <p>
+ * Transient disk cost is one segment: the complete file has to stay
readable until the bundles are written. If
+ * anything fails, the complete layout is left exactly as it was and the
segment loads the old way, so a location
+ * without the headroom degrades instead of breaking. Staging happens under
{@link #CONVERT_PATH}, swept at
+ * startup, so a crash leaves garbage rather than a half-converted segment
directory. The one lossy window is a
+ * crash between removing the complete layout and moving the converted one
into place, which costs a re-download
+ * of that segment.
+ */
+ private boolean convertCompleteLayoutToPartial(final DataSegment segment,
final StorageLocation location)
+ {
+ if (!config.isVirtualStorage() ||
!config.isVirtualStoragePartialDownloadsEnabled()) {
+ return false;
+ }
+ final File completeDir = new File(location.getPath(),
segment.getId().toString());
+ if (!new File(completeDir, IndexIO.V10_FILE_NAME).exists()) {
Review Comment:
sounds reasonable
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -370,6 +389,101 @@ private void addFilesToCachedSegments(
}
}
+ /**
+ * Rewrite a complete on-disk cache layout for {@code segment} as a partial
(bundle) layout, returning whether it
+ * did. Called from {@link #getCachedSegments}, which runs before anything
is reserved, mounted, or serving, so
+ * there is no live entry to unmap and no cache key to swap. This method
should never be called on a 'live' entry.
+ * <p>
+ * The conversion reads the local complete file as if it were deep storage,
via a
+ * {@link DirectoryBackedRangeReader}, and materializes the bundle layout
from it. Nothing is fetched remotely.
+ * Deep storage is still probed for range-readability, because a partial
entry that cannot range-read deep storage
+ * could never fetch a bundle it is missing later, and {@link
#reservePartialForBootstrap} reclaims such a layout
+ * on the next boot anyway.
+ * <p>
+ * Transient disk cost is one segment: the complete file has to stay
readable until the bundles are written. If
+ * anything fails, the complete layout is left exactly as it was and the
segment loads the old way, so a location
+ * without the headroom degrades instead of breaking. Staging happens under
{@link #CONVERT_PATH}, swept at
+ * startup, so a crash leaves garbage rather than a half-converted segment
directory. The one lossy window is a
+ * crash between removing the complete layout and moving the converted one
into place, which costs a re-download
+ * of that segment.
+ */
+ private boolean convertCompleteLayoutToPartial(final DataSegment segment,
final StorageLocation location)
+ {
+ if (!config.isVirtualStorage() ||
!config.isVirtualStoragePartialDownloadsEnabled()) {
+ return false;
+ }
+ final File completeDir = new File(location.getPath(),
segment.getId().toString());
+ if (!new File(completeDir, IndexIO.V10_FILE_NAME).exists()) {
+ // Only a V10 segment file can be re-sliced into bundles.
+ return false;
+ }
+ try {
+ if (tryOpenRangeReader(segment) == null) {
+ return false;
+ }
+ }
+ catch (Exception e) {
+ log.warn(
+ e,
+ "Failed to open a range reader for segment[%s]; leaving its complete
cache layout alone",
+ segment.getId()
+ );
+ return false;
+ }
+
+ final File stagingDir = new File(new File(location.getPath(),
CONVERT_PATH), segment.getId().toString());
+ boolean completeLayoutRemoved = false;
+ try {
+ if (stagingDir.exists()) {
+ FileUtils.deleteDirectory(stagingDir);
+ }
+ try (PartialSegmentFileMapperV10 mapper =
PartialSegmentFileMapperV10.create(
+ new DirectoryBackedRangeReader(completeDir),
+ jsonMapper,
+ stagingDir,
+ IndexIO.V10_FILE_NAME,
+ List.of(),
+ PartialSegmentDownloadListener.NOOP,
+ config.getVirtualStorageCoalesceGapBytes(),
+ config.getVirtualStorageMaxFetchRunBytes()
+ )) {
+ mapper.ensureAllDownloaded();
+ }
+ atomicMoveAndDeleteCacheEntryDirectory(completeDir);
+ completeLayoutRemoved = true;
+ Files.move(stagingDir.toPath(), completeDir.toPath(),
StandardCopyOption.ATOMIC_MOVE);
Review Comment:
this topic, I'm not so familiar with so idk if this is legit at face value
--
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]
