sajjad-moradi commented on a change in pull request #7896:
URL: https://github.com/apache/pinot/pull/7896#discussion_r768340557
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
##########
@@ -624,14 +624,14 @@ public void run() {
_state = State.DISCARDED;
break;
case DEFAULT:
- success = buildSegmentAndReplace();
- if (success) {
+ try {
+ buildSegmentAndReplace();
Review comment:
This is not the only place `buildSegmentAndReplace` is being used. It's
also being used two times in `goOnlineFromConsuming` method. If we throw
exception in buildSegmentAndRepalce, then the segment goes to error state in
external view upon receiving helix transition message consuming -> offline.
The existing behaviour is even worse because segment is not built but
external view will move to online state!!
I suggest we proceed with throwing exception in buildSegmentAndReplace, but
we try-catch its usage in `goOnlineFromConsuming` and in the catch part, we use
`downloadSegmentAndReplace`.
@mcvsubbu @Jackie-Jiang what do you guys think?
--
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]
