FrankChen021 commented on code in PR #20247:
URL: https://github.com/apache/druid/pull/20247#discussion_r3940649742
##########
server/src/main/java/org/apache/druid/server/coordinator/duty/CloneHistoricals.java:
##########
@@ -285,4 +319,72 @@ private boolean shouldLoadSegmentOnTargetServer(
final PartialLoadProfile targetProfile =
targetServer.getProjectedProfile(segment);
return !Objects.equals(fingerprintOf(sourceProfile),
fingerprintOf(targetProfile));
}
+
+ private boolean isSynced(CloningStats stats, CloneSyncCriteria criteria)
+ {
+ return stats.segmentsPendingSync <= criteria.getMaxSegmentsPendingSync()
+ && stats.percentPendingSync() <= criteria.getMaxPercentPendingSync();
+ }
+
+ /**
+ * Adds metrics to the run stats if the state has changed to SYNCED in this
run.
+ */
+ private void collectMetricsIfStateChanged(
+ ServerHolder server,
+ ServerCloneStatus newStatus,
+ CoordinatorRunStats stats
+ )
+ {
+ final String targetServerName = server.getServer().getName();
+ final ServerCloneStatus oldStatus =
cloneStatusManager.getStatusForServer(targetServerName);
+
+ stats.add(
+ Stats.Segments.PENDING_SYNC_ON_CLONE,
+ RowKey.of(Dimension.SERVER, targetServerName),
Review Comment:
[P2] Include tier in pending-sync metric
`segment/clone/pendingSync/count` is documented with `server,tier`
dimensions, and the other clone metrics now include the target tier, but this
row key is still server-only. In a multi-tier deployment the emitted event
cannot be filtered or grouped by tier and does not match the documented metric
contract. Include `Dimension.TIER` for the target in this row key.
##########
docs/operations/metrics.md:
##########
@@ -452,6 +452,9 @@ These metrics are emitted by the Druid Coordinator in every
run of the correspon
|`segment/assignSkipped/count`|Number of segments that could not be assigned
to any server for loading. This can occur due to replication throttling, no
available disk space, or a full load queue.|`dataSource`, `server`, `tier`,
`description`|Varies|
|`segment/moveSkipped/count`|Number of segments that were chosen for balancing
but could not be moved. This can occur when segments are already optimally
placed.|`dataSource`, `server`, `tier`, `description`|Varies|
|`segment/dropSkipped/count`|Number of segments that could not be dropped from
any server.|`dataSource`, `server`, `tier`, `description`|Varies|
+|`segment/clone/assigned/count`|Number of segments assigned to be loaded on a
historical clone.|`dataSource`, `server`, `tier`|Varies|
+|`segment/clone/dropped/count`|Number of segments assigned to be loaded on a
historical clone.|`dataSource`, `server`, `tier`|Varies|
Review Comment:
[P3] Correct dropped-clone metric description
This is the description for `segment/clone/dropped/count`, but it says
segments are assigned to be loaded. The stat is emitted from
`DROPPED_FROM_CLONE` when a drop is queued, so the description should say that
segments are assigned to be dropped; otherwise operators will interpret this
new metric backwards.
##########
server/src/main/java/org/apache/druid/server/coordinator/duty/CloneHistoricals.java:
##########
@@ -119,6 +125,9 @@ public DruidCoordinatorRuntimeParams
run(DruidCoordinatorRuntimeParams params)
final PartialLoadProfile sourceProfile =
sourceServer.getProjectedProfile(segment);
if (shouldLoadSegmentOnTargetServer(segment, sourceProfile,
targetServer, targetProjectedSegments)) {
loadSegmentOnTargetServer(segment, sourceProfile, targetServer,
params);
+
cloningStats.incrementMissingSegmentCount(sourceServer.isServingSegment(segment));
Review Comment:
[P2] Do not count skipped segments as pending
`getLoadableSegment` explicitly returns null for unused segments, so this
call can queue no load. The counter is incremented unconditionally after the
call whenever the source still serves the segment. An unused segment that
remains in the source inventory therefore contributes to `segmentsPendingSync`
and its percentage until it is unloaded, and can keep a clone `IN_PROGRESS`
even though the clone duty intentionally skips that segment. Count it only when
the segment is eligible for reconciliation, and cover a served-but-unused
source segment.
--
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]
