storj / edge

Storj edge services (including multi-tenant, S3-compatible server to interact with the Storj network)
GNU Affero General Public License v3.0
54 stars 18 forks source link

Race Conditions with Object Versioning and Retention Updates #460

Closed NiaStorj closed 3 months ago

NiaStorj commented 4 months ago

Summary:

potential race conditions related to object versioning and retention updates in our system. There are concerns that operations may not correctly account for the current state of versioning or retention, leading to possible inconsistencies

Acceptance Criteria:

Investigate and potentially implement a solution using database transactions or exclusive locks to ensure consistency. Ensure that all relevant code paths check the current state of object versioning and retention before proceeding with operations.

Note:

Expected Behavior: Operations should consistently respect the current state of versioning and retention, without leading to data inconsistencies or unintended behavior.

Actual Behavior: Potential for operations to proceed without correctly verifying the current state, leading to possible issues.

shaupt131 commented 3 months ago

Copied over from another ticket

Per Egon: in SetRetention: https://review.dev.storj.io/c/storj/storj/+/14066 deleteObjectExactVersionUsingObjectLock: https://github.com/storj/storj/blob/cb9fe240c01c483ae0b2d6f9847bc84a0ced9ac4/satellite/metabase/delete.go#L120 deleteObjectLastCommittedPlainUsingObjectLock: https://github.com/storj/storj/blob/cb9fe240c01c483ae0b2d6f9847bc84a0ced9ac4/satellite/metabase/delete.go#L518 precommitDeleteUnversioned: https://github.com/storj/storj/blob/cb9fe240c01c483ae0b2d6f9847bc84a0ced9ac4/satellite/metabase/precommit.go#L366 -- the delete happens first, so this approach is safe precommitDeleteUnversionedWithNonPendingUsingObjectLock: https://github.com/storj/storj/blob/cb9fe240c01c483ae0b2d6f9847bc84a0ced9ac4/satellite/metabase/precommit.go#L881 Other issues:

unexpected response code for request: https://review.dev.storj.io/c/storj/uplink/+/14096

iglesiasbrandon commented 3 months ago

for now we are going to allow the race conditions. please take a look at the conversation in this thread for more context: https://storj.slack.com/archives/C06KR3A8KB5/p1724693040090309