Test-DbaLsnChain: Fix bug in case log backup is taken during full backup

[andreasjordan]

Type of Change

• Bug fix (non-breaking change, fixes # )
• New feature (non-breaking change, adds functionality, fixes # )
• Breaking change (affects multiple commands or functionality, fixes # )
• Ran manual Pester test and has passed (Invoke-ManualPester)
• Adding code coverage to existing functionality
• Pester test is included
• If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
• Unit test is included
• Documentation
• Build system

I have a customer facing this issue, so I did some analysis.

Here is the new logging before the fix:

AUSFÜHRLICH: [15:47:45][Test-DbaLsnChain] Testing LSN Chain
AUSFÜHRLICH: [15:47:45][Test-DbaLsnChain] Testing LSN Chain - Type Type
AUSFÜHRLICH: [15:47:45][<Unknown>] Checking C:\Temp\FULL_20260202_020339.bak - isLogBackup False, isBasedOnAnchor False, hasGreaterFirstLsn False, FullDBAnchor.CheckPointLsn 34347000156416000046, DatabaseBackupLsn 34347000148382400046, FirstLsn 34246000021188000002 LastLsn 34347000156420000001
AUSFÜHRLICH: [15:47:45][<Unknown>] Checking C:\Temp\LOG_20260202_020500.trn - isLogBackup True, isBasedOnAnchor False, hasGreaterFirstLsn False, FullDBAnchor.CheckPointLsn 34347000156416000046, DatabaseBackupLsn 34347000148382400046, FirstLsn 34347000156393600001 LastLsn 34347000156428000001
AUSFÜHRLICH: [15:47:45][<Unknown>] Checking C:\Temp\LOG_20260202_021002.trn - isLogBackup True, isBasedOnAnchor True, hasGreaterFirstLsn True, FullDBAnchor.CheckPointLsn 34347000156416000046, DatabaseBackupLsn 34347000156416000046, FirstLsn 34347000156428000001 LastLsn 34347000156458400001
WARNUNG: [15:47:45][Test-DbaLsnChain] Break in LSN Chain between C:\Temp\FULL_20260202_020339.bak and C:\Temp\LOG_20260202_021002.trn 
AUSFÜHRLICH: [15:47:45][Test-DbaLsnChain] Anchor 34347000156420000001 - FirstLSN 34347000156428000001
AUSFÜHRLICH: [15:47:45][Test-DbaBackupInformation] LSN Check failed

And here is the logging after the fix:

AUSFÜHRLICH: [15:56:24][Test-DbaLsnChain] Testing LSN Chain
AUSFÜHRLICH: [15:56:24][Test-DbaLsnChain] Testing LSN Chain - Type Type
AUSFÜHRLICH: [15:56:24][<Unknown>] Checking C:\Temp\FULL_20260202_020339.bak - isLogBackup False, isBasedOnAnchor False, hasGreaterLastLsn True, FullDBAnchor.CheckPointLsn 34347000156416000046, DatabaseBackupLsn 34347000148382400046, FirstLsn 34246000021188000002 LastLsn 34347000156420000001
AUSFÜHRLICH: [15:56:24][<Unknown>] Checking C:\Temp\LOG_20260202_020500.trn - isLogBackup True, isBasedOnAnchor False, hasGreaterLastLsn True, FullDBAnchor.CheckPointLsn 34347000156416000046, DatabaseBackupLsn 34347000148382400046, FirstLsn 34347000156393600001 LastLsn 34347000156428000001
AUSFÜHRLICH: [15:56:24][<Unknown>] Checking C:\Temp\LOG_20260202_021002.trn - isLogBackup True, isBasedOnAnchor True, hasGreaterLastLsn True, FullDBAnchor.CheckPointLsn 34347000156416000046, DatabaseBackupLsn 34347000156416000046, FirstLsn 34347000156428000001 LastLsn 34347000156458400001
AUSFÜHRLICH: [15:56:24][Test-DbaLsnChain] Passed LSN Chain checks

The bug is that a log backup must have a higher FirstLsn than the anchor backup. This is true in most cases so we have not seen this issue. But here in this case, the first log backup that is needed for restore is still based on the "older" full backup, but has a higher LastLsn that the anchor backup and thus needs to be restored as the first log backup.

I also fixed a formatting issue and made the test more readable including a log message for future analysis. That's why I spilt the change into multiple commits for easier review of the change.

[andreasjordan]

@claude please review this pull request

[claude]

Claude finished @andreasjordan's task in 1m 51s —— View job

---

PR Review: Test-DbaLsnChain LSN Chain Bug Fix

• Read the changed file
• Analyze the diff
• Provide review feedback

---

Summary

This PR fixes a real bug in Test-DbaLsnChain affecting a specific (but valid) backup scenario: when a transaction log backup is taken during a full backup. The fix is correct and well-reasoned.

---

Core Bug Analysis

The scenario:

When a log backup is taken concurrently with a full backup, the log backup will have:

• DatabaseBackupLSN = an older LSN (from before the full backup started)
• FirstLSN = somewhere in the middle of the full backup's range
• LastLSN = past the full backup's CheckPointLSN

The old filter logic (private/functions/Test-DbaLsnChain.ps1, original line ~103):

($_.DatabaseBackupLSN.ToString() -ne $FullDBAnchor.CheckPointLSN) -and ($_.FirstLSN -gt $FullDBAnchor.CheckPointLSN)

This incorrectly excluded log-during-full backups because their FirstLSN is not greater than CheckPointLSN — it's in the middle of the full backup range.

The new filter logic (line 106):

$hasGreaterLastLsn = $_.LastLsn -gt $FullDBAnchor.CheckPointLsn

Using LastLsn instead of FirstLsn is correct: a log backup taken during a full backup must extend past CheckPointLSN to be required for restore — and its LastLSN confirms that.

The logic $isBasedOnAnchor -or $hasGreaterLastLsn correctly covers both cases:

1. Normal log backups (taken after full): DatabaseBackupLSN == CheckPointLSN
2. Log-during-full backups: LastLSN > CheckPointLSN

Verdict: The fix is correct.

---

Formatting / Indentation Fix

The for loop, return $true, and the closing } were previously at 4-space indent — the same level as the end {} block keyword — rather than 8-space indent inside the end {} block. While PowerShell's parser doesn't care about indentation (only brace matching), the formatting was misleading. The new code corrects the indentation. ✅

---

Minor Observations

1. Write-Message inside Where-Object emits for all backup types (line 108)

The verbose log is emitted for every item in $TestHistory, including non-log backups. This is visible in the PR's example output:

Checking C:\Temp\FULL_20260202_020339.bak - isLogBackup False ...

The <Unknown> source is because the message originates inside an anonymous scriptblock, not a named function. This is acceptable for diagnostics, but it's worth being aware that verbose output includes full/diff backup entries too.

2. .ToString() removed from LSN comparisons (line 105-106)

The original code used .ToString() for comparisons: $_.DatabaseBackupLSN.ToString() -eq $FullDBAnchor.CheckPointLSN. The new code compares directly: $_.DatabaseBackupLsn -eq $FullDBAnchor.CheckPointLsn. This is consistent with the existing pattern elsewhere in the file (lines 79, 82) and should be fine, but is worth noting.

3. Pre-existing break after return $false (lines 119, 125)

break after return is dead code. This is pre-existing and out of scope for this PR.

4. No newline at end of file — pre-existing issue, not introduced by this PR.

---

Conclusion

The fix correctly identifies the root cause: FirstLSN is the wrong comparator for log backups taken during a full backup; LastLSN is what determines whether the log backup covers the necessary checkpoint. The added verbose logging significantly improves future debuggability of this code path. The indentation cleanup is a bonus improvement.

The PR looks good to merge.

[potatoqualitee]

Oh la la @niphlod this seems different from the edge cases we usually refuse so I shall merge