mcvsubbu commented on code in PR #10216:
URL: https://github.com/apache/pinot/pull/10216#discussion_r1098058410
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -1000,6 +1000,8 @@ protected boolean commitSegment(String controllerVipUrl,
boolean isSplitCommit)
SegmentCompletionProtocol.Response commitResponse =
commit(controllerVipUrl, isSplitCommit);
if
(!commitResponse.getStatus().equals(SegmentCompletionProtocol.ControllerResponseStatus.COMMIT_SUCCESS))
{
+ _segmentLogger.warn("Controller response was {} and not {}",
commitResponse.getStatus(),
Review Comment:
Please move this log to the place where the return value is detected as not
being SUCCESS. We already have other logs for non-success cases in the layers
below.
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/PinotFSSegmentUploader.java:
##########
@@ -54,6 +54,8 @@ public PinotFSSegmentUploader(String segmentStoreDirUri, int
timeoutMillis) {
public URI uploadSegment(File segmentFile, LLCSegmentName segmentName) {
if (_segmentStoreUriStr == null || _segmentStoreUriStr.isEmpty()) {
+ LOGGER.error("Missing segment store uri. Failed to upload segment file
{} for {}.", segmentFile.getName(),
Review Comment:
This missing log should suffice? You don't need the additional log above in
the segment data manager?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
