Skip to content

[Internal] Change Feed: Adds Code to Read Globally Committed Data During PPAF Failovers#5379

Closed
kundadebdatta wants to merge 13 commits intomasterfrom
users/kundadebdatta/5376_ppaf_missing_primes
Closed

[Internal] Change Feed: Adds Code to Read Globally Committed Data During PPAF Failovers#5379
kundadebdatta wants to merge 13 commits intomasterfrom
users/kundadebdatta/5376_ppaf_missing_primes

Conversation

@kundadebdatta
Copy link
Copy Markdown
Member

@kundadebdatta kundadebdatta commented Aug 29, 2025

Pull Request Template

Background

Today, during a partition level automatic failover, there is a gap in the backend where for a change feed operation (incremental or full-fidelity) in a less than (<) strong consistency account, a false progress can cause data loss, since the change feed operation reads relies on the GLSN.

In a nut-shell a GLSN is unique for a given GCN, however, for multiple GCNs, there could be duplicate GLSNs (e.g. there could be a same GLSN0 for GCN1 and GCN2). In order to fix this, the SDK need to send the x-ms-cosmos-read-global-committed-data header with change feed operation so that the change feed can always read the globally committed data.

Description

This PR adds a new request header for Change Feed requests to read the globally committed data for less than strong consistency accounts.

  • x-ms-cosmos-read-global-committed-data is the header to read the globally committed data.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Closing issues

To automatically close an issue: closes #5376

@kundadebdatta kundadebdatta requested a review from Copilot August 29, 2025 19:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements support for reading globally committed data during partition-level automatic failover (PPAF) scenarios for Change Feed operations in less than strong consistency accounts. The key enhancement adds a new request header x-ms-cosmos-read-global-committed-data to prevent data loss during failovers.

  • Adds logic to detect PPAF scenarios and automatically include the global committed data header
  • Refactors consistency level validation to return the selected level for downstream use
  • Implements comprehensive test coverage for PPAF scenarios with different consistency levels

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
RequestInvokerHandler.cs Core logic to detect PPAF scenarios and add the global committed data header
ChangeFeedHelper.cs New utility method to determine when Change Feed can handle missing primes safely
MockDocumentClient.cs Mock enhancement to support consistency level testing
ChangeFeedIteratorCoreTests.cs Comprehensive test coverage for PPAF scenarios
EndToEndTraceWriterBaselineTests.MiscellanousAsync.xml Updated baseline removing client initialization trace

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread Microsoft.Azure.Cosmos/src/Handler/RequestInvokerHandler.cs Outdated
Comment thread Microsoft.Azure.Cosmos/src/ChangeFeedHelper.cs Outdated
@kundadebdatta kundadebdatta marked this pull request as ready for review August 29, 2025 19:41
@kundadebdatta kundadebdatta marked this pull request as draft August 29, 2025 19:42
@kundadebdatta kundadebdatta marked this pull request as ready for review August 29, 2025 23:36
@kundadebdatta kundadebdatta added the auto-merge Enables automation to merge PRs label Aug 29, 2025
Copy link
Copy Markdown
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Thanks

Comment thread Microsoft.Azure.Cosmos/src/ChangeFeedHelper.cs
Copy link
Copy Markdown
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - except for the feedback around having the consistency level condition.

NaluTripician
NaluTripician previously approved these changes Sep 2, 2025
Copy link
Copy Markdown
Contributor

@NaluTripician NaluTripician left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

Copy link
Copy Markdown
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Thanks!

@kundadebdatta
Copy link
Copy Markdown
Member Author

For short term, the new header is no longer required from the SDK. BE will go with the GCLSN based approach for change feeds, and the goal is for PPAF enabled accounts (irrespective of consistency levels), false progress changes will not be reported through CF.

Hence closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PPAF: Missing Primes - Add a New Request and Response Header to Read and Supply the GCN for Less Than Strong Accounts

4 participants