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

Adding support for S3 Backup URLs in SQL 2022 #714

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

benevoldson
Copy link

Modified logic checks for @url to allow s3:// format URLs to support SQL 2022. Tested with EC2 SQL 2022 standard.

@mbrrg
Copy link

mbrrg commented Mar 3, 2023

Great initiative! Improvement suggestion: support MAXTRANSFERSIZE up to 20 MB as supported by the S3 backend. Currently because of the parameter checks (max transfer size 4 MB) only databases up to 40 GB in size can be backed up. If I'm not mistaken.

@benevoldson
Copy link
Author

Great find! The new commit has checks specific to MAXTRANSERSIZE range for S3 as well as unsupported INIT option.

@alexsinkevich
Copy link

alexsinkevich commented May 3, 2023

I have modified the jobs on our test servers to backup straight to S3 using the changed backup procedure as listed in the PR. It works most of the time, however on a server with multiple databases in sizes up to 800GB, it started crashing the availability group and causing AG failovers. I am using the following parameters in the call. Note that I am generating the command sting so I can do uppercase on server name and AG name as we have naming inconsistency in our environment and S3 names are case sensitive:

DECLARE @AvailabilityGroupName sysname;
DECLARE @ServerName sysname;
DECLARE @URL VARCHAR(512);
DECLARE @Credential VARCHAR(512);

SELECT @ServerName = UPPER(@@SERVERNAME);
SELECT @AvailabilityGroupName = UPPER(Name) FROM sys.availability_groups;

-- If @AvailabilityGroupName is null, set it to the value that Ola's script is designed to process
SET @AvailabilityGroupName = ISNULL(@AvailabilityGroupName, 'AvailabilityGroupName');

SELECT @Credential = name FROM sys.credentials WHERE name LIKE 's3://%';

SELECT @URL = @Credential

DECLARE @SQL VARCHAR(MAX)
    = '
EXEC dbo.DatabaseBackup  
		@Databases =  ''USER_DATABASES'',
		@URL = ''' + @URL + '/'',  
		@Credential = ''' + @Credential + ''', 
		@BackupType = ''Full'',
		@Verify = ''Y'',
		@Checksum = ''Y'',
		@Compress = ''Y'',
		@MaxTransferSize = 20971520,
		@DatabaseOrder=''DATABASE_SIZE_DESC'',
		@MaxFileSize = 204800,
		@DirectoryStructure =''' + @ServerName     + '{DirectorySeparator}{BackupType}{DirectorySeparator}{DatabaseName}'',
		@AvailabilityGroupDirectoryStructure =''' + @AvailabilityGroupName     + '{DirectorySeparator}{BackupType}{DirectorySeparator}{DatabaseName}'',
		@FileName=''' + @ServerName     + '${InstanceName}_{DatabaseName}_{BackupType}_{Partial}_{CopyOnly}_{Year}{Month}{Day}_{Hour}{Minute}{Second}_{FileNumber}_of_{NumberOfFiles}.{FileExtension}'',
		@AvailabilityGroupFileName=''' + @AvailabilityGroupName    + '_{DatabaseName}_{BackupType}_{Partial}_{CopyOnly}_{Year}{Month}{Day}_{Hour}{Minute}{Second}_{FileNumber}_of_{NumberOfFiles}.{FileExtension}'',
		@LogToTable = ''Y''
		';

--PRINT @SQL;
EXEC (@SQL)

Have anybody else experienced this on SQL 2022? I mean the AG failovers when using backups of big databases to S3.

@jordiporto
Copy link

Hello, do you have any forecast to merge this change ? Thanks.

@dbaduck
Copy link

dbaduck commented Jun 29, 2023

But you may also need to add the provision to allow s3:// and no @credential clause when it comes to errors it throws when you don't supply a credential for Azure. S3 does not require a credential be specified.
So the line
IF @Credential IS NULL AND @URL IS NOT NULL AND NOT (@Version >= 13 OR SERVERPROPERTY('EngineEdition') = 8)
needs to add
AND @URL LIKE 'https://%/%'
so that you don't get an error of 'The value of @credential is not supported' when using S3 since it does not require a credential to be specified.

@benevoldson
Copy link
Author

But you may also need to add the provision to allow s3:// and no @credential clause when it comes to errors it throws when you don't supply a credential for Azure. S3 does not require a credential be specified. So the line IF @Credential IS NULL AND @URL IS NOT NULL AND NOT (@Version >= 13 OR SERVERPROPERTY('EngineEdition') = 8) needs to add AND @URL LIKE 'https://%/%' so that you don't get an error of 'The value of @credential is not supported' when using S3 since it does not require a credential to be specified.

Thank you, this modification has been made.

@Kevin-S-Lewis
Copy link

Kevin-S-Lewis commented Aug 10, 2023

I only just found this after hacking around myself... #629

"The IDENTITY should always be 'S3 Access Key' when using the S3 connector."

Given the above... may want a credential check as something like this (then its similar to Azure block blob):

IF (@URL LIKE 'https://%/%' AND @Credential IS NULL AND NOT EXISTS(SELECT * FROM sys.credentials WHERE UPPER(credential_identity) = 'SHARED ACCESS SIGNATURE')) OR (@URL LIKE 's3://%/%' AND @Credential IS NULL AND NOT EXISTS(SELECT * FROM sys.credentials WHERE UPPER(credential_identity) = 'S3 ACCESS KEY'))

Note its only Azure Page Blob that requires a credential in the backup string, Azure Block Blob and S3 backups don't, thus this bit of code could be reverted:

IF @Credential IS NULL AND @URL IS NOT NULL AND NOT (@Version >= 13 OR SERVERPROPERTY('EngineEdition') = 8) AND @URL LIKE 'https://%/%'

Back to:

IF @Credential IS NULL AND @URL IS NOT NULL AND NOT (@Version >= 13 OR SERVERPROPERTY('EngineEdition') = 8)

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

Successfully merging this pull request may close these issues.

None yet

6 participants