Skip to content
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

Jump to bottom

Internal PartitionKeyRangeHandler : Adds code to expose bug #1937

Closed
wants to merge 2 commits into from
Closed

Internal PartitionKeyRangeHandler : Adds code to expose bug #1937

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2f205b
exposes split bug
bchong95 Oct 15, 2020
b6f2f0e
Merge branch 'master' into users/brchon/ExposePartitionKeyRangeHandle…
bchong95 Oct 16, 2020
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
8 changes: 3 additions & 5 deletions Microsoft.Azure.Cosmos/src/Resource/FeedIterators/FeedRangeIteratorCore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,16 +182,14 @@ private async Task<ResponseMessage> ReadNextInternalAsync(
streamPayload: stream,
requestEnricher: request =>
{
// FeedRangeContinuationRequestMessagePopulatorVisitor needs to run before FeedRangeRequestMessagePopulatorVisitor,
// since they both set EPK range headers and in the case of split we need to run on the child range and not the parent range.
FeedRangeRequestMessagePopulatorVisitor feedRangeVisitor = new FeedRangeRequestMessagePopulatorVisitor(request);
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 think this is a bug in PartitionKeyRangeHandler, but rather the way FeedRange and FeedRangeContinuatoin work.

FeedRange describes the current range, but due to the backend current design, when a split happens, FeedRangeContinuation will have independent continuations for subranges (each representing one partition). So the order of precedence should be FeedRangeContinuation first (in case it has a subrange selection aftersplit) and then FeedRange after.

So the EPK range sent through the wire is the one defined by FeedRangeContinuation.

When the backend supports EPK filtering, the FeedRangeContinuation can then be simply the Etag continuation, without any subrange list, so it won't set the EPK range. The range will then be set by the FeedRange visitor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you read the source code it is clear that the code is reading the epk range to try and route to the correct partition. This code existed well before FeedRange and FeedRangeContinuation, so I am not sure why you are mentioning these things.

this.FeedRangeInternal.Accept(feedRangeVisitor);

FeedRangeContinuationRequestMessagePopulatorVisitor feedRangeContinuationVisitor = new FeedRangeContinuationRequestMessagePopulatorVisitor(
request,
QueryRequestOptions.FillContinuationToken);
this.FeedRangeContinuation.Accept(feedRangeContinuationVisitor);

FeedRangeRequestMessagePopulatorVisitor feedRangeVisitor = new FeedRangeRequestMessagePopulatorVisitor(request);
this.FeedRangeInternal.Accept(feedRangeVisitor);

if (this.querySpec != null)
{
request.Headers.Add(HttpConstants.HttpHeaders.ContentType, MediaTypes.QueryJson);
Expand Down