Skip to content
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

DatabaseBackup - configurable folder names #14

Closed
BrentOzar opened this issue Jun 23, 2017 · 16 comments
Closed

DatabaseBackup - configurable folder names #14

BrentOzar opened this issue Jun 23, 2017 · 16 comments

Comments

@BrentOzar
Copy link

BrentOzar commented Jun 23, 2017

Current behavior: backup file/folder names are hard-coded in this style:

SET @CurrentFilePath = @CurrentDirectoryPath + '\' + CASE WHEN @CurrentAvailabilityGroup IS NOT NULL THEN @Cluster + '$' + @CurrentAvailabilityGroup ELSE REPLACE(CAST(SERVERPROPERTY('servername') AS nvarchar(max)),'\','$') END + '_' + @CurrentDatabaseNameFS + '_' + UPPER(@CurrentBackupType) + CASE WHEN @ReadWriteFileGroups = 'Y' THEN '_PARTIAL' ELSE '' END + CASE WHEN @CopyOnly = 'Y' THEN '_COPY_ONLY' ELSE '' END + '_' + REPLACE(REPLACE(REPLACE((CONVERT(nvarchar,@CurrentDate,120)),'-',''),' ','_'),':','') + CASE WHEN @NumberOfFiles > 1 AND @NumberOfFiles <= 9 THEN '_' + CAST(@CurrentFileNumber AS nvarchar) WHEN @NumberOfFiles >= 10 THEN '_' + RIGHT('0' + CAST(@CurrentFileNumber AS nvarchar),2) ELSE '' END + '.' + @CurrentFileExtension

The server name, cluster, and Availability Group parts are hard-coded.

We need to be able to predict the backup folder path on this project in order to let multiple servers access the same set of backups, and keep backups in the same folder as we fail around from one server to another. (For more details, check out the advanced architecture diagram at the bottom of sp_AllNightLog's documentation.)

Proposed Solution

  • Don't add any new parameters (to keep things simple)
  • Don't touch file names (I don't need that, but if somebody else does, they can code it)
  • Use the existing @Directory NVARCHAR(MAX) parameter
  • If the @Directory parameter has '**' anywhere in it, trigger the new behavior

New behavior:

  • Declare a new @DirectoryOverride internal parameter (not visible to users, only inside the proc):
  • Set @DirectoryOverride to everything to the right of the first **. For example, if they passed in @Directory = 'C:\TEMP\**SERVERNAME**\**DATABASENAME**', then set @DirectoryOverride to '**SERVERNAME**\**DATABASENAME**'
  • Remove everything ** and after in @Directory. So for example, if they passed in @Directory = 'C:\TEMP\**SERVERNAME**\**DATABASENAME**', then set @Directory to 'C:\TEMP\'
  • In the loop of doing backups, if @DirectoryOverride is not null, create the necessary directories based on @DirectoryOverride
      IF @DirectoryOverride IS NULL
      BEGIN
        SET @CurrentDirectoryOverride = CASE WHEN @CurrentAvailabilityGroup IS NOT NULL THEN @Cluster + '$' + @CurrentAvailabilityGroup ELSE REPLACE(CAST(SERVERPROPERTY('servername') AS nvarchar(max)),'\','$') END + '\' + @CurrentDatabaseNameFS + '\' + UPPER(@CurrentBackupType);
      END
      ELSE /* IF @DirectoryOverride IS NULL */
      BEGIN
        SET @CurrentDirectoryOverride = @DirectoryOverride;
        SET @CurrentDirectoryOverride = REPLACE(@CurrentDirectoryOverride, '**CLUSTER**', COALESCE(@Cluster,''));
        SET @CurrentDirectoryOverride = REPLACE(@CurrentDirectoryOverride, '**AVAILABILITYGROUP**', COALESCE(@CurrentAvailabilityGroup,''));
        SET @CurrentDirectoryOverride = REPLACE(@CurrentDirectoryOverride, '**SERVERNAME**', REPLACE(CAST(SERVERPROPERTY('servername') AS nvarchar(max)),'\','$'));
        IF CHARINDEX('\',CAST(SERVERPROPERTY('servername') AS nvarchar(max))) > 0
        BEGIN
            SET @CurrentDirectoryOverride = REPLACE(@CurrentDirectoryOverride, '**SERVERNAMEWITHOUTINSTANCE**', SUBSTRING(CAST(SERVERPROPERTY('servername') AS nvarchar(max)), 1, (CHARINDEX('\',CAST(SERVERPROPERTY('servername') AS nvarchar(max))) - 1)));
            SET @CurrentDirectoryOverride = REPLACE(@CurrentDirectoryOverride, '**INSTANCENAME**', SUBSTRING(CAST(SERVERPROPERTY('servername') AS nvarchar(max)), CHARINDEX('\',CAST(SERVERPROPERTY('servername') AS nvarchar(max))), (LEN(CAST(SERVERPROPERTY('servername') AS nvarchar(max))) - CHARINDEX('\',CAST(SERVERPROPERTY('servername') AS nvarchar(max)))) + 1));
        END
        ELSE /* No instance installed */
        BEGIN
            SET @CurrentDirectoryOverride = REPLACE(@CurrentDirectoryOverride, '**SERVERNAMEWITHOUTINSTANCE**', CAST(SERVERPROPERTY('servername') AS nvarchar(max)));
            SET @CurrentDirectoryOverride = REPLACE(@CurrentDirectoryOverride, '**INSTANCENAME**', 'DEFAULT');
        END
        SET @CurrentDirectoryOverride = REPLACE(@CurrentDirectoryOverride, '**DATABASENAME**', @CurrentDatabaseNameFS);
        SET @CurrentDirectoryOverride = REPLACE(@CurrentDirectoryOverride, '**BACKUPTYPE**', UPPER(@CurrentBackupType));
      END /* IF @DirectoryOverride IS NOT NULL */

Things we won't replace in this code:

  • CurrentDate - that would have to be replaced at every database backup command since it changes throughout the execution as time marches ever forward
  • Read/Write filegroups
  • Number of files

Why Use Asterisks?

Because the asterisk * isn't allowed in Windows or Linux folder names.) I was originally going to use forward slashes, but it turns out there's known bugs in the BACKUP and RESTORE command (MS's, not Ola's) with those. For example, BACKUP simply discards forward slashes, so someone might have had //SERVERNAME// in their backups already, working despite themselves, and this new code would have broken.

The current DatabaseBackup behavior is to error out if @Directory has '**' - try running these:

EXECUTE [dbo].[DatabaseBackup] @Databases = 'USER_DATABASES', @Directory = N'C:\**TEMP**\',
 @BackupType = 'FULL', @Verify = 'Y', @CleanupTime = NULL, @CheckSum = 'Y', @LogToTable = 'Y'

EXECUTE [dbo].[DatabaseBackup] @Databases = 'USER_DATABASES', @Directory = N'C:\TEMP\**SERVERNAME**',
 @BackupType = 'FULL', @Verify = 'Y', @CleanupTime = NULL, @CheckSum = 'Y', @LogToTable = 'Y'

You get errors like:

Msg 50000, Level 16, State 1, Procedure DatabaseBackup, Line 621 [Batch Start Line 11]
The directory C:*TEMP*\ does not exist.

Msg 50000, Level 16, State 1, Procedure DatabaseBackup, Line 621 [Batch Start Line 11]
The directory C:\TEMP*SERVERNAME* does not exist.

Work Required

  • Implement & test the folder name replacement (draft done)
  • Implement & test @MirrorDirectory replacement (draft done)
  • Document the options (working on now)
  • Build input-validation code to make sure they type the params right (TBD)
  • Test standalone instance (done)
  • Test clustered instance
  • Test named instance (done)
  • Test on AG

Code in progress:
https://github.com/BrentOzarULTD/sql-server-maintenance-solution/blob/issue_14/brent/DatabaseBackup.sql

BrentOzar added a commit to BrentOzarULTD/sql-server-maintenance-solution that referenced this issue Jun 23, 2017
First working version, not tested on instances yet.
BrentOzar added a commit to BrentOzarULTD/sql-server-maintenance-solution that referenced this issue Jun 23, 2017
BrentOzar added a commit to BrentOzarULTD/sql-server-maintenance-solution that referenced this issue Jun 23, 2017
Thanks to idea from Jeff Stanford.
@MordechaiKilstein
Copy link

I don't understand why you're coding a solution when it can be specified on the parameter line... If you're concerned that different servers are doing backups, then either centralize the SQL Agent or when creating the jobs for Ola's script, customize value then.

@BrentOzar
Copy link
Author

BrentOzar commented Jun 25, 2017 via email

@BrentOzar
Copy link
Author

Code is working at the client, so did a pull request for it (above).

@JMFL92
Copy link

JMFL92 commented Sep 4, 2017

Hi BrentOzar

I was in touch with Ola on October 2016 about the backup directory structure. I didn't want to have the $ directory before having a directory per db, and idem for directory by backup type.

He sent to me a "non-production" update of the stored prcedure DatabaseBackup.

It has two new parameters with defaults like this:

@DirectoryStructure nvarchar(max) = '{ServerName}${InstanceName}{DatabaseName}{BackupType}',
@AvailabilityGroupDirectoryStructure nvarchar(max) = '{ClusterName}${AvailabilityGroupName}{DatabaseName}{BackupType}',

If you don't want the server name you could call the stored procedure like this:

@DirectoryStructure = '{DatabaseName}{BackupType}'

As I've no news from Ola since this moment, and that no new version has been release since october 2016 (I hope Ola is well !), I merged these modifications with the latest production package (07 Oct 2016), and I test it since few weeks. For the moment, it seem to be ok.
I use it with @DirectoryStructure = '{DatabaseName}'

Do you want I give you the updated stored procedure ?

@BrentOzar
Copy link
Author

@JMFL92 thanks, but I'd rather stick with the public versions on Github. If you want to do your own pull request, that's totally cool. Thanks!

@JMFL92
Copy link

JMFL92 commented Sep 4, 2017

I was wrong in my description. I just updated it.
Ok, I will do that as soon as i've some time ;)

@drstonephd
Copy link

drstonephd commented Mar 10, 2018

Hi Brent. If the override allows backups from different databases to land in the same folder, xp_delete_file would not be limited to one database. Perhaps server, instance, and database folders are required unless xp_delete_file is replace with code that limits the delete to files from the same server, instance, and database. Because the default is to use the BAK extension for both full and diff, it might mean the backup type is also needs to be a folder. (Is there a reason not to use DIF for diff backups?) A replacement of xp_delete_file could read the info directly from the backup file. If the default filename is not replaced, a replacement could use the filename to filter on server, instance, and database name.

If xp_cmdshell is not enabled, it might be hard to replace xp_delete_file and delete the right files in the procedure.

@BrentOzar
Copy link
Author

BrentOzar commented Mar 10, 2018

@drstonephd hmm, I'm confused. Are you saying you want to replace xp_delete_file with some other kind of command? If so, I'm afraid that's well beyond my spare time capabilities, but you're welcome to create an issue and work with Ola to implement that. It's way outside of the scope of this issue, though.

If you were suggesting something else, can you clarify?

@drstonephd
Copy link

Hi. Making the folder structure configurable requires either replacing xp_delete_file or a check to insure the files for retention are isolated. This means we still need all those folders.

@BrentOzar
Copy link
Author

OK, cool, if you need that for your use cases, you're welcome to add a separate issue for that. I don't need that for my use cases, so I wouldn't be doing that work. Thanks though!

@olahallengren
Copy link
Owner

The tricky thing with having a configurable directory structure is the cleanup.

DatabaseBackup is using xp_delete_file to delete backup files. xp_delete_file deletes backup files based on a directory path, a file extension, and a modified date.

This means that if for example full and differential backups are in the same directory and have the same file extension, then you could have a differential backup job deleting your full backups. There can also be scenarios where you have databases with the same name on multiple instances or servers, backing up to the same directory.

On the other hand, this is one of the most requested features in the backup script.

@olahallengren
Copy link
Owner

olahallengren commented May 6, 2018

I have been looking at this some more, and I am leaning towards adding new parameters for the directory structure.

One question with using the existing parameter is that if the user specifies for example C:\Backup, should the stored procedure create the sub-directories (as it would today), or should it back up directly to C:\Backup?

There is also the question how to specify different directory structures, for databases that are in availability groups, and databases that are not in availability groups.

Another thing is that today the directory parameter supports specifying multiple directories with a comma in between (striped backup). If I would use the existing parameter, the user would need to specify the directory structure for each of the directories.

@olahallengren
Copy link
Owner

olahallengren commented May 7, 2018

I now have a test version where the directory structure, the file names, and the file extensions are configurable. Please contact me (https://github.com/olahallengren) if you would like to test it.

@olahallengren
Copy link
Owner

I now have a public preview version. You can download it here.

@olahallengren
Copy link
Owner

I created a branch for this feature, [feature-configurable_directory_structure].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants