noob-se7en commented on code in PR #18914:
URL: https://github.com/apache/pinot/pull/18914#discussion_r3529050948
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -1539,7 +1499,13 @@ protected void postStopConsumedMsg(String reason) {
ConsumptionStopIndicator indicator = new
ConsumptionStopIndicator(_currentOffset,
_segmentNameStr, _instanceId, _protocolHandler, reason,
_segmentLogger);
do {
- SegmentCompletionProtocol.Response response =
indicator.postSegmentStoppedConsuming();
+ if (hasDifferentSegmentDataManagerRegistered()) {
+ _segmentLogger.info(
+ "Skip segmentStoppedConsuming for segment: {}, another segment
data manager is already registered",
+ _segmentNameStr);
+ return;
+ }
Review Comment:
We should add this check before this line:
https://github.com/apache/pinot/blob/8bfeeaa208331ab415c9a26c8455f189238cda63/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java#L1988
Reason: `postStopConsumedMsg` is called from two places. So it makes the
intent ("only the failed-construction janitor should second-guess itself")
explicit and guarantees zero behaviour change to the healthy consumer path.
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/SegmentOnlineOfflineStateModelFactory.java:
##########
@@ -236,6 +236,15 @@ public void onBecomeDroppedFromOnline(Message message,
NotificationContext conte
@Transition(from = "ERROR", to = "OFFLINE")
public void onBecomeOfflineFromError(Message message, NotificationContext
context) {
_logger.info("SegmentOnlineOfflineStateModel.onBecomeOfflineFromError()
: {}", message);
+ try {
+ _instanceDataManager.offloadSegment(message.getResourceName(),
message.getPartitionName());
Review Comment:
I don't think this fix targets the ghost consumer problem mentioned in :
https://github.com/apache/pinot/issues/18867 (because error -> offline
transition will never occur). So we can separate out this change from this PR
and raise a new one as a different problem statement.
--
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]
