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

Implement the "put-and-verify" lock for external storage. #56523

Closed
Tracked by #56522
YuJuncen opened this issue Oct 10, 2024 · 3 comments · Fixed by #56597
Closed
Tracked by #56522

Implement the "put-and-verify" lock for external storage. #56523

YuJuncen opened this issue Oct 10, 2024 · 3 comments · Fixed by #56597
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@YuJuncen
Copy link
Contributor

YuJuncen commented Oct 10, 2024

Enhancement

Background

Sometimes, we need to control concurrency access to the same backup archive, like:

  • When compacting / restoring, we want to block other clients from migrating the backup storage to a new version.
  • When migrating the backup storage to a new version, we want to forbid reading.
  • When truncating the backup storage, we don't want another truncating operation join.
  • When backing up, we don't want another backup join.

But external storage locking isn't trivial. Simply putting a lock file isn't safe enough: because after checking there isn't such a lock file, another one may write it immediately. Object locks provide stronger consistency, but also require extra configuration and permissions.

We can use a "put-and-verify" lock to lock the storage, in detail, a locker need to implement this interface:

type PutAndVerify interface {
    Prefix() string
    Content() io.Reader
    Verify(s ExternalStorage) error
}

Then we lock the storage by:

  • Put a intention file with a random suffix to the Prefix().
  • Check if there is another file in the Prefix().
  • If there is, remove our intention file and back off or report error to caller.
  • If there isn't, and Verify() returns no error, put the Content() to the Prefix(), without suffix, then remove the intention file.
@YuJuncen YuJuncen added the type/enhancement The issue or PR belongs to an enhancement. label Oct 10, 2024
@D3Hunter
Copy link
Contributor

I'm not that familiar with BR, can you please clearify some description to help me understand

When compacting / restoring, we want to block migrating to a new version.

migrating WHO to a new version? you mean converting a backup stored in S3 to format of another version?

When migrating to a new version, we want to forbid reading.

you mean read/write S3 backup at same time?

When truncating the storage, we don't want another trancating operation join.

the storage is the external storage, such as s3 right?

@YuJuncen
Copy link
Contributor Author

@D3Hunter The "storage" should be "backup storage", i.e. the set of files generated by log backup. Let me clarify this. "Migration" is an atomic operation that applies to the "stabilized" backup storage (a brief reference here, after applying a "migration", the "stabilized" backup storage goes to a new version.

@D3Hunter
Copy link
Contributor

redefine migration, confusing😓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants