FrankChen021 commented on code in PR #19843:
URL: https://github.com/apache/druid/pull/19843#discussion_r3690441806
##########
server/src/main/java/org/apache/druid/server/coordinator/loading/PartialLoadProfile.java:
##########
@@ -93,6 +93,17 @@ public static PartialLoadProfile forLoaded(Map<String,
Object> wrappedLoadSpec,
return intern(new PartialLoadProfile(wrappedLoadSpec, fingerprint,
loadedBytes));
}
+ /**
+ * This profile in request form, for reissuing to another server the same
partial load that produced it. A profile
+ * read back off a server carries the footprint that server realized, which
belongs to that server's announcement
+ * and not to a request; the wrapped load spec and fingerprint are what
identify the request. Returns {@code this}
+ * when the profile is already a request.
+ */
+ public PartialLoadProfile asRequest()
+ {
+ return loadedBytes == null ? this : forRequest(wrappedLoadSpec,
fingerprint);
Review Comment:
[P2] Rebase forwarded profiles onto the current load spec
wrappedLoadSpec contains the source announcement's original deep-storage
delegate, and asRequest carries it forward verbatim. Both new callers start
with a DataSegment fetched from the current metadata snapshot, but
SegmentHolder subsequently replaces that segment's loadSpec with this stale
wrapper. If a segment payload was corrected or migrated since the source
loaded, clone and move requests therefore use the old location; after that
object is removed, cloning fails and balancing or decommissioning moves cannot
complete. Preserve the selection and fingerprint while rebuilding the wrapper
with the latest metadata loadSpec as its delegate.
##########
server/src/main/java/org/apache/druid/server/coordinator/duty/CloneHistoricals.java:
##########
@@ -141,7 +161,12 @@ private void loadSegmentOnTargetServer(
rowKey.and(Dimension.DESCRIPTION, "Segment not found in metadata
cache"),
1L
);
- } else if (loadQueueManager.loadSegment(loadableSegment, targetServer,
SegmentAction.LOAD)) {
+ } else if (loadQueueManager.loadSegment(
+ loadableSegment,
+ targetServer,
+ SegmentAction.LOAD,
+ sourceProfile == null ? null : sourceProfile.asRequest()
Review Comment:
[P1] Clear partial state before cloning a full load
When the source profile is null and the target already has a partial
profile, this queues a plain LOAD. On a virtual-storage Historical, however,
the plain SegmentLocalCacheManager.load path does not clear
PartialSegmentMetadataCacheEntry's rule or rewrite its partial info file;
SegmentManager also leaves the existing segment in place. The announcer then
reports a plain profile, so the coordinator believes the clone is full while
the old rule holds remain, and a restart re-applies and re-announces the stale
partial profile. Explicitly clear the persisted partial rule for this
conversion or perform a drop/reload. The added unit test only verifies the
queued request, not this Historical-side transition.
--
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]
