Skip to content

sp_BlitzBackups: refresh pushed media facts - #4120

Merged
BrentOzar merged 2 commits into
devfrom
codex/test-backup-remote-push-4118
Sep 12, 2026
Merged

BrentOzar merged 2 commits into
devfrom
codex/test-backup-remote-push-4118

Conversation

@BrentOzar

Copy link
Copy Markdown
Member

Recompute existing media facts in the push window and retained full backups, so stale values are corrected. Missing source rows remain unchanged; widen the existing lookback to refresh older logs.

Fixes #4118.

Validation: committed manual integration test passed across two disposable SQL Server 2022 instances: remote schema upgrade, populated-fact refresh outside the window, and RPC-disabled pushes with/without media columns. No new options or unrelated changes.

Copilot AI lite review requested due to automatic review settings September 12, 2026 17:50
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-12T18:10:24.940228Z c19ef13 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2be90dff9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread sp_BlitzBackups.sql Outdated
Comment thread .github/scripts/backup-remote-push-regression.sql Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Three moderate findings remain involving cleanup reliability, RPC-disabled execution, and unnecessary full-history updates.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request refreshes existing pushed media facts for the active window and retained full backups, with remote linked-server regression coverage.

Changes:

  • Recomputes source-derived media facts and metadata.
  • Preserves missing-source rows and refreshes older logs.
  • Adds remote schema, refresh, and RPC-disabled push scenarios.
File summaries
File Summary Final review findings
sp_BlitzBackups.sql Broadens centralized media-fact refresh behavior. moderate (1 vote, L1961): RPC-disabled execution can fail during earlier remote batches before reaching the refresh. moderate (1 vote, L1961): unchanged retained rows are rewritten on every push, causing unnecessary history-wide updates.
.github/scripts/backup-remote-push-regression.sql Tests remote push behavior and cleanup. moderate (3 votes, L49): unconditional cleanup can mask setup failures and leave the fixture database behind.
Review details

Suppressed comments (2)

sp_BlitzBackups.sql:1961

  • This refresh is unreachable when RPC OUT is disabled: before execution reaches this line, the push path unconditionally runs the index-maintenance batches at lines 1652-1786 via EXEC (...) AT [linked_server], and those pass-through calls require RPC OUT even when all destination indexes already exist. Therefore the new regression's RPC-disabled calls cannot exercise the distributed-DML path and will fail first; move the RPC-disabled guard before those batches or skip/replace the remote EXEC AT work in that mode.
    WHERE (b.backup_start_date>=DATEADD(hour,@Hours,SYSDATETIME()) OR b.type=''D'');';

sp_BlitzBackups.sql:1961

  • Removing the NULL predicate makes every push rewrite every matching retained full row, even when both media facts and copied metadata already match the source. A centralized history with many full backups will therefore incur a full-history UPDATE (and its logging/locking/version-store cost) on every scheduled push. Keep the broadened candidate set, but add change detection (or stage the source values) so unchanged rows are not rewritten.
    WHERE (b.backup_start_date>=DATEADD(hour,@Hours,SYSDATETIME()) OR b.type=''D'');';
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/scripts/backup-remote-push-regression.sql Outdated

Copy link
Copy Markdown
Member Author

Addressed the two in-scope findings in c19ef13:

  • Compare recomputed media facts and all copied metadata using NULL-safe EXCEPT; update only changed rows. The remote regression now audits affected rows and confirms an unchanged repeat push writes zero rows.
  • Track remote fixture creation before cleanup. Injecting a failure before backup/remote creation preserves the original error and leaves neither fixture database behind.

The suppressed RPC-disabled finding is not applicable: the cited table/index EXEC AT batches are inside IF @@rowcount = 0 for the missing backupset table. Existing-table pushes skip them. Actual two-server SQL Server 2022 regression passed again with RPC OUT disabled, both with and without media columns. diff check passed. No unrelated changes or expanded review scope.

@BrentOzar
BrentOzar merged commit fabffe0 into dev Sep 12, 2026
3 checks passed
@BrentOzar
BrentOzar deleted the codex/test-backup-remote-push-4118 branch September 12, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sp_BlitzBackups: cover remote history-push media facts and refresh behavior

2 participants