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/03/05 00:50:41 UTC

[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #6647: Recover the segment from controller when LLC table cannot load it

Jackie-Jiang opened a new pull request #6647:
URL: https://github.com/apache/incubator-pinot/pull/6647


   ## Description
   For LLC real-time table, when the server failed to load a local segment (e.g. segment is collapsed or default column got removed), try to download a new copy from the controller. This is the same behavior as the offline table.
   
   There is no easy way to add a test for this. Manually tested with the following steps:
   - Start a LLC table and consume some records
   - Manually break the segment file
   - Disable then re-enable the table to trigger the segment load
   - Observe a new segment downloaded from the controller and replaces the collapsed local segment


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6647: Recover the segment from controller when LLC table cannot load it

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6647:
URL: https://github.com/apache/incubator-pinot/pull/6647#discussion_r587956134



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##########
@@ -268,56 +269,69 @@ public void addSegment(String segmentName, TableConfig tableConfig, IndexLoading
     LoaderUtils.reloadFailureRecovery(indexDir);
 
     boolean isLLCSegment = SegmentName.isLowLevelConsumerSegmentName(segmentName);
-    LLCSegmentName llcSegmentName = null;
-    PartitionUpsertMetadataManager partitionUpsertMetadataManager = null;
-    if (isLLCSegment) {
-      llcSegmentName = new LLCSegmentName(segmentName);
-      if (_tableUpsertMetadataManager != null) {
-        partitionUpsertMetadataManager =
-            _tableUpsertMetadataManager.getOrCreatePartitionManager(llcSegmentName.getPartitionId());
+    if (indexDir.exists()) {
+      // Segment already exists on disk
+      if (realtimeSegmentZKMetadata.getStatus() == Status.DONE) {
+        // Metadata has been committed, load the local segment
+        try {
+          addSegment(ImmutableSegmentLoader.load(indexDir, indexLoadingConfig, schema));
+          return;
+        } catch (Exception e) {
+          if (isLLCSegment) {
+            // For LLC and segments, delete the local copy and download a new copy from the controller
+            FileUtils.deleteQuietly(indexDir);
+            if (e instanceof V3RemoveIndexException) {
+              _logger.info("Unable to remove index from V3 format segment: {}, downloading a new copy", segmentName, e);

Review comment:
       Why do we need to log the exception here? 




----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #6647: Recover the segment from controller when LLC table cannot load it

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #6647:
URL: https://github.com/apache/incubator-pinot/pull/6647


   


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6647: Recover the segment from controller when LLC table cannot load it

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6647:
URL: https://github.com/apache/incubator-pinot/pull/6647#discussion_r587965796



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
##########
@@ -268,56 +269,69 @@ public void addSegment(String segmentName, TableConfig tableConfig, IndexLoading
     LoaderUtils.reloadFailureRecovery(indexDir);
 
     boolean isLLCSegment = SegmentName.isLowLevelConsumerSegmentName(segmentName);
-    LLCSegmentName llcSegmentName = null;
-    PartitionUpsertMetadataManager partitionUpsertMetadataManager = null;
-    if (isLLCSegment) {
-      llcSegmentName = new LLCSegmentName(segmentName);
-      if (_tableUpsertMetadataManager != null) {
-        partitionUpsertMetadataManager =
-            _tableUpsertMetadataManager.getOrCreatePartitionManager(llcSegmentName.getPartitionId());
+    if (indexDir.exists()) {
+      // Segment already exists on disk
+      if (realtimeSegmentZKMetadata.getStatus() == Status.DONE) {
+        // Metadata has been committed, load the local segment
+        try {
+          addSegment(ImmutableSegmentLoader.load(indexDir, indexLoadingConfig, schema));
+          return;
+        } catch (Exception e) {
+          if (isLLCSegment) {
+            // For LLC and segments, delete the local copy and download a new copy from the controller
+            FileUtils.deleteQuietly(indexDir);
+            if (e instanceof V3RemoveIndexException) {
+              _logger.info("Unable to remove index from V3 format segment: {}, downloading a new copy", segmentName, e);

Review comment:
       The exception contains the column that cannot be removed for debugging purpose. Here I just keep the same behavior as the offline one in `SegmentFetcherAndLoader`.




----------------------------------------------------------------
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:
users@infra.apache.org



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