siddharthteotia commented on a change in pull request #4882:  Failure to 
recover after segment reload failure does not release segment lock
URL: https://github.com/apache/incubator-pinot/pull/4882#discussion_r353512823
 
 

 ##########
 File path: 
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java
 ##########
 @@ -237,8 +237,13 @@ private void reloadSegment(@Nonnull String 
tableNameWithType, @Nonnull SegmentMe
       // Delete segment temporary directory
       FileUtils.deleteDirectory(segmentTempDir);
     } finally {
-      LoaderUtils.reloadFailureRecovery(indexDir);
-      segmentLock.unlock();
+      try {
+        LoaderUtils.reloadFailureRecovery(indexDir);
+      } catch (Exception e) {
 
 Review comment:
   To avoid overriding, I had earlier chosen to just log the recovery failure 
exception. 
   
   Now the recovery is done only during catch block and we retain the top level 
exception (due to reload failure), log the recovery failure exception (if any) 
and also add it as a suppressed one to the parent exception. 
   
   Lock is released in finally and we still throw the top level exception.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to