-
Notifications
You must be signed in to change notification settings - Fork 815
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
Replace Cosmos Db metadata query with container query #2215
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,23 +31,21 @@ public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context | |
{ | ||
try | ||
{ | ||
if (_options is {DatabaseId: not null, ContainerIds: not null}) | ||
{ | ||
foreach (var containerId in _options.ContainerIds) | ||
{ | ||
var container = _cosmosClient.GetContainer(_options.DatabaseId, containerId); | ||
var query = new QueryDefinition($"SELECT 1 FROM {containerId}"); | ||
using var queryStream = container.GetItemQueryStreamIterator(query); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't you need to call .ReadNextAsync() to execute the operation against the container? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @idkburkes is correct: This is correct. The query is not executed until you call await ReadNextAsync() |
||
} | ||
} | ||
await _cosmosClient.ReadAccountAsync().ConfigureAwait(false); | ||
|
||
if (_options.DatabaseId != null) | ||
{ | ||
var database = _cosmosClient.GetDatabase(_options.DatabaseId); | ||
await database.ReadAsync(cancellationToken: cancellationToken).ConfigureAwait(false); | ||
|
||
if (_options.ContainerIds != null) | ||
{ | ||
foreach (var container in _options.ContainerIds) | ||
{ | ||
await database | ||
.GetContainer(container) | ||
.ReadContainerAsync(cancellationToken: cancellationToken) | ||
.ConfigureAwait(false); | ||
} | ||
} | ||
} | ||
|
||
return HealthCheckResult.Healthy(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leminh98 @Maya-Painter @sboshra @adityasa could someone from the CosmosDB Team please review this change?
Context: this code is executed quite frequently, from an ASP.NET health check which has almost 6m downloads on nuget.org: https://www.nuget.org/packages/AspNetCore.HealthChecks.CosmosDb
BTW we are soon going to implement #2325 and hopefully reduce the number of users of Microsoft.Azure.DocumentDB.Core package