Set-DbaDbQueryStoreOption - Refresh the Query Store options after changing them with T-SQL - #10593
Merged
Merged
Conversation
…nging them with T-SQL MaxPlansPerQuery, WaitStatsCaptureMode and the four CustomCapturePolicy options are changed with an ALTER DATABASE statement rather than through SMO. The SMO object knows nothing about that and still holds what it read before, so the command reported the values from before its own change and left the object of the caller saying the same. This is not the staleness a reused connection brings with it: it happens on every call, because the command reads $db.QueryStoreOptions before it runs the statement and that fills the cache. The SMO path a few lines above already does Alter followed by Refresh. The T-SQL path now does the same, right after the statement, which is one round trip and only for the calls that use one of these options. Copy-DbaDbQueryStoreOption is fixed with it, because it copies the settings through this command. Fixes #10561. Tests: a Context that sets MaxPlansPerQuery and WaitStatsCaptureMode and expects the returned object and the SMO object of the caller to carry them, plus a check that the instance really has them, which passes either way and says the T-SQL was never the problem. Both new assertions fail against development with "Expected 555, but got 200". It uses the database of the Describe rather than creating one: a CREATE DATABASE at that point fails intermittently with "Could not obtain exclusive lock on database 'model'", because the tests above read Query Store on model and leave a session parked in it. That is #10584, which is not merged yet. (do Set-DbaDbQueryStoreOption, Copy-DbaDbQueryStoreOption)
Member
|
looks good 👌🏼 ty |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of Change
Purpose
MaxPlansPerQuery,WaitStatsCaptureModeand the fourCustomCapturePolicy*options are changed with anALTER DATABASEstatement rather than through SMO. The SMO object knows nothing about that and still holds what it read before, so the command reported the values from before its own change and left the object of the caller saying the same:This is not the staleness that a reused connection brings with it. It happens on every call, including one where the command opens the connection itself, because the command reads
$db.QueryStoreOptionsbefore it runs the statement and that fills the cache.Copy-DbaDbQueryStoreOptionis fixed with it, because it copies the settings by calling this command.Approach
The SMO path a few lines above already does the right thing:
The T-SQL path now gets the same treatment, right after the statement:
That is one round trip per database, and only for the calls that actually use one of these options. What a reused connection reports about changes made elsewhere is unaffected, as it should be.
Commands to test
Tests
A
Contextthat sets both options and expects:Get-command cannot paper overThe first two fail against
developmentwithExpected 555, but got 200.Set-DbaDbQueryStoreOption: 11 tests, all passing, no leftovers in the lab.Note on the test fixture
The new
Contextuses the database that theDescribealready creates rather than one of its own. ACREATE DATABASEat that point fails intermittently withbecause the tests above it read Query Store on
modeland leave a session parked in it. That is the leak #10584 fixes, which is not merged yet - worth knowing that it now reaches test files that have nothing to do with connection handling.This text was created by Claude and reviewed by Andreas Jordan.