Issues some implementations issues in `lock` package about cozy-stack HOT 5 CLOSED

I would like to propose to refactor this package in several steps in order to fix those issues.

from cozy-stack.

Memory leak in simple_mem.go and simple_redis.go

I agree, but in practice, it is not an issue at our scale.

The Redis implementation is not evenly distributed

Yes, the plan was to go for the solution 1: Move the RWMutex on Redis. But, as you said, it was more difficult than anticipated, and we didn't achieve it.

Possible datarace on Redis locking

I don't understand how using context.WithDeadline helps for this issue.

The locks should all have a TTL

If you have a way to cancel the action, then yes, having a TTL would be a good idea. But, I don't think we can have a way to cancel the action for a lot of cases. No transaction mechanism in CouchDB is really painful here.

from cozy-stack.

Hi @nono 👋

If you have a way to cancel the action, then yes, having a TTL would be a good idea. But, I don't think we can have a way to cancel the action for a lot of cases. ...

IMHO this problem could be solved by handling a context on all you I/O (means all your couchdb methods) . If you have a context matching your lock TTL it would allow you to interrupt your requests to the database at the moment your lock is released instead of continuing.

I don't understand how using context.WithDeadline helps for this issue.

Same if your context is canceled when your lock expire it would make you interrupt your requests and avoid a possible comflict.

The only issue with this solution is when you have several data that need to be modified together. In that case you can't abort indeed but this situation should not be frequent and should be avoided at all cost. In that case you can simple use an another context in order to avoid any cancellation.

If you have a lot of situations requiring consecutive writes maybe the event sourcing pattern with something like watermill could be a great idea but this is a different approach to data where it can be eventualy consistent. At least it will be consistent at a given moment.

from cozy-stack.

IMHO this problem could be solved by handling a context on all you I/O (means all your couchdb methods) . If you have a context matching your lock TTL it would allow you to interrupt your requests to the database at the moment your lock is released instead of continuing.

The context will allow to not wait for the response, but the CouchDB/redis/Swift server will still do the action. It doesn't cancel it. The request has been sent, the server process it, and cutting the connection is not enough to stop the server.

The only issue with this solution is when you have several data that need to be modified together. In that case you can't abort indeed but this situation should not be frequent and should be avoided at all cost.

Making several changes together is the true use case for locks. When we are making a single change, we can often use other mechanism to avoid conflicts:

• Redis has a lot of commands for that (like SetNX)
• CouchDB has revision per document
• for Swift, we can use idempotency.

But, when we need to update several CouchDB documents, or update one CouchDB document after making a change in Swift, then locks are useful.

If you have a lot of situations requiring consecutive writes maybe the event sourcing pattern with something like watermill could be a great idea but this is a different approach to data where it can be eventualy consistent. At least it will be consistent at a given moment.

Maybe, but I'm not sure how you ensure eventual consistency without locks. If you have an event that says that directory foo has been renamed to bar, and just after that, another event that says that its parent has been renamed from qux to courge, we want its path updated to /courge/bar, not /courge/foo or /qux/bar. And I think, a lock is the proper way to do that.

from cozy-stack.

The context will allow to not wait for the response, but the CouchDB/redis/Swift server will still do the action. It doesn't cancel it. The request has been sent, the server process it, and cutting the connection is not enough to stop the server.

Yes of course, it will not rollback the changes already done but at least it will allow you to stop continuing to change your data without lock. As said previously this point is valid only if stopping between two modification will not corrupt the data. In data case you are forced to continue but at least in most of the case you have stopped your process.

Making several changes together is the true use case for locks. When we are making a single change, we can often use other mechanism to avoid conflicts:

Agree

Maybe, but I'm not sure how you ensure eventual consistency without locks. If you have an event that says that directory foo has been renamed to bar, and just after that, another event that says that its parent has been renamed from qux to courge, we want its path updated to /courge/bar, not /courge/foo or /qux/bar. And I think, a lock is the proper way to do that.

If you want some really cool ideas for you file system you can take a look at the Merkle Tree used by the git internals or the Directed Acrylic Graph (DAG) used by the ipfs tool. Both handle a filesystem, can do atomic changes, handle versioning and are able to deduplicate the content.

from cozy-stack.