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

Subpartitioning: Fixes handling of split physical partitions #3879

Merged
10 commits merged into from
Jun 12, 2023
Merged

Subpartitioning: Fixes handling of split physical partitions #3879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
af580e9
Initial Commit DO NOT REVIEW
NaluTripician May 19, 2023
ef246ba
bug fix, needs Direct Package Changes
NaluTripician May 25, 2023
7c1dad1
Merge branch 'master' of https://github.com/Azure/azure-cosmos-dotnet…
NaluTripician May 30, 2023
afa3228
fix for change feed and query plus tests
NaluTripician May 30, 2023
4eb9c14
clean up
NaluTripician May 30, 2023
cc82322
query + clean up
NaluTripician Jun 8, 2023
b5a5a23
Merge branch 'master' into users/nalutripician/subpartitioningChangeF…
NaluTripician Jun 8, 2023
8ec68c7
Merge branch 'master' into users/nalutripician/subpartitioningChangeF…
NaluTripician Jun 9, 2023
a03832c
Merge branch 'master' into users/nalutripician/subpartitioningChangeF…
kirankumarkolli Jun 10, 2023
1c4dfd5
Merge branch 'master' into users/nalutripician/subpartitioningChangeF…
NaluTripician Jun 12, 2023
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
11 changes: 9 additions & 2 deletions Microsoft.Azure.Cosmos/src/FeedRange/FeedRanges/FeedRangePartitionKey.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

namespace Microsoft.Azure.Cosmos
{
using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Azure.Cosmos.Routing;
using Microsoft.Azure.Cosmos.Tracing;
using Microsoft.Azure.Documents;

/// <summary>
/// FeedRange that represents an exact Partition Key value.
Expand All @@ -31,8 +33,13 @@ public FeedRangePartitionKey(PartitionKey partitionKey)
return Task.FromResult(
new List<Documents.Routing.Range<string>>
{
Documents.Routing.Range<string>.GetPointRange(
this.PartitionKey.InternalKey.GetEffectivePartitionKeyString(partitionKeyDefinition))
Documents.Routing.PartitionKeyInternal.GetEffectivePartitionKeyRange(
partitionKeyDefinition,
new Documents.Routing.Range<Documents.Routing.PartitionKeyInternal>(
min: this.PartitionKey.InternalKey,
max: this.PartitionKey.InternalKey,
isMinInclusive: true,
isMaxInclusive: true))
Copy link
Member

Choose a reason for hiding this comment

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

Is this expected?

Copy link
Member

Choose a reason for hiding this comment

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

Its already answered for Matias, need to spend little more time.

Copy link
Member

Choose a reason for hiding this comment

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

@kirankumarkolli - the question above is whether it is expected to use a range with min and max inclusive? I think for a PartitionKeyFeedRange it has always been the case - but it is a good question - in Java we normalize to EPKs (always Min inclusive / max Exclusive) in query, changefeed etc. and wherever we need EPKs. And we also created a helper function to create a NormalizedEPK (with min Inclusive/max Exclusive) even publicly in the Spark Connector. So, agreed - would be good to discuss whether we should just always use normalized EPK.

Copy link
Member

Choose a reason for hiding this comment

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

A PK value however (single value) it is min/max inclusive with the min/max being the PK value hash

Copy link
Member

Choose a reason for hiding this comment

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

@ealsur that's my take-away from last Nalu update.
Where-as EPK -> Physical partition ownership is [Min, Max).

Copy link

Choose a reason for hiding this comment

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

@kirankumarkolli @ealsur Hi, was the conclusion about this that it is OK, or did it somehow slip through the review?

We are experiencing issue with the same symptoms that are discussed in this PR in the latest version of the SDK (3.38.1) and in #3934 (SDK stuck in an infinite loop of updating pkranges after partition split) and I can see in the current code of the SDK that both are still inclusive.

I can also see the same thing in CosmosQueryClientCore.GetCachedContainerQueryPropertiesAsync

image

image

We are getting 410 Gone from the pkranges requests after partition split happened on our DB which puts the SDK into an infinite loop.

So I am wondering if it could be caused by this

We will create a new issue for it, just collecting any possibly related facts... please let me know if this is a bad clue or a good one

});
}

Expand Down
6 changes: 1 addition & 5 deletions Microsoft.Azure.Cosmos/src/Query/Core/QueryPlan/QueryPartitionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,7 @@ internal PartitionedQueryExecutionInfo ConvertPartitionedQueryExecutionInfo(
List<Documents.Routing.Range<string>> effectiveRanges = new List<Documents.Routing.Range<string>>(queryInfoInternal.QueryRanges.Count);
foreach (Documents.Routing.Range<PartitionKeyInternal> internalRange in queryInfoInternal.QueryRanges)
{
effectiveRanges.Add(new Documents.Routing.Range<string>(
internalRange.Min.GetEffectivePartitionKeyString(partitionKeyDefinition, false),
internalRange.Max.GetEffectivePartitionKeyString(partitionKeyDefinition, false),
internalRange.IsMinInclusive,
internalRange.IsMaxInclusive));
effectiveRanges.Add(PartitionKeyInternal.GetEffectivePartitionKeyRange(partitionKeyDefinition, internalRange));
}

effectiveRanges.Sort(Documents.Routing.Range<string>.MinComparer.Instance);
Expand Down
Loading