Change Feed: Fixes pull model request count for NotModified scenario

[ealsur]

Expectation

Assuming a FeedRange that spans multiple partitions/EPK ranges, for example, EPK Range 1, 2, and 3.

Change Feed pull model logic when receiving a NotModified should be:

1. Received 304 (NotModified) on EPK Range 1.
2. Check EPK Range 2, if received 304 (NotModified), then:
3. Check EPK Range 3, if received 304 (NotModified) then:
4. Return 304 (NotModified) to the user, since we tested all ranges and all are returning 304.

If the FeedRange spans a single partition/EPK range, for example, EPK Range 1.

Change Feed pull model logic when receiving a NotModified should be:

1. Received 304 (NotModified) on EPK Range 1.
2. Return 304 (NotModified) to the user, since the FeedRange does not span more ranges.

Current behavior

The current behavior is doing one extra request, that on the case of FeedRanges that span a single partition/EPK range, causes a 2x increase in number of requests.

Using the previous example, if the FeedRange spans EPK Range 1, 2, and 3, Change Feed pull model is doing:

1. Received 304 (NotModified) on EPK Range 1.
2. Check EPK Range 2, if received 304 (NotModified), then:
3. Check EPK Range 3, if received 304 (NotModified) then:
4. Check EPK Range 1, if received 304 (NotModified) then:
5. Return 304 (NotModified) to the user, since the last EPK Range matches the initial that returned 304.

If the FeedRange spans a single partition/EPK range, for example, EPK Range 1:

1. Received 304 (NotModified) on EPK Range 1.
2. Check EPK Range 1 again, if received 304 (NotModified) then:
3. Return 304 (NotModified) to the user, since the last EPK Range matches the initial that returned 304.

Logical fix

To solve it, we are now adding a TryPeekNext to the CrossPartitionRangePageAsyncEnumerator, which will peek on the internal queue and attempt to return the next state that would be read.

And doing 2 optimizations:

1. When getting a 304, if the Current range is the same as the Next range, it means the FeedRange spans 1 EPK Range, do nothing and return the 304. This reduces the number of requests to 1 on this scenario.
2. Call MoveNext on the Enumerator until the Next matches the initial one that received the first 304. This avoids the repeat on the scenario where the FeedRange spans multiple EPK ranges.

The PR contains tests to cover the logic and also updates the Contract tests to match what internal teams are actually really doing, the Contract tests got modified on previous PRs and diverged from what they were meant to cover.

Review tips

The 2 major changes in CrossPartitionChangeFeedAsyncEnumerator were:

From:

while (!(backendPage is ChangeFeedSuccessPage
                                || this.crossPartitionEnumerator.CurrentRange.Equals(originalRange)
                                || this.bufferedException.HasValue));

To:

while (!(backendPage is ChangeFeedSuccessPage
                                || IsNextRangeEqualToOriginal(this.crossPartitionEnumerator, originalRange)
                                || this.bufferedException.HasValue));

And adding a check for a single range scenario:

if (!IsNextRangeEqualToOriginal(this.crossPartitionEnumerator, originalRange))

Type of change

• Bug fix (non-breaking change which fixes an issue)

[kirankumarkolli]

Older Async enumerator contract seems reasonable.
Current/Next model is ambiguous and leads to mis-interpretations.

Can the looping be hardened with the current enumerator contract?

[FabianMeiswinkel]

LGTM