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.

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

Change Feed StartFrom.Now() not respected with multiple physical partitions #1918

Closed
abranaugh opened this issue Oct 8, 2020 · 6 comments · Fixed by #1933
Closed

Change Feed StartFrom.Now() not respected with multiple physical partitions #1918

abranaugh opened this issue Oct 8, 2020 · 6 comments · Fixed by #1933
Assignees
Labels
bug Something isn't working ChangeFeed customer-reported Issue created by a customer

Comments

@abranaugh
Copy link

abranaugh commented Oct 8, 2020

We are continuously addressing and improving the SDK, if possible, make sure the problem persist in the latest SDK version.

Describe the bug
We updated to the newest version of the change feed pull model, when we updated our code to use the new ChangeFeedStartFrom.Time() or ChangeFeedStartFrom.Now(), we received documents that were changed prior to the time we specified. We tested this with several different collections. ChangeFeedStartFrom.Time() and ChangeFeedStartFrom.Now() returned the expected number of documents when we ran against a collection with a single physical partition. We only are seeing this for collections with multiple physical partitions.

To Reproduce
We pulled the latest version of the SDK down, added the following test to the Microsoft.Azure.Cosmos.EmulatorTests.FeedRanges.ChangeFeedIteratorCoreTests file and pointed to our dev environment

[TestMethod]
public async Task ChangeFeedIteratorCore_StartTime_LargeContainer()
{
    int totalCount = 0;
    int batchSize = 25;
    await this.CreateRandomItems(this.LargerContainer, batchSize, randomPartitionKey: true);
    await Task.Delay(1000);
    DateTime now = DateTime.UtcNow;
    await Task.Delay(1000);
    await this.CreateRandomItems(this.LargerContainer, batchSize, randomPartitionKey: true);
    ContainerInternal itemsCore = this.LargerContainer;

    FeedIterator feedIterator = itemsCore.GetChangeFeedStreamIterator(
		ChangeFeedStartFrom.Time(now));
    while (feedIterator.HasMoreResults)
    {
        using (ResponseMessage responseMessage =
	    await feedIterator.ReadNextAsync(this.cancellationToken))
        {
            if (responseMessage.IsSuccessStatusCode)
            {
				Collection<ToDoActivity> response = TestCommon.SerializerCore.FromStream<CosmosFeedResponseUtil<ToDoActivity>>(responseMessage.Content).Data;
                totalCount += response.Count;
            }
        }
    }
	
    Assert.AreEqual(batchSize, totalCount);
}

Expected behavior
We were expecting the second set of 25 items that were inserted to be returned.

Actual behavior
We are seeing 42 items returned.

Environment summary
SDK Version: 3.13.0 - Preview and 3.13.0

@j82w j82w added bug Something isn't working ChangeFeed labels Oct 8, 2020
@j82w
Copy link
Contributor

j82w commented Oct 21, 2020

@bchong95 can you confirm this is fixed by the PR #1933 ?

@j82w
Copy link
Contributor

j82w commented Oct 21, 2020

This will be fixed in 3.15.0-preview which should be released in the next few days.

@j82w j82w closed this as completed Oct 21, 2020
@j82w
Copy link
Contributor

j82w commented Oct 22, 2020

@abranaugh please try the latest 3.15.0-preview

@j82w j82w added the customer-reported Issue created by a customer label Oct 22, 2020
@abranaugh
Copy link
Author

@j82w I tested out the change yesterday and it looks like it is working as expected now, I did have to make a few changes though to our tests.

In the original test that I posted, I had to modify the while loop to this:

    while (feedIterator.HasMoreResults)
    {
        using (ResponseMessage responseMessage =
	    await feedIterator.ReadNextAsync(this.cancellationToken))
        {
            if (!responseMessage.IsSuccessStatusCode)
            {
                break;
            }
	        Collection<ToDoActivity> response = TestCommon.SerializerCore.FromStream<CosmosFeedResponseUtil<ToDoActivity>>(responseMessage.Content).Data;
                totalCount += response.Count;
            }
        }
    }

It looked like the HasMoreResults never flipped to false, even after I have received all of the documents in the container. Instead, the responseMessage had a 304 and the IsSuccessStatusCode was false. I also ran a test against the same container, and used ChangeFeedStartFrom.Now(). I didn't add anything to the container in this test, so there should have been no changes to retrieve, and in this case the HasMoreResults is still true. Based on the description of the HasMoreResults property on the Feed Iterator, I would expect that to be false when there aren't any more documents to be processed.

@j82w
Copy link
Contributor

j82w commented Oct 23, 2020

@bchong95 please take a look at the above feedback since you made the change.

@ghost
Copy link

ghost commented Dec 15, 2021

Closing due to in-activity, pease feel free to re-open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ChangeFeed customer-reported Issue created by a customer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants