Add ShrinkDriver sample: parallel DBCC SHRINKFILE with reporting, retries, and tests#1496
Add ShrinkDriver sample: parallel DBCC SHRINKFILE with reporting, retries, and tests#1496dimitri-furman wants to merge 2 commits into
Conversation
…ries, and tests A PowerShell sample that reclaims unused space from a database's data files by running parallel DBCC SHRINKFILE operations. Includes a report mode, incremental shrinking toward an optional target, progress monitoring, retries, graceful stop handling, and low-priority locking. Supports Entra ID, Windows, and SQL authentication, connects with certificate validation first, and ships unit and integration tests (LocalDB, SQL Server, and Azure SQL Database).
|
Suggested PR Description (GHCP+my own skills generated - so take with grain of salt). Why: What:
Also:
|
Matteo-T
left a comment
There was a problem hiding this comment.
I've asked my GHCP skill to take a look.
I also added a few things on my own.
Please have a look and see if anything makes sense to you.
| @@ -0,0 +1,1614 @@ | |||
| #Requires -Version 7.0 | |||
There was a problem hiding this comment.
If you derpend on SQLServer module, add it here, no?
There was a problem hiding this comment.
The SqlServer dependency is enforced at runtime in New-ShrinkConnection: it checks Get-Module -ListAvailable -Name SqlServer and throws an actionable message (Install-Module SqlServer -Scope CurrentUser), then imports the module on demand. I kept it out of #Requires -Modules deliberately, so dot-sourcing the script or running Get-Help doesn't force the (fairly heavy) module import, and so the failure message tells the user exactly how to fix it instead of the generic "module is missing".
Happy to add #Requires -Modules SqlServer as well if you'd prefer fail-fast at load time - though then the custom message becomes unreachable, so it'd be one or the other. Which do you prefer?
There was a problem hiding this comment.
I tend to stay away from "dynamic stuff"; so, declaring the dependency right there is my preference.
But I've seen it done both ways, so... "there's more than one way to do it" as usual :)
| $useColor = [string]::IsNullOrEmpty($env:NO_COLOR) | ||
| function Write-ShrinkLog { | ||
| param([string]$Message, [string]$Level = 'INFO') | ||
| $stamp = (Get-Date).ToString('yyyy-MM-dd HH:mm:ss') |
There was a problem hiding this comment.
I personally dislike any home-brewed timestamp formats - for many reasons (including obscure LOC issues, but mainly for the fact that I never know what TZ/Offset it referts to).
Roundripping is always the way to go.
(Get-Date).ToString('o')
Longer, but... expressive.
There was a problem hiding this comment.
Agreed on the ambiguity. I've switched the log timestamp to include the UTC offset - yyyy-MM-dd HH:mm:ss zzz (e.g. 2026-07-13 11:30:04 -07:00) - rather than full round-trip o, since these lines are meant to be read at a glance and o adds a T and 7-digit fractional seconds to every row. Happy to go to strict o if you'd rather. (The -LogPath file-name stamp has to stay colon-free, so it keeps the compact form.)
There was a problem hiding this comment.
For a sample, what you have is acceptable. I would not do it, but you can do it :-)
| default { | ||
| if ($Message -match '^-{3,}') { 'Cyan' } # section dividers/headers | ||
| elseif ($Message -match '(Grew|Gave up|Partly shrunk)\s*:\s*[1-9]') { 'Yellow' } # non-zero problem outcomes | ||
| elseif ($Message -match '(Shrunk|Repacked)\s*:\s*[1-9]') { 'Green' } # non-zero successful outcomes |
There was a problem hiding this comment.
Done - collapsed the alignment padding before those comments to a single space.
| $nextReport = (Get-Date).AddSeconds($StatusIntervalSeconds) | ||
| } | ||
| if (-not ($workers | Where-Object { -not $_.Handle.IsCompleted })) { Write-ShrinkLog 'All workers finished.'; break } | ||
| if ($deadline -and (Get-Date) -ge $deadline -and -not $shared.Stop) { |
There was a problem hiding this comment.
[concern] Use a monotonic clock for run-time limits and elapsed math
This deadline check — and the related elapsed/run-time math (lines 906, 984, 1052), the `` at line 865, and the ConnectTime fields at 565/581 — all use `Get-Date`, i.e. local wall-clock time. Local time is non-monotonic: a DST transition or any clock adjustment mid-run can make the deadline fire early or late, or produce negative elapsed values.
Suggestion: drive the interval logic from a monotonic source, e.g. \ = [System.Diagnostics.Stopwatch]::StartNew() and compare \.Elapsed against the limit. Keep Get-Date only for human-facing display timestamps (e.g. in Write-ShrinkLog and the yyyyMMdd-HHmmss stamp at line 177).
Comment by GitHub Copilot (Claude Opus 4.8) with Matteo Taveggia's supervision
There was a problem hiding this comment.
Good catch - these runs can last hours, so wall-clock non-monotonicity is a real risk for the deadline and elapsed math. I've moved all run-time and interval logic to a monotonic source: a [System.Diagnostics.Stopwatch] for the run clock (deadline, report scheduling, run-time display) and the static [System.Diagnostics.Stopwatch]::GetTimestamp() / Frequency for each worker's elapsed - the static form is thread-safe across the worker runspaces, where sharing a single Stopwatch instance would not be. Update-ShrinkStuckState now takes monotonic seconds too. Get-Date is kept only for the human-facing log timestamp and the log file name.
| while ((Get-Date) -lt $deadline -and $kills -lt 2) { | ||
| try { | ||
| $r = Invoke-Sqlcmd -ServerInstance $serverInstance -Database $database -TrustServerCertificate -ErrorAction Stop ` | ||
| -Query "SELECT TOP (1) session_id AS spid FROM sys.dm_exec_sessions WHERE program_name LIKE 'ShrinkDriver-w%'" |
There was a problem hiding this comment.
[concern] Reconnect test can kill a session from another database on a shared instance
The session to KILL is selected filtering only on program_name LIKE 'ShrinkDriver-w%'. On a shared SQL instance where the driver is running against more than one database, this SELECT TOP (1) can pick — and the next line kills — a worker session belonging to a different database's run.
Suggestion: scope the selection to the current test database by adding AND database_id = DB_ID() to the WHERE clause. Since the query runs via Invoke-Sqlcmd -Database \, DB_ID() resolves in the intended database context.
Comment by GitHub Copilot (Claude Opus 4.8) with Matteo Taveggia's supervision
There was a problem hiding this comment.
Agreed - I've scoped the selection with AND database_id = DB_ID(). The kill job connects with -Database $database, and the driver's workers connect to that same database, so DB_ID() resolves in the intended context and won't pick a worker session from another database's run.
Add a PSScriptInfo block (version 1.0.0) to ShrinkDriver.ps1 for script version metadata.
8e9ffe4 to
63c50b1
Compare
Why:
Reclaiming unused space from a large multi-file database is painful to do by hand: it is slow, can be interrupted by transient errors and failovers, and can be hard to finish successfully in the presence of active workloads.
This sample turns that into a single, observable, resumable operation.
What:
Adds a PowerShell
ShrinkDriversample that reclaims allocated-but-unused space from a database's data files by runningDBCC SHRINKFILEin parallel, with:Also: