sp_DatabaseRestore - Not all commands are logged to table

[fvanderhaegen]

In sp_DatabaseRestore not all commands are executed with [dbo].[CommandExecute]. So if something goes wrong nothing gets logged.

I'll adjust the code

[fvanderhaegen]

I've adjusted the code, ready to test.
See: https://github.com/fvanderhaegen/SQL-Server-First-Responder-Kit/blob/issue_2700/fvanderhaegen/sp_DatabaseRestore.sql

[BrentOzar]

Thanks for the code! I've merged it into the dev branch, and it'll be in the December release with credit to you in the release notes.

[gdoddsy]

@BrentOzar @fvanderhaegen - when I run this with Ola's latest version of CommandExecute, it requires a parameter called DatabaseContext which is not supplied in sp_DatabaseRestore. Happy to try and fix it, but if we go down this route then you are strongly coupling sp_DatabaseRestore to Ola's CommandExecute which may not be desirable if he changes the parameters again. Thoughts?

[BrentOzar]

That's a great question. I've tried asking him to make new parameters default to null, and he hasn't been responsive to that. I agree that strongly coupling it is a problem, but I think it's a great idea too, given how useful it would be to have the logging. How do you feel about it?

[gdoddsy]

I'm ok with it, but we need to make sure that we're on the current version of Ola's script. I'm working on some changes at the moment to sp_DatabaseRestore, should I just fix it while I'm in here?

[BrentOzar]

Perfect, sounds good! Thanks sir!

[gdoddsy]

I thought about this a bit more overnight, and I'm less ok with it than I was. I think if you're going to take on a dependency like this, then you need to make it easy to find the right (supported) version of Ola's script, ideally have it install with this script.

The problem with that though, is that if sp_DatabaseRestore isn't depending on the same version of Ola's scripts as I'm using for backups, then I can't use Ola's scripts for backups and sp_DatabaseRestore for restores on the same server.

To highlight the problem, my change last night has now forced @fvanderhaegen to upgrade Ola's scripts to use sp_DatabaseRestore...maybe they don't want to, maybe they can't find the time to test it, maybe some management process means they can't easily update it. Whatever the case, sp_DatabaseRestore is now unworkable until they upgrade their backup process.

My final thought is that I have sp_DatabaseRestore as part of a DR run-sheet. If everything burns down and we're restoring from tape, then sp_DatabaseRestore is the easiest way to get our backups running on a new server. There's no an extra dependency I need to install. I'm happy to update my run sheet now I know about it, but if I found that problem in the middle of the night during a disaster then I'd be less happy about it.

The logging is great, and I want to keep it for that reason, but it's a fundamental change that could go very wrong.

That's just my opinion, happy for you to disagree.

[fvanderhaegen]

There was already a dependency with Ola's script before my change.
It was used when putting the database offline or in single_user, so I decided to replace all the sp_executesql calls so that everything happens in a consistent manner.