AmatyaAvadhanula commented on code in PR #16563:
URL: https://github.com/apache/druid/pull/16563#discussion_r1630961064


##########
docs/operations/metrics.md:
##########
@@ -280,6 +280,8 @@ If the JVM does not support CPU time measurement for the 
current thread, `ingest
 |`segment/added/bytes`|Size in bytes of new segments created.| `dataSource`, 
`taskId`, `taskType`, `groupId`, `interval`, `tags`|Varies|
 |`segment/moved/bytes`|Size in bytes of segments moved/archived via the Move 
Task.| `dataSource`, `taskId`, `taskType`, `groupId`, `interval`, `tags`|Varies|
 |`segment/nuked/bytes`|Size in bytes of segments deleted via the Kill Task.| 
`dataSource`, `taskId`, `taskType`, `groupId`, `interval`, `tags`|Varies|
+| `segment/upgraded/count`| Number of published segments upgraded by a replace 
task.|`dataSource`, `taskId`, `taskType`, `groupId`, `interval`, `tags`|Varies|
+| `segment/upgradedRealtime/count`| Number of realtime segments upgraded by a 
replace task.|`dataSource`, `taskId`, `taskType`, `groupId`, `interval`, 
`tags`|Varies|

Review Comment:
   Please rename this to `pendingSegment/upgraded/count` or something similar.
   Upgraded pending segments need not correspond only to realtime tasks.



##########
indexing-service/src/main/java/org/apache/druid/indexing/common/actions/SegmentTransactionalReplaceAction.java:
##########
@@ -162,6 +164,51 @@ public SegmentPublishResult perform(Task task, 
TaskActionToolbox toolbox)
     return publishResult;
   }
 
+  private void emitSegmentUpgradeMetrics(SegmentPublishResult publishResult, 
Task task, TaskActionToolbox toolbox)
+  {
+    // Log and emit metric for number of upgraded append segments
+    final List<DataSegment> upgradedAppendSegments = 
publishResult.getUpgradedAppendSegments();
+    if (!CollectionUtils.isNullOrEmpty(upgradedAppendSegments)) {
+      final Map<String, Integer> versionToNumUpgradedSegments = new 
HashMap<>();
+      upgradedAppendSegments.forEach(
+          segment -> 
versionToNumUpgradedSegments.merge(segment.getId().getVersion(), 1, 
Integer::sum)
+      );
+      versionToNumUpgradedSegments.forEach(
+          (version, numUpgradedSegments) -> log.info(
+              "Task[%s] of datasource[%s] upgraded [%d] append segments to 
replace version[%s].",

Review Comment:
   ... upgraded [%d] segments ...



##########
server/src/main/java/org/apache/druid/indexing/overlord/SegmentPublishResult.java:
##########
@@ -50,16 +50,22 @@ public class SegmentPublishResult
   @Nullable
   private final String errorMsg;
   @Nullable
+  private final List<DataSegment> upgradedAppendSegments;

Review Comment:
   Just upgradedSegments seems sufficient.



-- 
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]

Reply via email to