Skip to content

[Internal] Thin Client Integration: Adds changefeed support in thinclient mode #5720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kirankumarkolli merged 2 commits into from
Apr 1, 2026
+323 −40
Merged

[Internal] Thin Client Integration: Adds changefeed support in thinclient mode #5720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69e105b
Add changefeed support in thinclient mode
aavasthy Mar 25, 2026
9007564
Address PR review: positive A-IM matching, better doc comment, improv…
Copilot Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 48 additions & 25 deletions Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,31 +552,54 @@ internal static bool IsStoredProcedureCrudOperation(
operationType != Documents.OperationType.ExecuteJavaScript;
}

internal static bool IsOperationSupportedByThinClient(DocumentServiceRequest request)
{
// Document operations
if (request.ResourceType == ResourceType.Document
&& (request.OperationType == OperationType.Batch
|| request.OperationType == OperationType.Patch
|| request.OperationType == OperationType.Create
|| request.OperationType == OperationType.Read
|| request.OperationType == OperationType.Upsert
|| request.OperationType == OperationType.Replace
|| request.OperationType == OperationType.Delete
|| request.OperationType == OperationType.Query
|| request.OperationType == OperationType.QueryPlan))
{
return true;
}

// Stored Procedure execution
if (request.ResourceType == ResourceType.StoredProcedure
&& request.OperationType == OperationType.ExecuteJavaScript)
{
return true;
}

return false;
internal static bool IsOperationSupportedByThinClient(DocumentServiceRequest request)
{
// Document operations
if (request.ResourceType == ResourceType.Document
&& (request.OperationType == OperationType.Batch
|| request.OperationType == OperationType.Patch
|| request.OperationType == OperationType.Create
|| request.OperationType == OperationType.Read
|| request.OperationType == OperationType.Upsert
|| request.OperationType == OperationType.Replace
|| request.OperationType == OperationType.Delete
|| request.OperationType == OperationType.Query
|| request.OperationType == OperationType.QueryPlan))
{
return true;
}

// LatestVersion (Incremental) ChangeFeed on documents.
Comment thread
aavasthy marked this conversation as resolved.
// AllVersionsAndDeletes (FullFidelity) is excluded because it requires
// split-handling logic in Compute Gateway (UseGatewayMode is set by ChangeFeedModeFullFidelity).
if (request.ResourceType == ResourceType.Document
&& request.OperationType == OperationType.ReadFeed
&& GatewayStoreModel.IsLatestVersionChangeFeedRequest(request))
Comment thread
aavasthy marked this conversation as resolved.
{
return true;
}

// Stored Procedure execution
if (request.ResourceType == ResourceType.StoredProcedure
&& request.OperationType == OperationType.ExecuteJavaScript)
{
return true;
}

return false;
}

/// <summary>
/// Determines if the request is a LatestVersion (Incremental) change feed request that can
/// be routed to the thin client. Returns true only when the A-IM header is exactly
/// <c>HttpConstants.A_IMHeaderValues.IncrementalFeed</c>. Any other value — including
/// Full-Fidelity Feed (AllVersionsAndDeletes) or an unknown future mode — falls back to
/// Compute Gateway so that new modes are not accidentally routed to the thin client.
/// </summary>
internal static bool IsLatestVersionChangeFeedRequest(DocumentServiceRequest request)
Copy link
Copy Markdown
Contributor

@NaluTripician NaluTripician Mar 26, 2026

Choose a reason for hiding this comment

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

🟡 Recommendation · Correctness: Fragile Negation Logic

Prefer positive matching for IncrementalFeed over negation of FullFidelityFeed

This method checks !string.Equals(aImHeaderValue, FullFidelityFeed) instead of positively matching string.Equals(aImHeaderValue, IncrementalFeed). While the doc comment says 'LatestVersion uses Incremental feed', the code accepts any A-IM value that isn't FullFidelityFeed.

Why it matters: If a third change feed mode is added with a new A-IM value (e.g., Streaming-Feed), it would be automatically routed to thin client without anyone explicitly validating thin client support. The failure mode could be errors or incorrect results. Using the positive check makes new modes default to gateway (the safe path) until explicitly added.

Currently there are only two A-IM values (IncrementalFeed and FullFidelityFeed per ChangeFeedModeIncremental and ChangeFeedModeFullFidelity), so this is correct today — but fragile for tomorrow.

Suggested change:

return string.Equals(aImHeaderValue, HttpConstants.A_IMHeaderValues.IncrementalFeed, StringComparison.OrdinalIgnoreCase);

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

Copy link
Copy Markdown
Contributor

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

Switched to positive matching in commit 9007564. The method now returns string.Equals(aImHeaderValue, HttpConstants.A_IMHeaderValues.IncrementalFeed, StringComparison.OrdinalIgnoreCase), so any unknown future A-IM value defaults to the gateway (safe path).

Copy link
Copy Markdown
Contributor

@NaluTripician NaluTripician Mar 26, 2026

Choose a reason for hiding this comment

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

🟢 Suggestion · Naming: Method Name vs. Implementation Mismatch

Method name implies positive check but implementation is negative

IsLatestVersionChangeFeedRequest suggests a positive check for LatestVersion, but the implementation is a negative check (not FullFidelity). If Finding #1 is addressed (positive matching for IncrementalFeed), this mismatch resolves naturally. If the negation is kept intentionally, consider renaming to IsNonFullFidelityChangeFeedRequest() to match the actual semantics.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

Copy link
Copy Markdown
Contributor

@Praveen-Msft Praveen-Msft Mar 27, 2026

Choose a reason for hiding this comment

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

This is a good suggestion, Possibly IsFullFidelityChangeFeedRequest will be better given the functionality

Copy link
Copy Markdown
Contributor

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

With the switch to positive matching (== IncrementalFeed), IsLatestVersionChangeFeedRequest now accurately reflects what the method checks, so the naming mismatch is resolved naturally.

{
string aImHeaderValue = request.Headers[HttpConstants.HttpHeaders.A_IM];
Comment thread
aavasthy marked this conversation as resolved.
return string.Equals(aImHeaderValue, HttpConstants.A_IMHeaderValues.IncrementalFeed, StringComparison.OrdinalIgnoreCase);
}
private async Task<AccountProperties> GetDatabaseAccountPropertiesAsync()
{
Expand Down
48 changes: 47 additions & 1 deletion ...soft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemThinClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class CosmosItemThinClientTests
[TestInitialize]
public async Task TestInitAsync()
{
Environment.SetEnvironmentVariable(ConfigurationManager.ThinClientModeEnabled, "True");
Environment.SetEnvironmentVariable(ConfigurationManager.ThinClientModeEnabled, "True");
this.connectionString = Environment.GetEnvironmentVariable("COSMOSDB_THINCLIENT");

if (string.IsNullOrEmpty(this.connectionString))
Expand Down Expand Up @@ -1354,6 +1354,52 @@ public async Task TestThinClientQueryPlanMultiPartitionFanout()
}
}

[TestMethod]
[TestCategory("ThinClient")]
public async Task TestThinClientChangeFeedLatestVersionAsync()
Copy link
Copy Markdown
Contributor

@NaluTripician NaluTripician Mar 26, 2026

Choose a reason for hiding this comment

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

🟢 Suggestion · Testing: Add Emulator Test for AllVersionsAndDeletes

Consider adding a companion emulator test for FullFidelity change feed

This test verifies LatestVersion change feed routes through thin client by checking |F4 in diagnostics. A companion test verifying AllVersionsAndDeletes does not contain |F4 would increase confidence that the header-based routing works end-to-end in the real stack.

Unit tests already cover the FullFidelity → gateway path, so this is a nice-to-have for integration confidence.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

{
// Arrange:
string pk = "pk_changefeed";
List<TestObject> items = this.GenerateItems(pk).Take(10).ToList();
List<TestObject> createdItems = await this.CreateItemsSafeAsync(items);

Assert.IsTrue(createdItems.Count > 0, "At least one item must be created for the change feed test.");

// Act: Read change feed using LatestVersion mode
List<TestObject> changeFeedResults = new List<TestObject>();
FeedIterator<TestObject> changeFeedIterator = this.container.GetChangeFeedIterator<TestObject>(
ChangeFeedStartFrom.Beginning(),
ChangeFeedMode.LatestVersion,
new ChangeFeedRequestOptions()
{
PageSizeHint = 10
});

while (changeFeedIterator.HasMoreResults)
{
FeedResponse<TestObject> response = await changeFeedIterator.ReadNextAsync();

if (response.StatusCode == HttpStatusCode.NotModified)
{
break;
}

string diagnostics = response.Diagnostics.ToString();
Assert.IsTrue(diagnostics.Contains("|F4"), "Diagnostics User Agent should contain '|F4' for ThinClient change feed");

changeFeedResults.AddRange(response);
}

// Assert: Verify all created items appear in the change feed
Assert.IsTrue(changeFeedResults.Count >= createdItems.Count,
$"Change feed should return at least {createdItems.Count} items but got {changeFeedResults.Count}.");

HashSet<string> createdIds = new HashSet<string>(createdItems.Select(i => i.Id));
HashSet<string> changeFeedIds = new HashSet<string>(changeFeedResults.Select(i => i.Id));
Assert.IsTrue(createdIds.IsSubsetOf(changeFeedIds),
"All created items should appear in the change feed results.");
}

/// <summary>
/// DelegatingHandler that intercepts HTTP requests and can inject faults
/// </summary>
Expand Down
Loading
Loading