New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sp_Blitz updates specific for Azure SQL ManagedInstance #1919

Closed
JocaPC opened this Issue Jan 12, 2019 · 8 comments

Comments

Projects
None yet
2 participants
@JocaPC
Copy link

JocaPC commented Jan 12, 2019

There are some Azure SQL Managed Instance-specific changes that might be applied in sp_blitz:

  1. In Azure SQL Managed Instance "sa" is disabled and the name of the real admin is specified when the instance is created. There are some info/warnings in sp_blitz that warn me that I'm not using "sa":
    • https://BrentOzar.com/go/sa - Login [.....] is a sysadmin - meaning they can do absolutely anything in SQL Server, including dropping databases or hiding their tracks.
    • https://BrentOzar.com/go/owndb - Database name: WideWorldImportersStandard Owner name: [...]
    • https://BrentOzar.com/go/owners - Job [RESTORE WWI DATABASES] is owned by [...] - meaning if their login is disabled or not available due to Active Directory problems, the job will stop working.
  2. System databases are placed on C: drive by design. In BC all other databases are on C: while in GP user databases are on http: while system databases are on C:. Could you remove this warning:
    • File Configuration | System Database on C Drive | The master database has a file on the C drive. Putting system databases on the C drive runs the risk of crashing the server when it runs out of space.
  3. Hekaton is enabled by default on all databases in BC and cannot be removed. You are reporting this as a warning on every database as Non-Default Database Config: https://BrentOzar.com/go/dbdefaults but it is not actionable because user cannot disable it.
  4. In Azure SQL Read Committed snapshot isolation and Transparent Data Encryption (and Hekaton mentioned above) are default database settings. I know that they are not default in on-premises, but should they be treated as defaults in MI case?

Optional:

  • MSDB is replicated as any other user database, so users can put tables there. I'm not sure is https://www.brentozar.com/blitz/tables-msdb-database/ valid or not. There are some scripts like adaptive index defrag that place tables in MSDB. From Mi point-of-view this is valid action. I'm not sure should it be reported as warning.

Describe the solution you'd like
I think that these items should not be reported if applied on Managed Instance

Are you ready to build the code for the feature?
I could, especially since it should be simple condition SERVERPROPERTY('EngineEdition') = 8 on the items that should be ignored, but I guess that you should decide what of the reported issues you want to apply/ignore in MI case (especially for MSDB and default settings RCSI and TDE). Also, I guess that it would be good to update text https://www.brentozar.com/blitz/ for changed issues and describe that this is not applied on Managed Instance.

@BrentOzar

This comment has been minimized.

Copy link
Member

BrentOzar commented Jan 12, 2019

OK, great! For the MSDB one - can users do a point-in-time restore on MSDB? (That's generally the guideline I use for which tables should be put in user databases - if you want to be able to restore 'em to a point in time, you want 'em in a user database.)

@BrentOzar

This comment has been minimized.

Copy link
Member

BrentOzar commented Jan 12, 2019

For the login being a sysadmin - that is actually a legit warning. We want to warn folks of all sysadmins on the system. Is there a hard-coded MI sysadmin name that should be ignored on MIs?

BrentOzar added a commit that referenced this issue Jan 12, 2019

@BrentOzar

This comment has been minimized.

Copy link
Member

BrentOzar commented Jan 12, 2019

I've added a pull request with the changes. You can see 'em here:

https://github.com/BrentOzarULTD/SQL-Server-First-Responder-Kit/pull/1920/files

And you can download the updated version of sp_Blitz here:

https://raw.githubusercontent.com/BrentOzarULTD/SQL-Server-First-Responder-Kit/a1d1d7a77415d49d1280b0a6c5570ce6d0489cd8/sp_Blitz.sql

There was already a section around line 343 that skipped specific checks for Managed Instances, like this:

		/* If the server is an Azure Managed Instance, skip checks that it doesn't allow */
		IF SERVERPROPERTY('EngineEdition') = 8
			BEGIN
						INSERT INTO #SkipChecks (CheckID) VALUES (1);  /* Full backups - because of the MI GUID name bug mentioned here: https://github.com/BrentOzarULTD/SQL-Server-First-Responder-Kit/issues/1481 */
						INSERT INTO #SkipChecks (CheckID) VALUES (2);  /* Log backups - because of the MI GUID name bug mentioned here: https://github.com/BrentOzarULTD/SQL-Server-First-Responder-Kit/issues/1481 */
						INSERT INTO #SkipChecks (CheckID) VALUES (6);  /* Security - Jobs Owned By Users per https://github.com/BrentOzarULTD/SQL-Server-First-Responder-Kit/issues/1919 */

I just added more stuff in there and in the database defaults section. I didn't do the MSDB one and the sysadmin one since I don't think those are bugs, but let me know.

@JocaPC

This comment has been minimized.

Copy link
Author

JocaPC commented Jan 12, 2019

I tried it and looks good.

There is no PITR for MSDB so I agree with MSDB decision.
I will need to check is it possible to use TSQL to determine who is sql admin set via portal (we are checking this in the background, but I don't see any user-visible flag in sys views for this login). For now, let's keep this warning, and I will file another issue once I find a way to distinguish the default sql admin from the others.
I see that you are keeping RSCI as non-default:
210 Non-Default Database Config Read Committed Snapshot Isolation Enabled
Another item that I notices: Enterprise Edition Features In Use has description "if you restore database to standard it would fail", but actually any restore to on-premises will fail (unless if you take a backup and wait a couple of months for some 2019 CTP that will have higher version than MI :) ). Not critical but might be confusing especially since user cannot fix it to migrate back to on-premises. I would remove this one since I think that you are not checking are there some hekaton object inside you are just checking is Hekaton enabled (and it is enabled on every database). Currently, this warning is reported on every database.

Thanks

@JocaPC

This comment has been minimized.

Copy link
Author

JocaPC commented Jan 13, 2019

Another interesting thing I noticed - sp_Blitz told me that:
MSDB Backup History Not Purged - Database backup history retained back to Jul 25 2014 1:14PM

It seems that I restored old .bak file and info about the backup is placed in backupset table although this date is earlier than the time I created MI.
Since Azure is handling backups and backup space, I'm not sure is this applicable to MI:
https://www.brentozar.com/blitz/msdb-history-not-purged/

Generally, could this be misleading on SQL Server too? If I keep backups up to 30 days and I restore a new database from some one old .bak from would this check warn me that I need to purge backup history although the old .bak file might be on external storage? Maybe you could check is the backupset row coming from backup or restore: https://stackoverflow.com/questions/51522342/determine-if-msdb-dbo-backupset-record-is-a-backup-or-a-restore

BrentOzar added a commit that referenced this issue Jan 13, 2019

#1919 sp_Blitz excluding restored databases
When checking backup history. Working on #1919.
@BrentOzar

This comment has been minimized.

Copy link
Member

BrentOzar commented Jan 13, 2019

About the RCSI - can you include screenshots or a full copy/paste of the entire row from sp_Blitz when you report these? The full details (including the CheckID) are really helpful. I'm not seeing how it would fire, but I don't have an MI handy to test this (we talked about how having small or dev editions of this would really help, hint hint, hahaha.)

About MSDB - ooo, great catch! I've never seen someone restoring a 4-year-old backup in production before. (In staging & dev, definitely.) I've added an exclusion for any databases that have been restored.

@JocaPC

This comment has been minimized.

Copy link
Author

JocaPC commented Jan 14, 2019

I apologize, I made a mistake - RSCI is not default settings on MI. It seems that we decided to align with the SQL Server instead of Azure Singleton in this case, so this change should be reverted. I don't believe that this default will be changed in the near future. I debugged the script in sp_blitz and then I found the issue.

The thing that confused me was that check reported 'Enabled' while some databases was already enabled because the value was changed and not the label. If you want to make it work for classic Azure SQL Database (EngineEdition=5), I maybe that code should be:

    CASE WHEN SERVERPROPERTY('EngineEdition') = 5 THEN 1 ELSE 0 END,
    133, 210, 
    CASE WHEN SERVERPROPERTY('EngineEdition') = 5 THEN 'Read Committed Snapshot Isolation Enabled' ELSE 'Read Committed Snapshot Isolation Disabled' END
, 'https://BrentOzar.com/go/dbdefaults', NULL

BrentOzar added a commit that referenced this issue Jan 14, 2019

#1919 sp_Blitz ignore some checks on Mgd Instances
Changing RCSI wording on Azure SQL DB. Working on #1919.

BrentOzar added a commit that referenced this issue Jan 14, 2019

1919 sp_Blitz ignoring a few checks for Managed Instances (#1920)
* 1919 sp_Blitz ignoring a few checks for Managed Instances

Working on #1919.

* #1919 sp_Blitz excluding restored databases

When checking backup history. Working on #1919.

* #1919 sp_Blitz ignore some checks on Mgd Instances

Changing RCSI wording on Azure SQL DB. Working on #1919.
@BrentOzar

This comment has been minimized.

Copy link
Member

BrentOzar commented Jan 14, 2019

OK, great! That's a good point about the "Enabled" wording on RCSI - I made those changes, and merged it into dev. This is now in the dev branch, and we'll do a release first thing in February and credit you. Thanks for the help!

@BrentOzar BrentOzar closed this Jan 14, 2019

BrentOzar added a commit that referenced this issue Jan 28, 2019

2019-01 Release (#1932)
* Updating readme.md for sp_DatabaseRestore

Documenting @ExistingDBAction for https://dba.stackexchange.com/questions/226145/sp-databaserestore-msg-50000.

* 1900 sp_BlitzIndex add histograms

When @TableName is specified and sys.dm_db_stats_histogram is available. Closes #1900.

* #1903 sp_Blitz SQLServerCheckup

Adding filter for that app name. Closes #1903.

* #1905 sp_BlitzIndex remove BOU link

Nothing against BOU, just don't need it in that particular place. Closes #1905.

* #1908 Update copyright dates

Ah, the glamour. Closes #1908.

* Issue #1904 Change RAISERROR 'severity' for that should trigger throw and error.

* Issue #1910 Add SQL Server version check before choosing 'memory grant' as the @BlitzCacheSortOrder.

* Issue #1910 Add temp table creation.

* #1914 sp_BlitzIndex partition error severity

Dropping severity level from 16 to 0 since we're logging it in the result set anyway. Closes #1914.

* Issue 1894 Moved RESTORE HEADERONLY up

* LF line endings

* added some extra checks + corrected some nesting

* rebase and line endings

* Web site commit

Does this fix line endings?

* #1916 sp_Blitz ignore backup on TempDB drive

Closes #1916.

* Auto line endings

Dealing with sp_DatabaseRestore's line endings.

* 1919 sp_Blitz ignoring a few checks for Managed Instances (#1920)

* 1919 sp_Blitz ignoring a few checks for Managed Instances

Working on #1919.

* #1919 sp_Blitz excluding restored databases

When checking backup history. Working on #1919.

* #1919 sp_Blitz ignore some checks on Mgd Instances

Changing RCSI wording on Azure SQL DB. Working on #1919.

* #1921 Sp_BlitzIndex Add Drop and Create Columns to Output (#1923)

Joined output query to #IndexCreateTsql to retrieve the CreateTsql already generated earlier in the script.  Created Drop TSQL based off evaluating what type of index was present.

* #1921 sp_BlitzIndex adding drops (#1924)

Moving drop and create TSQL to the end of mode 2's results. Closes #1921.

* #1925 sp_Blitz AWS RDS detection (#1926)

Don't just rely on EC2 VM name to detect RDS. Also adds a new result noting that checks were skipped. Closes #1925.

* #1927 sp_BlitzIndex skip rdsadmin db (#1928)

GetAllDatabases = 1 fails when it hits rdsadmin because they're referring to the resource db. Closes #1927.

* 2019_01 Release (#1931)

Prep work for the release - changing version numbers, building build scripts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment