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

backupccl: deflake TestMetadataSST #83561

Merged
merged 1 commit into from
Jun 29, 2022
Merged

backupccl: deflake TestMetadataSST #83561

merged 1 commit into from
Jun 29, 2022

Conversation

stevendanna
Copy link
Collaborator

This is a temporary fix for failures in TestMetadataSST.

First,

backup_metadata_test.go:49: error executing 'BACKUP TO $1': pq: a
CCL binary is required to use this statement type: *tree.Backup

is solved by moving the test back into the backupccl package so that
the plan hook for BACKUP is definitely registered.

Second,

backup_metadata_test.go:89: file /0/BACKUP_MANIFEST does not exist
in the UserFileTableSystem: external_storage: file doesn't exist

is solved by disabling tenants by default in the setup functions in
backuputils. There is duplication between these functions and the
functions in backupccl, where we already disable tenants. The error
above is likely the result of us directly using the internal executor
of the server to query the userfile storage, which may be incorrect if
we were actually talking to a tenant.

Release note: None

This is a temporary fix for failures in TestMetadataSST.

First,

    backup_metadata_test.go:49: error executing 'BACKUP TO $1': pq: a
    CCL binary is required to use this statement type: *tree.Backup

is solved by moving the test back into the backupccl package so that
the plan hook for BACKUP is definitely registered.

Second,

    backup_metadata_test.go:89: file /0/BACKUP_MANIFEST does not exist
    in the UserFileTableSystem: external_storage: file doesn't exist

is solved by disabling tenants by default in the setup functions in
backuputils. There is duplication between these functions and the
functions in backupccl, where we already disable tenants. The error
above is likely the result of us directly using the internal executor
of the server to query the userfile storage, which may be incorrect if
we were actually talking to a tenant.

Release note: None
@stevendanna stevendanna requested review from a team and msbutler June 29, 2022 01:04
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@stevendanna
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 29, 2022

Build failed (retrying...):

@stevendanna
Copy link
Collaborator Author

bors retry

@craig
Copy link
Contributor

craig bot commented Jun 29, 2022

Already running a review

@stevendanna
Copy link
Collaborator Author

bors r+ p=9999 single on

@craig
Copy link
Contributor

craig bot commented Jun 29, 2022

Already running a review

@craig
Copy link
Contributor

craig bot commented Jun 29, 2022

Build succeeded:

@craig craig bot merged commit 73e4816 into master Jun 29, 2022
@craig craig bot deleted the ssd/deflake-2 branch June 29, 2022 10:54
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.

3 participants