awslabs / git-remote-s3

Apache License 2.0
478 stars 8 forks source link

Conditional writes for conditional locks #3

Open ad-m-ss opened 4 weeks ago

ad-m-ss commented 4 weeks ago

Hey

Have you considered using conditional writes to resolve the issue of a broken repository (fixable by the doctor) for concurrent writes? https://aws.amazon.com/about-aws/whats-new/2024/08/amazon-s3-conditional-writes/

Kind regards

massi-ang commented 4 weeks ago

Hi Adam,

as the probability of the occurrence of such issue is really low, I chose to solve it in the way I mention as it keeps the complexity of the code low.

I considered adding a locking mechanism using conditional writes, but with increased complexity in the client code.

Let's say I have two git clients A and B doing simultaneous push of 2 different commit of the same ancestor Happy path:

  1. A is first doing the operation and performs a conditional write of a LOCK file before doing the ListObjects operation
  2. B conditional write of the LOCK file fails
  3. A writes the new bundle
  4. A deletes the LOCK file

Failure:

  1. A is first doing the operation and performs a conditional write of a LOCK file before doing the ListObjects operation
  2. B conditional write of the LOCK file fails
  3. A process exits without properly cleaning up
  4. The LOCK file is not removed and needs to be manually cleaned

To avoid the manual cleaning one can use the timestamp of the LOCK file to implement session expiration, ie if the LOCK file is older that T, it is considered as stale and can be removed, but the value of T would be dependent on factors like the size of the bundle and the write throughput of the client, and might be hard to choose. It must also be enforced to be the same for all clients working on a repo and setup as repo metadata.

When developing this tool, one of the principles was to keep it as simple as possible, since simple code is more robust in the end but I am keen on getting yours and others feedback on the need of this additional logic and on ways to implement it.

Massimiliano