You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/12/15 00:14:16 UTC

[GitHub] [pinot] mcvsubbu commented on a change in pull request #7896: make buildSegmentInternal raise error out explicitly

mcvsubbu commented on a change in pull request #7896:
URL: https://github.com/apache/pinot/pull/7896#discussion_r769141254



##########
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();
                     _state = State.RETAINED;
-                  } else {
+                  } catch (Exception e) {
                     // Could not build segment for some reason. We can only download it.
-                    _state = State.ERROR;
                     _realtimeTableDataManager.addSegmentError(_segmentNameStr,
-                        new SegmentErrorInfo(System.currentTimeMillis(), "Could not build segment", null));
+                        new SegmentErrorInfo(System.currentTimeMillis(), "Could not build segment!", e));
+                    _state = State.ERROR;

Review comment:
       see comment above regarding state

##########
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:
       The state should be set to DISCARDED if buildSegmentAndReplace fails for whatever reason (It really should not, since another host was able to build). This will allow us to replace the segment by downloading, as a last effort to keep the service going.
   
   What exception are you seeing? Can you paste a stack?




-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org