Improve performance#57
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves read/query performance for check status displays by denormalizing the “latest check result” fields onto the Checks table and updating query paths to use those columns instead of querying/sorting CheckResults.
Changes:
- Add denormalized latest-result columns to
Checks(migration + entity/config updates) and backfill them from existingCheckResults. - Update
WorkspaceQueryServiceandCheckQueryServicelist queries to readLatest*columns rather than computing latest results viaCheckResults. - Update
CheckExecutorJobto maintain the denormalized columns and extend integration tests to validate the behavior.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| SAMA.Web/Services/Queries/WorkspaceQueryService.cs | Uses Check.LatestStatus/LatestCheckedAt for workspace status counts instead of querying CheckResults. |
| SAMA.Web/Services/Queries/CheckQueryService.cs | Projects Latest* columns directly into list view models, removing the extra “latest results” query. |
| SAMA.Web/Services/CheckExecutorJob.cs | Updates Check.Latest* columns when persisting a new CheckResult. |
| SAMA.Web/SAMA.Web.csproj | Bumps several runtime dependencies (Quartz, EF health checks, MailKit, etc.). |
| SAMA.Tests.Unit/SAMA.Tests.Unit.csproj | Updates test tooling packages (coverlet, MSTest). |
| SAMA.Tests.System/SAMA.Tests.System.csproj | Updates MSTest package version. |
| SAMA.Tests.Integration/Web/Services/Queries/WorkspaceQueryServiceTests.cs | Updates test helper to maintain Check.Latest* while inserting CheckResults. |
| SAMA.Tests.Integration/Web/Services/Queries/CheckQueryServiceTests.cs | Updates test helper to maintain Check.Latest* while inserting CheckResults. |
| SAMA.Tests.Integration/Web/Services/CheckExecutorJobIntegrationTests.cs | Adds coverage to ensure CheckExecutorJob updates denormalized columns. |
| SAMA.Tests.Integration/SAMA.Tests.Integration.csproj | Bumps EF/Npgsql/MSTest + coverage tool versions for integration tests. |
| SAMA.Shared/SAMA.Shared.csproj | Bumps Microsoft.Extensions.Http package version. |
| SAMA.Data/SAMA.Data.csproj | Bumps EF/Npgsql/Identity EF package versions. |
| SAMA.Data/Migrations/SamaDbContextModelSnapshot.cs | Reflects the new Check.Latest* columns in the model snapshot. |
| SAMA.Data/Migrations/20260612212029_AddLatestResultDenormalizationToChecks.Designer.cs | New migration designer including Check.Latest* columns. |
| SAMA.Data/Migrations/20260612212029_AddLatestResultDenormalizationToChecks.cs | Adds columns + backfills them from CheckResults via SQL. |
| SAMA.Data/Entities/Check.cs | Adds LatestStatus, LatestCheckedAt, LatestResponseTimeMs, LatestErrorMessage properties. |
| SAMA.Data/Configuration/CheckConfiguration.cs | Configures max lengths for the new denormalized columns. |
Files not reviewed (1)
- SAMA.Data/Migrations/20260612212029_AddLatestResultDenormalizationToChecks.Designer.cs: Generated file
Comment on lines
+29
to
+33
| public string? LatestStatus { get; set; } | ||
|
|
||
| public DateTimeOffset? LatestCheckedAt { get; set; } | ||
|
|
||
| public int? LatestResponseTimeMs { get; set; } |
Comment on lines
+22
to
+24
| modelBuilder | ||
| .HasAnnotation("ProductVersion", "10.0.5") | ||
| .HasAnnotation("Relational:MaxIdentifierLength", 63); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.