Resolve failures pauseless ingestion #14798

9aman · 2025-01-13T09:46:46Z

Pauseless Ingestion Failure Resolution

Please refer to PR: #14741 for happy path. This PR aims to only cover the failure scenarios. Once the above one is merged a better diff covering only failures will be visible.

To view only diff covering failure scenarios, for the time being, refer to:

Summary

This PR aims to provide ways to resolve the failure scenarios that we can encounter during pauseless ingestion. The detailed list of failure scenarios can be found here: link along with the failure handling strategies: link

Following sequence diagrams summarizes the failure scenarios and the resolution.

Failure Scenarios & Resolution Approaches

Failures encountered during the commit protocol can be categorized into two types: recoverable and unrecoverable failures.

Recoverable failures are those in which at least one of the servers retains the segment on disk.

Unrecoverable failures occur when none of the servers have the segment on disk.

Recoverable Failures

Recoverable failures will be addressed through RealtimeSegmentValidationManager. This approach will handle scenarios such as upload failures and incomplete commit protocol executions.

The controller or server can run into issues in between any of the steps of the commit protocol as listed below:

Request Type: COMMIT_START

Update the Segment ZK metadata for the committing segment (seg__0__0)
- Change status to COMMITTING
- Set endOffset
Create Segment ZK metadata for the new segment (seg__0__1) with status IN_PROGRESS
Update the Ideal State for the:
- Committing segment (seg__0__0) to ONLINE
- New/ Consuming segment (seg__0__1) to CONSUMING

Request Type: COMMIT_END_METADATA
4. Update Segment ZK metadata for the committing segment (seg__0__0):
- Change status to DONE.
- Update deepstore url.
- Any additional metadata.

The RealtimeSegmentValidationManager figures out which step of the commit protocol failed and how can it be fixed. This is very similar to how commit protocol failures were handled before with some minor changes.

Non-recoverable Failures

These failures require ingesting the segment again from upstream, followed by build, upload and ZK metadata update.

1. Changing FSM 2. Changing the 3 steps performed during the commit protocol to update ZK and Ideal state

1. Changes in the commit protocol to start segment commit before the build 2. Changes in the BaseTableDataManager to ensure that the locally built segment is replaced by a downloaded one only when the CRC is present in the ZK Metadata 3. Changes in the download segment method to allow waited download in case of pauseless consumption

…segment commit end metadata call Refactoing code for redability

… ingestion by moving it out of streamConfigMap

…auseless ingestion in RealtimeSegmentValidationManager

…d by RealtimeSegmentValitdationManager to fix commit protocol failures

…g commit protocol

KKcorps · 2025-01-14T02:06:45Z

...r/src/main/java/org/apache/pinot/controller/validation/RealtimeSegmentValidationManager.java

@@ -171,6 +172,9 @@ private void runSegmentLevelValidation(TableConfig tableConfig) {
    // Update the total document count gauge
    _validationMetrics.updateTotalDocumentCountGauge(realtimeTableName, computeTotalDocumentCount(segmentsZKMetadata));

+    _llcRealtimeSegmentManager.reIngestSegmentsWithErrorState(tableConfig.getTableName());
+//    _llcRealtimeSegmentManager.resetUploadedSegmentsInErrorState(tableConfig.getTableName());


Check if this commented line is needed or not, otherwise remove it

Now that reingestion does this I think it's okay to remove this.
@KKcorps can you please do this.

KKcorps · 2025-01-14T02:08:01Z

pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java

-      return;
+    TableConfig tableConfig = indexLoadingConfig.getTableConfig();
+    // For pauseless tables, we should replace the segment if download url is missing even if crc is same
+    if (!PauselessConsumptionUtils.isPauselessEnabled(tableConfig)) {


Should also LOG that since pauseless is enabled we are not going directly for segment download but waiting for catchup

@KKcorps I don't understand why this is different for pauseless. I see you have added these along with reingestion tests.

KKcorps · 2025-01-14T02:08:38Z

.../org/apache/pinot/integration/tests/realtime/FailureInjectingRealtimeSegmentDataManager.java

+
+  protected SegmentBuildDescriptor buildSegmentInternal(boolean forCommit) {
+     if (_failCommit) {
+      System.out.println("Forcing failure in buildSegmentInternal");


Remove this sysout

KKcorps · 2025-01-14T02:09:56Z

pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java

@@ -884,7 +892,7 @@ public String uploadLLCSegment(
      @ApiResponse(code = 404, message = "Table or segment not found", response = ErrorInfo.class),
      @ApiResponse(code = 400, message = "Bad request", response = ErrorInfo.class)
  })
-  public TableLLCSegmentUploadResponse uploadLLCSegmentV2(
+  public String uploadLLCSegmentV2(


Do not change this to String as it will not ensure backward compatibility.
If you need something, add a new param in TableLLCSegmentUploadResponse itself

I feel like you need a new API here

I am adding a new API and making the required changes on the controller end.

Added a new API

KKcorps · 2025-01-14T02:12:11Z

pinot-server/src/main/java/org/apache/pinot/server/api/resources/ReIngestionResource.java

+  private static final ConcurrentHashMap<String, AtomicBoolean> SEGMENT_INGESTION_MAP = new ConcurrentHashMap<>();
+
+  // Semaphore to enforce global concurrency limit
+  private static final Semaphore REINGESTION_SEMAPHORE = new Semaphore(MAX_PARALLEL_REINGESTIONS);


Make this count configurable

KKcorps · 2025-01-14T02:14:23Z

...ain/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java

@@ -543,13 +555,22 @@ private void commitSegmentMetadataInternal(String realtimeTableName,
    SegmentZKMetadata committingSegmentZKMetadata =
        updateCommittingSegmentMetadata(realtimeTableName, committingSegmentDescriptor, isStartMetadata);

+    // Used to inject failure for testing. RealtimeSegmentValidationManager should be able to fix the
+    // segment that encounter failure at this stage of commit protocol.
+    FailureInjectionUtils.injectFailure(FailureInjectionUtils.FAULT_BEFORE_NEW_SEGMENT_METADATA_CREATION,


A better way to inject failures for test can be to extends this PinotLLCRealtimeSegmentDataManager that throws failure.

For now I have kept the current approach.

KKcorps · 2025-01-14T02:15:56Z

...ain/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java

+   */
+  private void updateSegmentMetadata(SegmentZKMetadata segmentZKMetadata, SegmentZKMetadata uploadedSegmentZKMetadata,
+      boolean isPauselessEnabled) {
+    if (isPauselessEnabled && segmentZKMetadata.getCrc() == SegmentZKMetadata.DEFAULT_CRC_VALUE) {


Should the CRC check here be changed to Status?

Have changes it to status

...ain/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java

pinot-server/src/main/java/org/apache/pinot/server/api/resources/ReIngestionResource.java

pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java

Jackie-Jiang

Partially reviewed. I'd suggest breaking this PR into smaller ones, separating the recoverable handling (i.e. ValidationManager) and reingest part

Jackie-Jiang · 2025-01-14T02:33:32Z

pinot-common/src/main/java/org/apache/pinot/common/metadata/segment/SegmentZKMetadataUtils.java

+    return mapper;
+  }
+
+  public static String serialize(SegmentZKMetadata metadata) throws IOException {


(format) Please apply Pinot Style and reformat all the changes

Jackie-Jiang · 2025-01-14T02:33:41Z

pinot-common/src/main/java/org/apache/pinot/common/metadata/segment/SegmentZKMetadataUtils.java

+  private SegmentZKMetadataUtils() {
+  }
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(SegmentZKMetadataUtils.class);


(nit) Not used

Jackie-Jiang · 2025-01-14T02:36:00Z

pinot-common/src/main/java/org/apache/pinot/common/metadata/segment/SegmentZKMetadataUtils.java

+  private static final Logger LOGGER = LoggerFactory.getLogger(SegmentZKMetadataUtils.class);
+  public static final ObjectMapper MAPPER = createObjectMapper();
+
+  private static ObjectMapper createObjectMapper() {


Any specific reason for adding this custom mapper instead of using the JsonUtils?
This change is independent of failure handling, so if this is really required, can you put this as a separate PR?

uploadLLCToSegmentStoreWithZKMetadata uses this. I was running into issues while serializing/ deserializing SegmentZKMetadata directly.

pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java

Jackie-Jiang · 2025-01-14T02:38:28Z

pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java

   * @throws URISyntaxException
   * @throws IOException
   * @throws HttpErrorStatusException
   */
-  public TableLLCSegmentUploadResponse uploadLLCToSegmentStore(String uri)
+  public SegmentZKMetadata uploadLLCToSegmentStore(String uri)


This change is backward incompatible. You might need to add a new API for this

Have added a new APi. On the controller side I have ensured backward compatibility by trying the previous endpoints as well in case the latest endpoint for segment upload fails.

pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java

KKcorps · 2025-01-15T03:44:44Z

Also add server metrics to count how many reIngestions are in progress

… checks

codecov-commenter · 2025-01-15T09:33:31Z

Codecov Report

Attention: Patch coverage is 12.08936% with 669 lines in your changes missing coverage. Please review.

Project coverage is 63.55%. Comparing base (59551e4) to head (7974ab9).
Report is 1593 commits behind head on master.

Files with missing lines	Patch %	Lines
...estion/utils/SimpleRealtimeSegmentDataManager.java	0.00%	246 Missing ⚠️
...inot/server/api/resources/ReIngestionResource.java	0.00%	176 Missing ⚠️
.../core/realtime/PinotLLCRealtimeSegmentManager.java	32.54%	109 Missing and 5 partials ⚠️
...che/pinot/server/api/resources/TablesResource.java	0.00%	70 Missing ⚠️
...e/pinot/common/utils/FileUploadDownloadClient.java	0.00%	30 Missing ⚠️
.../api/resources/reingestion/ReIngestionRequest.java	0.00%	7 Missing ⚠️
...ommon/metadata/segment/SegmentZKMetadataUtils.java	75.00%	3 Missing and 3 partials ⚠️
...r/validation/RealtimeSegmentValidationManager.java	0.00%	6 Missing ⚠️
...api/resources/reingestion/ReIngestionResponse.java	0.00%	6 Missing ⚠️
...troller/helix/core/util/FailureInjectionUtils.java	50.00%	1 Missing and 1 partial ⚠️
... and 4 more

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #14798      +/-   ##
============================================
+ Coverage     61.75%   63.55%   +1.79%     
- Complexity      207     1610    +1403     
============================================
  Files          2436     2714     +278     
  Lines        133233   151939   +18706     
  Branches      20636    23425    +2789     
============================================
+ Hits          82274    96559   +14285     
- Misses        44911    48137    +3226     
- Partials       6048     7243    +1195

Flag	Coverage Δ
custom-integration1	`100.00% <ø> (+99.99%)`	⬆️
integration	`100.00% <ø> (+99.99%)`	⬆️
integration1	`100.00% <ø> (+99.99%)`	⬆️
integration2	`0.00% <ø> (ø)`
java-11	`63.46% <12.08%> (+1.75%)`	⬆️
java-21	`63.43% <12.08%> (+1.80%)`	⬆️
skip-bytebuffers-false	`63.48% <12.08%> (+1.74%)`	⬆️
skip-bytebuffers-true	`63.41% <12.08%> (+35.68%)`	⬆️
temurin	`63.55% <12.08%> (+1.79%)`	⬆️
unittests	`63.54% <12.08%> (+1.79%)`	⬆️
unittests1	`56.32% <46.05%> (+9.43%)`	⬆️
unittests2	`33.97% <8.67%> (+6.24%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…ption is enabled or not

…eepstore path with fallbacks

9aman and others added 15 commits January 2, 2025 16:57

Controller side changes to allow pauseless ingestion:

6defce3

1. Changing FSM 2. Changing the 3 steps performed during the commit protocol to update ZK and Ideal state

Minor improvements to improve readability

54ab7b3

Fixes in the commit protocol for pauseless ingestion to complete the …

012da87

…segment commit end metadata call Refactoing code for redability

Changing the way to enable pausless post introduction of multi-stream…

a97847f

… ingestion by moving it out of streamConfigMap

WIP: Changes in the expected state while performing recovery during p…

2c2ba86

…auseless ingestion in RealtimeSegmentValidationManager

Adding changes for ensurePartitionsConsuming function that is utilize…

4d7c893

…d by RealtimeSegmentValitdationManager to fix commit protocol failures

Add Server side Reingestion API

a041a75

run segment level validation

b6d0904

Add method to trigger reingestion

58f6c51

Linting fixes

d6313b3

Adding integration tests for 3 failure scenarios that can occur durin…

ca6134a

…g commit protocol

WIP: Reingestion test

845f616

Fix bug with indexes in reingestion

d2dd313

Add tests for reingestion

fb34fc8

This was referenced Jan 13, 2025

Add DR and Failure scenario handling for Pauseless Ingestion #14794

Closed

Pauseless ingestion without failure scenarios #14741

Merged

KKcorps requested a review from Jackie-Jiang January 14, 2025 01:59

Jackie-Jiang added feature release-notes Referenced by PRs that need attention when compiling the next release notes ingestion real-time labels Jan 14, 2025

KKcorps reviewed Jan 14, 2025

View reviewed changes

...ain/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java Outdated Show resolved Hide resolved

KKcorps reviewed Jan 14, 2025

View reviewed changes

pinot-server/src/main/java/org/apache/pinot/server/api/resources/ReIngestionResource.java Outdated Show resolved Hide resolved

KKcorps reviewed Jan 14, 2025

View reviewed changes

pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java Outdated Show resolved Hide resolved

Jackie-Jiang reviewed Jan 14, 2025

View reviewed changes

KKCorps added 2 commits January 14, 2025 23:28

Fix controller url in Reingestion

50725bd

Support https and auth in reingestion

e74d360

9aman changed the base branch from pauseless_ingestion_without_failure_scenarios to master January 15, 2025 03:30

9aman changed the base branch from master to pauseless_ingestion_without_failure_scenarios January 15, 2025 03:31

Merge branch 'master' into resolve-failures-pauseless-ingestion

ce3c851

9aman changed the base branch from pauseless_ingestion_without_failure_scenarios to master January 15, 2025 08:26

Removing checks on default crc and replacing them with segment status…

d6208a6

… checks

9aman force-pushed the resolve-failures-pauseless-ingestion branch from 2b7e26a to d6208a6 Compare January 15, 2025 08:51

9aman and others added 9 commits January 15, 2025 16:09

Formatting improvements

7f5b720

Allowing null table config to be passed for checking pauseless consum…

3f05b2f

…ption is enabled or not

Ensuring upload retries are backward compatible

2012e38

Removing unnecessary code

7b9da37

Fixing existing test cases and adding unit tests to check upload to d…

ded8962

…eepstore path with fallbacks

Refactor test cases to reduce repetition

e703d84

Add missing header file

c2fda4a

Fix reingestion test

c836009

refactoring file upload download client

7974ab9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve failures pauseless ingestion #14798

Resolve failures pauseless ingestion #14798

9aman commented Jan 13, 2025

KKcorps Jan 14, 2025

9aman Jan 15, 2025

9aman Jan 16, 2025

KKcorps Jan 14, 2025

9aman Jan 15, 2025

KKcorps Jan 14, 2025

9aman Jan 16, 2025

KKcorps Jan 14, 2025

KKcorps Jan 14, 2025

9aman Jan 15, 2025

9aman Jan 15, 2025

KKcorps Jan 14, 2025

KKcorps Jan 14, 2025

9aman Jan 15, 2025

KKcorps Jan 14, 2025

9aman Jan 15, 2025

Jackie-Jiang left a comment

Jackie-Jiang Jan 14, 2025

9aman Jan 15, 2025

Jackie-Jiang Jan 14, 2025

9aman Jan 15, 2025

Jackie-Jiang Jan 14, 2025

9aman Jan 16, 2025

Jackie-Jiang Jan 14, 2025

9aman Jan 15, 2025

KKcorps commented Jan 15, 2025

codecov-commenter commented Jan 15, 2025 •

edited

Loading

Resolve failures pauseless ingestion #14798

Are you sure you want to change the base?

Resolve failures pauseless ingestion #14798

Conversation

9aman commented Jan 13, 2025

Pauseless Ingestion Failure Resolution

Summary

Failure Scenarios & Resolution Approaches

Recoverable Failures

Non-recoverable Failures

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jackie-Jiang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KKcorps commented Jan 15, 2025

codecov-commenter commented Jan 15, 2025 • edited Loading

Codecov Report

codecov-commenter commented Jan 15, 2025 •

edited

Loading