Fix stale cache issue in Add-DbaAgDatabase

[agowa]

When multiple databases are joined to an AG using Add-DbaAgDatabase using the same SqlInstance objects Get-DbaAgDatabase returns $null for all except the first database.

It looks like this issue does NOT happen when the dbatools internal backup function is used beforehand. But if a stored procedure is invoked to create the backup it does.

In order to fix this I added a switch to Get-DbaAgDatabase to update the Availability group object before use.

Note: I was unable to run .\tests\manual.pester.ps1 within the GitHub codespace, as even after manually installing PSScriptAnalyzer it still failed to find and use it.

EDIT: Only partially fixes the problem, it still sometimes returns null. Interestingly even if the Get-DbaAgReplica call still returns $null, changing it to Get-DbaAvailabilityGroup -SqlInstance $replicaServerSMO[$replicaName] -SqlCredential $SqlCredential -AvailabilityGroup $AvailabilityGroup | Get-DbaAgDatabase -AvailabilityGroup $Avail abilityGroup -Database $db.Name -EnableException appears to almost always work even in these cases?!

Edit2: Fixed now too, the issue is that AvailabilityReplicas.Refresh() needs to be called for $InputObject.AvailabilityDatabases to be updated. (No, that's not a typo, it's really refreshing Replicas for Databases to be updated 🙄)

Please read -- recent changes to our repo

On November 10, 2022, we removed some bloat from our repository (for the second and final time). This change requires that all contributors reclone or refork their repo.

PRs from repos that have not been recently reforked or recloned will be closed and @potatoqualitee will cherry-pick your commits and open a new PR with your changes.

• Please confirm you have the smaller repo (85MB .git directory vs 275MB or 110MB or 185MB .git directory)

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 (.\tests\manual.pester.ps1)
• 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

Purpose

Fix caching issue within Add-DbaAgdatabase

Approach

Triggers a refresh of the AvailabilityGroup object. But only does so if it is required.

Commands to test

The foreach around Add-DbaAgDatabase is what causes the bug (but not for the very first database)

[bool]$isDev = $true
[string]$sqlNode001 = "sql-srv001.local"
[string]$ag = "ag001
[string[]]$dbs = "db001", "db002", "db003"

[Microsoft.SqlServer.Management.Smo.Server]$node = Connect-DbaInstance -SqlInstance $sqlNode001 -TrustServerCertificate:$isDev

[Microsoft.SqlServer.Management.Smo.Server[]]$peerNodes = @()
$peerNodes = Get-DbaAgReplica -SqlInstance $node -AvailabilityGroup $ag -EnableException | Where-Object {
  # Get hostname from fqdn and remove main node from list of replicaas
  $_.Name -ne ($sqlNode001 -replace '\..*$', '')
} | ForEach-Object {
  # Open connection to each replica
  Connect-DbaInstance -SqlInstance $_.Name -TrustServerCertificate:$isDev
}

foreach($db in $dbs) {
  # this invokes a backup using a stored procedure from https://ola.hallengren.com/, the stored procedure itself is irrelevant to this issue.
  # But callign Invoke-DbaQuery may taint the SqlInstance object, therefore I'm going to include it just in case.
  Invoke-DbaQuery -SqlInstance $node -Database 'master' -Query '[dbo].[DatabaseBackup]' -CommandType StoredProcedure -EnableException -SqlParameter @{...}

  # Btw, calling `Backup-DbaDatabase -SqlInstance $DbaInstanceNode -Database $restoreSettings.database_name -Type Full -EnableException` here would also fix the caching issue.
  # But it alwas works for the 1st element in $dbs, so this issue is NOT related to sql database backup itself it's just a side effect.

  Add-DbaAgDatabase -Confirm:$false -SqlInstance $node -AvailabilityGroup $ag -Database $db -Secondary $peerNodes -EnableException
}

Screenshots

[potatoqualitee]

Fantastic PR! Thank you so much. @andreasjordan do you see any potential issues with the changes?

[andreasjordan]

I would like to find a way to fix the issue without adding a new parameter. Can you give me some days to investigate before merging?

[andreasjordan]

I have setup my lab and can reproduce the issue - thanks to the detailed description.

Please give me some more days to have a deeper look.

[potatoqualitee]

Thank you both 🙇🏼 I'll also take a closer look if I've got time in the upcoming days.

[andreasjordan]

I have a theory: I think this is "normal" SMO behavior. With the first database, the SMO property $server.AvailabilityGroups['AG'].AvailabilityDatabases is read the first time, so the SMO fetches the information from the server. But it also marks this property as "feched" and will not update its value without a refresh. And as we use the same SMO object with the second database, we still use the old information - where the new database is missing.

I will test a bit and see if I can find the best place for the refresh.

[andreasjordan]

Ok, I have added one line to fix the problem.

Please add $ag.AvailabilityDatabases.Refresh() between lines 95 and 96 of Get-DbaAvailabilityGroup and test if that solves the problem in your environment.

[potatoqualitee]

LGTM! I like this, thank you!

[potatoqualitee]

btw @andreasjordan, i removed the param

[andreasjordan]

@potatoqualitee sorry i'm only on mobile and can't figure out what you have merged, but i think the change is to big. Only one lines needed to be added. The pr should have been canceled and only the one line should have been added.

Please don't release and wait a day so I can check that.

[andreasjordan]

This needs to be undone!

$replicaAgDb = Get-DbaAgDatabase -Refresh -SqlInstance $replicaServerSMO[$replicaName] -AvailabilityGroup $AvailabilityGroup -Database $db.Name -EnableException would call the command with a parameter "-Refresh" that's not there - and fail.

[andreasjordan]

I just added #9101 to fix the issue.

[potatoqualitee]

oh dear, sorry! I didnt realize the refresh and depended on the tests passing 😱