walterddr commented on a change in pull request #7896:
URL: https://github.com/apache/pinot/pull/7896#discussion_r768815234
##########
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:
yes I thought about exactly the same thing. the return null also make
the caller alter the state --> if we do not try catch it properly the states
wont change.
Ultimately I think we need to rethink when should
`_realtimeTableDataManager.addSegmentError` occur, my opinion is that we should
make a single state machine function for each functionality, in this case: the
only places we catch and process exception should be: `PartitionConsumer` and
`goOnlineFromConsuming`, in which both modifies the internal state. the rest of
the internal methods should all bubble up the exception by catch and
rethrowing.
--
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]
