docker-archive / docker-registry

This is **DEPRECATED**! Please go to https://github.com/docker/distribution
Apache License 2.0
2.88k stars 879 forks source link

fix registry._setup_database.lock remove issue and rollback the session after UNIQUE constraint failed exception #917

Open harrisonfeng opened 9 years ago

harrisonfeng commented 9 years ago

It will try to remove non-existing registry._setup_database.lock when search_backend setting to sqlalchemy

Signed-off-by: Harrison Feng harrison_feng@163.com

dmp42 commented 9 years ago

I don't get it. Under what condition may that file not exist?

https://github.com/harrisonfeng/docker-registry/blob/master/docker_registry/toolkit.py#L325

harrisonfeng commented 9 years ago

when search_backend sets to sqlalchemy, first start the container, then the container will stop automatically as that error happened. Actually, that error will happen every time. So I cannot run docker registry container at all. With my fix, I can run it happily.

dmp42 commented 9 years ago

Ok. The better fix here would be to use lockfile (http://pythonhosted.org/lockfile/). Would you be willing to try this? Thanks.

pires commented 9 years ago

@dmp42 why is lockfile a better solution?

docwhat commented 9 years ago

I'm not sure what @dmp42 is thinking, but as a general rule -- if you can let someone else do the tricky bits, the better. And lockfiles are notoriously tricky.

dmp42 commented 9 years ago

@docwhat @pires the current implementation is a parody of a proper file locking mechanism, and it fails miserably as this report demonstrates... So yeah, let's use something that does work :)

pires commented 9 years ago

Got it, thanks ;-)

pwais commented 9 years ago

+1 if this change reduces crashes at least in the interim. Currently docker run -p 5000:5000 registry is broken out-of-the-box (try running it several times).

hans-d commented 9 years ago

+1. First quick fix the broken part (done with this PR), then in a separate PR improve locking