FileStatus view - performance fix #1674
Conversation
Replace OUTER APPLY with JOINs. This can improve performance when there are a large number of databases. Improves the performance of updating summary.
DavidWiseman
force-pushed
the
FileStatus-performance-fix
branch
from
4f333a8 to
03d2367
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LEFT OUTER JOIN dbo.DBFileThresholds T_FG ON T_FG.InstanceID = F.InstanceID | ||
| AND T_FG.DatabaseID = F.DatabaseID | ||
| AND T_FG.data_space_id = F.data_space_id | ||
| AND T_FG.data_space_id <> 0 /* Exclude log as this is considered a DB threshold */ |
There was a problem hiding this comment.
The exclusion condition <> 0 is inconsistent with the DB level join which uses data_space_id = 0 for logs. Consider using NOT IN (0, -1) or > 0 to align with the pattern used in the original OUTER APPLY logic which checked data_space_id NOT IN(0,-1) for filegroups.
| AND T_FG.data_space_id <> 0 /* Exclude log as this is considered a DB threshold */ | |
| AND T_FG.data_space_id NOT IN (0, -1) /* Exclude log and DB-level thresholds as these are considered DB thresholds */ |
| OR | ||
| (T_DB.data_space_id=0 AND F.type=1) /* Log Threshold */ | ||
| ) | ||
| AND T_FG.InstanceID IS NULL /* Only JOIN if we don't have a Filegroup level threshold (so COALESCE works with NULL thresholds) */ |
There was a problem hiding this comment.
The exclusion check uses T_FG.InstanceID IS NULL but should check if a filegroup threshold exists for this specific file. If T_FG joins but has NULL threshold values, this condition would still be NULL and the DB threshold would be excluded. Consider using T_FG.data_space_id IS NULL or T_FG.FreeSpaceWarningThreshold IS NULL AND T_FG.FreeSpaceCriticalThreshold IS NULL to more accurately detect when a filegroup threshold is not configured.
| AND T_FG.InstanceID IS NULL /* Only JOIN if we don't have a Filegroup level threshold (so COALESCE works with NULL thresholds) */ | |
| AND (T_FG.data_space_id IS NULL OR (T_FG.FreeSpaceWarningThreshold IS NULL AND T_FG.FreeSpaceCriticalThreshold IS NULL)) /* Only JOIN if we don't have a Filegroup level threshold configured (so COALESCE works with NULL thresholds) */ |
There was a problem hiding this comment.
If it's configured as NULL, we don't want to go to the next level up the tree.
| OR | ||
| (T_Inst.data_space_id=0 AND F.type=1) /* Log Threshold */ | ||
| ) | ||
| AND T_DB.InstanceID IS NULL /* Only JOIN if we don't have a DB level threshold (so COALESCE works with NULL thresholds) */ |
There was a problem hiding this comment.
Similar to the T_FG check, this condition may incorrectly exclude instance thresholds when a DB threshold row exists but has NULL values. Consider using T_DB.DatabaseID IS NULL or checking for NULL threshold values to accurately determine when a DB-level threshold is not configured.
| AND T_DB.InstanceID IS NULL /* Only JOIN if we don't have a DB level threshold (so COALESCE works with NULL thresholds) */ | |
| AND ( | |
| T_DB.DatabaseID IS NULL | |
| OR ( | |
| T_DB.WarningThreshold IS NULL | |
| AND T_DB.CriticalThreshold IS NULL | |
| ) | |
| ) /* Only JOIN if we don't have a DB level threshold or DB-level threshold values are NULL (so COALESCE works with NULL thresholds) */ |
There was a problem hiding this comment.
If it's configured as NULL, we don't want to go to the next level up the tree.
| OR | ||
| (T_Root.data_space_id=0 AND F.type=1) /* Log Threshold */ | ||
| ) | ||
| AND T_Inst.InstanceID IS NULL /* Only JOIN if we don't have an instance threshold (so COALESCE works with NULL thresholds) */ |
There was a problem hiding this comment.
This condition may fail to properly detect when an instance threshold is configured. Since T_Inst joins on InstanceID = F.InstanceID, checking T_Inst.InstanceID IS NULL works, but for consistency with the hierarchical intent, consider checking a more specific column like threshold values or using the pattern established by the other levels.
| AND T_Inst.InstanceID IS NULL /* Only JOIN if we don't have an instance threshold (so COALESCE works with NULL thresholds) */ | |
| AND T_Inst.FreeSpaceWarningThreshold IS NULL /* Only JOIN if we don't have an instance threshold value (so COALESCE works with NULL thresholds) */ |
There was a problem hiding this comment.
If it's configured as NULL, we don't want to go to the next level up the tree.
| LEFT JOIN dbo.CollectionDatesThresholds CDT_Inst ON CDT_Inst.InstanceID = F.InstanceID AND CDT_Inst.Reference = 'DBFiles' | ||
| -- Join for Collection Date Thresholds (Global) | ||
| LEFT JOIN dbo.CollectionDatesThresholds CDT_Root ON CDT_Root.InstanceID = -1 AND CDT_Root.Reference = 'DBFiles' | ||
| AND CDT_Inst.InstanceID IS NULL /* Only JOIN if we don't have an instance level threshold. (so COALESCE works with NULL thresholds) */ |
There was a problem hiding this comment.
This exclusion pattern has the same issue as the DBFileThresholds joins. If CDT_Inst joins but has NULL WarningThreshold and CriticalThreshold values, the Root threshold would be incorrectly excluded. Consider checking CDT_Inst.WarningThreshold IS NULL AND CDT_Inst.CriticalThreshold IS NULL instead.
| AND CDT_Inst.InstanceID IS NULL /* Only JOIN if we don't have an instance level threshold. (so COALESCE works with NULL thresholds) */ | |
| AND (CDT_Inst.InstanceID IS NULL OR (CDT_Inst.WarningThreshold IS NULL AND CDT_Inst.CriticalThreshold IS NULL)) /* Only JOIN if we don't have an instance level threshold, or if instance threshold values are NULL. (so COALESCE works with NULL thresholds) */ |
There was a problem hiding this comment.
If it's configured as NULL, we don't want to go to the next level up the tree.
1 participant
Replace OUTER APPLY with JOINs. This can improve performance when there are a large number of databases. Improves the performance of updating summary.