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

Fixed a bug that caused sortedRanges exception #835

Merged
merged 3 commits into from
Sep 24, 2019
Merged

Fixed a bug that caused sortedRanges exception #835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8efa038
Adding repo test
Sep 20, 2019
4553e88
There was a bug where the SDK could not handle collisions in the effe…
Sep 20, 2019
467b51c
Updated changelog
Sep 20, 2019
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
129 changes: 77 additions & 52 deletions Microsoft.Azure.Cosmos/src/Routing/IRoutingMapProviderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Microsoft.Azure.Cosmos
{
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.Azure.Cosmos.Routing;
Expand All @@ -19,22 +20,43 @@ private static string Max(string left, string right)
return StringComparer.Ordinal.Compare(left, right) < 0 ? right : left;
}

private static bool IsSortedAndNonOverlapping<T>(IList<Range<T>> list)
public static async Task<PartitionKeyRange> TryGetRangeByEffectivePartitionKeyAsync(
this IRoutingMapProvider routingMapProvider,
string collectionResourceId,
string effectivePartitionKey)
{
IReadOnlyList<PartitionKeyRange> ranges = await routingMapProvider.TryGetOverlappingRangesAsync(
collectionResourceId,
Range<string>.GetPointRange(effectivePartitionKey));

if (ranges == null)
{
return null;
}

return ranges.Single();
}

private static bool IsNonOverlapping<T>(SortedSet<Range<T>> sortedSet)
where T : IComparable<T>
{
IComparer<T> comparer = typeof(T) == typeof(string) ? (IComparer<T>)StringComparer.Ordinal : Comparer<T>.Default;

for (int i = 1; i < list.Count; i++)
IEnumerable<Range<T>> withoutLast = sortedSet.Take(sortedSet.Count - 1);
IEnumerable<Range<T>> withoutFirst = sortedSet.Skip(1);

IEnumerable<Tuple<Range<T>, Range<T>>> currentAndNexts = withoutLast.Zip(withoutFirst, (current, next) => new Tuple<Range<T>, Range<T>>(current, next));
foreach (Tuple<Range<T>, Range<T>> currentAndNext in currentAndNexts)
{
Range<T> previousRange = list[i - 1];
Range<T> currentRange = list[i];
Range<T> current = currentAndNext.Item1;
Range<T> next = currentAndNext.Item2;

int compareResult = comparer.Compare(previousRange.Max, currentRange.Min);
int compareResult = comparer.Compare(current.Max, next.Min);
if (compareResult > 0)
{
return false;
}
else if (compareResult == 0 && previousRange.IsMaxInclusive && currentRange.IsMinInclusive)
else if (compareResult == 0 && current.IsMaxInclusive && next.IsMinInclusive)
{
return false;
}
Expand All @@ -43,82 +65,85 @@ private static bool IsSortedAndNonOverlapping<T>(IList<Range<T>> list)
return true;
}

public static async Task<PartitionKeyRange> TryGetRangeByEffectivePartitionKeyAsync(
public static async Task<List<PartitionKeyRange>> TryGetOverlappingRangesAsync(
this IRoutingMapProvider routingMapProvider,
string collectionResourceId,
string effectivePartitionKey)
IEnumerable<Range<string>> sortedRanges,
bool forceRefresh = false)
{
IReadOnlyList<PartitionKeyRange> ranges = await routingMapProvider.TryGetOverlappingRangesAsync(
collectionResourceId,
Range<string>.GetPointRange(effectivePartitionKey));

if (ranges == null)
if (sortedRanges == null)
{
return null;
throw new ArgumentNullException(nameof(sortedRanges));
}

return ranges.Single();
}
// Remove the duplicates
SortedSet<Range<string>> distinctSortedRanges = new SortedSet<Range<string>>(sortedRanges, Range<string>.MinComparer.Instance);

public static async Task<List<PartitionKeyRange>> TryGetOverlappingRangesAsync(
this IRoutingMapProvider routingMapProvider,
string collectionResourceId,
IList<Range<string>> sortedRanges,
bool forceRefresh = false)
{
if (!IsSortedAndNonOverlapping(sortedRanges))
// Make sure there is no overlap
if (!IRoutingMapProviderExtensions.IsNonOverlapping(distinctSortedRanges))
{
throw new ArgumentException("sortedRanges");
throw new ArgumentException($"{nameof(sortedRanges)} had overlaps.");
}

// For each range try to figure out what PartitionKeyRanges it spans.
List<PartitionKeyRange> targetRanges = new List<PartitionKeyRange>();
int currentProvidedRange = 0;
while (currentProvidedRange < sortedRanges.Count)
foreach (Range<string> range in sortedRanges)
{
if (sortedRanges[currentProvidedRange].IsEmpty)
// if the range is empty then it by definition does not span any ranges.
if (range.IsEmpty)
{
currentProvidedRange++;
continue;
}

Range<string> queryRange;
if (targetRanges.Count > 0)
// If current range already is covered by the most recently added PartitionKeyRange, then we can skip it
// (to avoid duplicates).
if ((targetRanges.Count != 0) && (Range<string>.MaxComparer.Instance.Compare(range, targetRanges.Last().ToRange()) <= 0))
{
string left = Max(
targetRanges[targetRanges.Count - 1].MaxExclusive,
sortedRanges[currentProvidedRange].Min);

bool leftInclusive = string.CompareOrdinal(left, sortedRanges[currentProvidedRange].Min) == 0
? sortedRanges[currentProvidedRange].IsMinInclusive
: false;
continue;
}

queryRange = new Range<string>(
left,
sortedRanges[currentProvidedRange].Max,
leftInclusive,
sortedRanges[currentProvidedRange].IsMaxInclusive);
// Calculate what range to look up.
Range<string> queryRange;
if (targetRanges.Count == 0)
{
// If there are no existing partition key ranges,
// then we take the first to get the ball rolling.
queryRange = range;
}
else
{
queryRange = sortedRanges[currentProvidedRange];
// We don't want to double count a partition key range
// So we form a new range where
// * left of the range is Max(lastPartitionKeyRange.Right(), currentRange.Left())
// * right is just the right of the currentRange.
// That way if the current range overlaps with the partition key range it won't double count it when doing:
// routingMapProvider.TryGetOverlappingRangesAsync
string left = IRoutingMapProviderExtensions.Max(targetRanges.Last().MaxExclusive, range.Min);
bool leftInclusive = string.CompareOrdinal(left, range.Min) == 0 ? range.IsMinInclusive : false;
queryRange = new Range<string>(
left,
range.Max,
leftInclusive,
range.IsMaxInclusive);
}

IReadOnlyList<PartitionKeyRange> overlappingRanges =
await routingMapProvider.TryGetOverlappingRangesAsync(collectionResourceId, queryRange, forceRefresh);
IReadOnlyList<PartitionKeyRange> overlappingRanges = await routingMapProvider.TryGetOverlappingRangesAsync(
collectionResourceId,
queryRange,
forceRefresh);

if (overlappingRanges == null)
{
// null means we weren't able to find the overlapping ranges.
// This is due to a stale cache.
// It is the caller's responsiblity to recall this method with forceRefresh = true
return null;

// Design note: It would be better if this method just returned a bool and followed the standard TryGet Pattern.
// It would be even better to remove the forceRefresh flag just replace it with a non TryGet method call.
}

targetRanges.AddRange(overlappingRanges);

Range<string> lastKnownTargetRange = targetRanges[targetRanges.Count - 1].ToRange();
while (currentProvidedRange < sortedRanges.Count &&
Range<string>.MaxComparer.Instance.Compare(sortedRanges[currentProvidedRange], lastKnownTargetRange) <= 0)
{
currentProvidedRange++;
}
}

return targetRanges;
Expand Down
106 changes: 88 additions & 18 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosBasicQueryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
namespace Microsoft.Azure.Cosmos.SDK.EmulatorTests
{
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using System.Linq;
using System.Net;
Expand Down Expand Up @@ -170,7 +168,7 @@ public async Task ItemTest(bool directMode)

if (queryResults.Count < 3)
{
foreach(string id in createdIds)
foreach (string id in createdIds)
{
dynamic item = new
{
Expand Down Expand Up @@ -203,15 +201,15 @@ public async Task ItemTest(bool directMode)
CollectionAssert.IsSubsetOf(createdIds, ids);

//Read All with partition key
results = await this.ToListAsync(
container.GetItemQueryStreamIterator,
container.GetItemQueryIterator<dynamic>,
null,
new QueryRequestOptions()
{
MaxItemCount = 1,
PartitionKey = new PartitionKey("BasicQueryItem")
});
results = await this.ToListAsync(
container.GetItemQueryStreamIterator,
container.GetItemQueryIterator<dynamic>,
null,
new QueryRequestOptions()
{
MaxItemCount = 1,
PartitionKey = new PartitionKey("BasicQueryItem")
});

Assert.AreEqual(1, results.Count);

Expand Down Expand Up @@ -267,9 +265,9 @@ public async Task ScriptsStoredProcedureTest(bool directMode)
"select * from T where STARTSWITH(T.id, \"BasicQuerySp\")",
CosmosBasicQueryTests.RequestOptions);

if(queryResults.Count < 3)
if (queryResults.Count < 3)
{
foreach(string id in createdIds)
foreach (string id in createdIds)
{
StoredProcedureProperties properties = await scripts.CreateStoredProcedureAsync(new StoredProcedureProperties()
{
Expand Down Expand Up @@ -407,7 +405,7 @@ public async Task ScriptsTriggerTest(bool directMode)
public async Task UserTests(bool directMode)
{
CosmosClient client = directMode ? DirectCosmosClient : GatewayCosmosClient;
DatabaseCore database = (DatabaseCore) client.GetDatabase(DatabaseId);
DatabaseCore database = (DatabaseCore)client.GetDatabase(DatabaseId);
List<string> createdIds = new List<string>();

try
Expand Down Expand Up @@ -466,7 +464,7 @@ public async Task PermissionTests(bool directMode)
{
UserResponse createUserResponse = await database.CreateUserAsync(userId);
Assert.AreEqual(HttpStatusCode.Created, createUserResponse.StatusCode);
user = (UserCore) createUserResponse.User;
user = (UserCore)createUserResponse.User;

ContainerResponse createContainerResponse = await database.CreateContainerIfNotExistsAsync(Guid.NewGuid().ToString(), partitionKeyPath: "/pk");
Container container = createContainerResponse.Container;
Expand Down Expand Up @@ -525,8 +523,8 @@ public async Task PermissionTests(bool directMode)
private delegate FeedIterator QueryStream(string querytext, string continuationToken, QueryRequestOptions options);

private async Task<List<T>> ToListAsync<T>(
QueryStream createStreamQuery,
Query<T> createQuery,
QueryStream createStreamQuery,
Query<T> createQuery,
string queryText,
QueryRequestOptions requestOptions)
{
Expand Down Expand Up @@ -614,5 +612,77 @@ private async Task<List<T>> ToListAsync<T>(

return results;
}

public const string DatabaseName = "testcosmosclient";
public const int Throughput = 1200;
public const string DefaultKey = "objectKey";
public const string TestCollection = "testcollection";
private static readonly Random Random = new Random();

[TestMethod]
public async Task InvalidRangesOnQuery()
{
CosmosClient cosmosClient = DirectCosmosClient;

DatabaseResponse databaseResponse = await cosmosClient.CreateDatabaseIfNotExistsAsync(DatabaseName, Throughput);
Database database = databaseResponse.Database;

Container container = await database.DefineContainer(TestCollection, $"/{DefaultKey}")
.WithUniqueKey().Path($"/{DefaultKey}").Attach().CreateIfNotExistsAsync();

List<string> queryKeys = new List<string>();

List<TestCollectionObject> testCollectionObjects = JsonConvert.DeserializeObject<List<TestCollectionObject>>(
"[{\"id\":\"70627503-7cb2-4a79-bcec-5e55765aa080\",\"objectKey\":\"message~phone~u058da564bfaa66cb031606db664dbfda~phone~ud75ce020af5f8bfb75a9097a66d452f2~Chat~20190927000042Z\",\"text\":null,\"text2\":null},{\"id\":\"507079b7-a5be-4da4-9158-16fc961cd474\",\"objectKey\":\"message~phone~u058da564bfaa66cb031606db664dbfda~phone~ud75ce020af5f8bfb75a9097a66d452f2~Chat~20190927125742Z\",\"text\":null,\"text2\":null}]");
foreach (TestCollectionObject testCollectionObject in testCollectionObjects)
{
await WriteDocument(container, testCollectionObject);
queryKeys.Add(testCollectionObject.ObjectKey);
}

List<TestCollectionObject> results = container
.GetItemLinqQueryable<TestCollectionObject>(true, requestOptions: RunInParallelOptions())
.Where(r => queryKeys.Contains(r.ObjectKey))
.ToList(); // ERROR OCCURS WHEN QUERY IS EXECUTED

Console.WriteLine($"[\"{string.Join("\", \n\"", results.Select(r => r.ObjectKey))}\"]");
}

private static async Task WriteDocument(Container container, TestCollectionObject testData)
{
try
{
await container.CreateItemAsync(testData, requestOptions: null);
}
catch (CosmosException e)
{
if (e.StatusCode != HttpStatusCode.Conflict)
{
throw;
}
}
}

private static QueryRequestOptions RunInParallelOptions()
{
return new QueryRequestOptions
{
MaxItemCount = -1,
MaxBufferedItemCount = -1,
MaxConcurrency = -1
};
}
}

public class TestCollectionObject
{
[JsonProperty("id")]
public Guid Id { get; set; }
[JsonProperty(CosmosBasicQueryTests.DefaultKey)]
public string ObjectKey { get; set; }
[JsonProperty("text")]
public string Text { get; set; }
[JsonProperty("text2")]
public string Text2 { get; set; }
}
}
Loading