imply-cheddar commented on code in PR #14089:
URL: https://github.com/apache/druid/pull/14089#discussion_r1168371060
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialSegmentMergeTask.java:
##########
@@ -266,6 +271,7 @@ private Set<DataSegment> mergeAndPushSegments(
persistDir,
0
);
+ long mergeFinishTime = System.nanoTime();
Review Comment:
It might be nice to add a message here stating that the merge completed. It
can be nice to see it in its own line when you are watching logs for progress.
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialSegmentMergeTask.java:
##########
@@ -294,7 +300,20 @@ private Set<DataSegment> mergeAndPushSegments(
exception -> !(exception instanceof NullPointerException) &&
exception instanceof Exception,
5
);
+ long pushFinishTime = System.nanoTime();
pushedSegments.add(segment);
+ LOG.info(
+ "Segment[%s] of %,d bytes "
+ + "built from %d partial segment(s) in %,dms; "
+ + "pushed to deep storage in %,dms. "
+ + "Load spec is: %s",
+ segment.getId(),
+ segment.getSize(),
+ segmentFilesToMerge.size(),
+ (mergeFinishTime - startTime) / 1000000,
+ (pushFinishTime - mergeFinishTime) / 1000000,
+ jsonMapper.writeValueAsString(segment.getLoadSpec())
Review Comment:
I don't think it's worth passing in an `ObjectMapper` for it. `.toString()`
is hopefully good enough. So, please remove the `ObjectMapper` from the
constructor and just toString the thing.
Also, this log message can be better. You can see a write-up of the idea
here: https://github.com/apache/druid/blob/master/dev/style-conventions.md
Perhaps something like
`"built segment [%s] ([%,d] input segments in [%,d] ms) and pushed ([%,d]
ms) to deep storage [%s]"`
--
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]
