Upgrade all NuGet bits, and fix issues. And address #287 #294
Conversation
...crosoft.Azure.CosmosRepository.AspNetCore/Microsoft.Azure.CosmosRepository.AspNetCore.csproj
Show resolved
Hide resolved
src/Microsoft.Azure.CosmosRepository/Logging/LoggerMessageDefinitions.cs
Outdated
Show resolved
Hide resolved
|
|
||
| _logger.LogQueryConstructed(query); | ||
|
|
||
| IEnumerable<TItem>? items = await GetByQueryAsync( |
There was a problem hiding this comment.
Would this implementation, using a non-async enumerable approach to load all the records let's say 10 pages into memory, before then starting to stream them back to the client? Would we want to stream the first page back to the caller as soon as it's available to us?
There was a problem hiding this comment.
Yeah, that is a really good callout. I was trying to think of a more appropriate approach. Perhaps, I should refactor this to:
- Get the count first.
- Using the count determine a reasonable page size.
- Page over the data, then for each page yield it back.
That's probably better, right? Or do you have another idea?
There was a problem hiding this comment.
There was a problem hiding this comment.
Yeah that could work, but I am pretty sure running a count query across a large data set can rack up a hefty RU bill, @robertbennett1998 did you have the docs or article we read around this?
I am wondering whether we allow the caller to specify there chunk size, we can assume a reasonable default maybe?
There was a problem hiding this comment.
Hey, it has been a while but I found some threads discussing count before and it seemed to be quite an expensive operation and we probably shouldnt run it without the user explicitly knowing about it. This is the closest thread I can find atm
https://stackoverflow.com/questions/71958612/how-to-use-where-count-in-cosmos-db
But there was one that dug into it a bit deeper.
There was a problem hiding this comment.
I'm asking one of the PMs on the team, I'm not sure if there is any way to avoid this. Perhaps we just omit the count? /CC @jcocchi
There was a problem hiding this comment.
Could the continuation token being null let you know there are no more pages?
There was a problem hiding this comment.
I was trying to ask this question to @jcocchi. Let's ask her here, and see if this additional context helps to make the question more clear.
IAsyncEnumerabe PageAsyncAPI, fixes IAsyncEnumerable<TItem> Continuation Token Paging Implementation #197