Issue where active document expiration during enumeration causes early return

[jrpavoncello]

I think I found an issue with the Redis.OM nuget package where enumerating the collection will return early if documents are actively expiring while enumerating.

Repro Steps:

You can find a somewhat minimum reproducible project here: https://github.com/jrpavoncello/RedisOMTestBed

On latest Redis Stack and Redis.OM, if you:

1. Set up an index (the POST /Person/CreateIndex endpoint in the test bed)

2. Insert 10,000 documents, let's call each document a Person, and set half of them to randomly expire in the next minute (/Person/CreateIndex would already have done this for you) Example:

   // Create an even distribution so that people are expiring in the middle of enumeration
   var expirationMilliseconds = Random.Shared.Next(msToCreateEveryPerson, msToCreateEveryPerson + msToAllowForTesting);
   await database.KeyExpireAsync($"{nameof(Person)}:{person.Id}", TimeSpan.FromMilliseconds(expirationMilliseconds));

3. Enumerate the whole index via something like: await foreach(var person in redisConnectionProvider.RedisCollection<Person>(2000)) until it's done enumerating (GET /Person/EnumeratorTest_GetFirstPerson in the test bed w/ default values)

4. Throw an exception if the number of documents returned is less than 4999 (10,000 / 2 - 1)

You will find that the exception gets thrown while documents are actively expiring. The enumerator will stop early with anywhere between 1800-1999 documents (i.e. not 10,000, and less than the chunk size).

I think I've found this is because the SearchResponse interprets those null documents returned by RedisSearch as (docArray.Length < 2) which skips the document so then RedisCollectionEnumerator interprets "documents count less than the chunk size" as "we're done enumerating, don't get the next chunk".

Once the documents are done actively expiring, Redis.OM will start working again it'll give you the expected 5000 documents every time.

Versions tested

Redis Stack version: Latest Redis.OM nuget package version: Latest (0.5.4)

Proposed Fix

Maybe SearchResponse could expose a property to allow RedisCollectionEnumerator to detect when any documents were skipped. If any documents were skipped, adjust the offsets as if all documents were read for that chunk, and grab the next chunk. If the next chunk is empty and no documents were skipped then we know we're done.

[slorello89]

Hi @jrpavoncello - thanks for opening this, your analysis is spot-on, RediSearch nulls out fields if they are caught up by the index and expired before they are actually materialized out of the index.

I think your solution is spot on too. Have a number of skipped documents and base whether to go and get the next chunk on that and the number of returned documents to the enumerator.

[jrpavoncello]

@slorello89 Awesome, thanks for the confirmation. Are you able to provide some kind of rough ETA on fixing this or where this falls priority-wise for your team? I'm certainly open to taking a shot at this if it wouldn't be able to be fixed in the next week or so, but I haven't started the effort yet.

[slorello89]

Will probably be greater than a week turnaround on this - always open to a PR if you have bandwidth :)

[slorello89]

@jrpavoncello, just pushed v0.5.5 - thanks for submitting!