Skip to content

Conversation

xinlian12
Copy link
Member

@xinlian12 xinlian12 commented Sep 24, 2025

Changes included:

  • Use change feed to get partition key ranges
  • Currently the throttlingRetryPolicy is applied across all pages to get partition key ranges, changed to apply for each page

@Copilot Copilot AI review requested due to automatic review settings September 24, 2025 05:16
@xinlian12 xinlian12 requested review from kirankumarkolli and a team as code owners September 24, 2025 05:16
Copy link
Contributor

@Copilot 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 enhances the partition key range fetching mechanism by switching from regular queries to change feed queries, improving diagnostics tracking, and modifying retry policy application.

Key changes:

  • Switch to change feed approach for fetching partition key ranges to enable incremental updates
  • Fix incorrect end time tracking in CosmosDiagnostics for PARTITION_KEY_RANGE_LOOK_UP operations
  • Apply throttling retry policy per page instead of across all pages

Reviewed Changes

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

Show a summary per file
File Description
InMemoryCollectionRoutingMap.java Added change feed continuation token support and logging
CollectionRoutingMap.java Extended interface to support change feed continuation tokens
RxPartitionKeyRangeCache.java Refactored to use change feed for partition key range fetching with proper diagnostics timing
RxDocumentClientImpl.java Modified to support change feed headers and per-request retry policy
Paginator.java Added change feed flag parameter
DocumentProducer.java Updated method call to include change feed flag
DefaultDocumentQueryExecutionContext.java Updated method call to include change feed flag
Multiple test files Updated method signatures and test data to accommodate new parameters

xinlian12 and others added 4 commits September 23, 2025 22:18
@xinlian12 xinlian12 force-pushed the useChangeFeedToGetPkRanges branch from e2eaac0 to ee9cba9 Compare September 24, 2025 06:02
@xinlian12
Copy link
Member Author

/azp run java - cosmos - tests

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@xinlian12
Copy link
Member Author

/azp run java - cosmos - tests

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Iterable<ImmutablePair<PartitionKeyRange, IServerIdentity>> ranges, String collectionUniqueId) {
Iterable<ImmutablePair<PartitionKeyRange, IServerIdentity>> ranges,
String collectionUniqueId,
String changeFeedNextIfNoneMatch) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't get this - worth going throguh in a call. For PKRanges map we have to guarantee that it is always a complete set - how could this variable ever be false? We would have to always drain all changes and only try to update the map at a piint when no more changes are left? Otherwise we could corrupt the map?

Copy link
Member Author

Choose a reason for hiding this comment

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

but this method is only called when we get all changes from backend

DocumentClientRetryPolicy retryPolicy = this.resetSessionTokenRetryPolicy.getRequestPolicy(null);
retryPolicy.onBeforeSendRequest(request);
return ObservableHelper.inlineIfPossibleAsObs(
() -> readFeed(request)
Copy link
Member

Choose a reason for hiding this comment

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

q: what is the reason ChangeFeedFetcher isn't used?

Copy link
Member Author

Choose a reason for hiding this comment

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

changeFeedFetcher is used for document fetches, it has too many document related logics, so I think a separate one for metadata will be much better

@xinlian12
Copy link
Member Author

/azp run java - cosmos - tests

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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.

3 participants