Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

harrisonfeng
Copy link

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

Signed-off-by: Harrison Feng [email protected]

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

Signed-off-by: Harrison Feng <[email protected]>
Here is the error message:
InvalidRequestError: This Session's transaction has been rolled
back due to a previous exception during flush. To begin a new
transaction with this Session, first issue Session.rollback().
Original exception was: (IntegrityError) UNIQUE constraint failed:
version.id u'INSERT INTO version (id) VALUES (?)' (1,)

Signed-off-by: Harrison Feng <[email protected]>
@harrisonfeng harrisonfeng changed the title fix registry._setup_database.lock remove issue fix registry._setup_database.lock remove issue and rollback the session after UNIQUE constraint failed exception Jan 29, 2015
@dmp42
Copy link
Contributor

dmp42 commented Jan 29, 2015

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
Copy link
Author

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
Copy link
Contributor

dmp42 commented Jan 30, 2015

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

@pires
Copy link

pires commented Mar 17, 2015

@dmp42 why is lockfile a better solution?

@docwhat
Copy link

docwhat commented Mar 17, 2015

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
Copy link
Contributor

dmp42 commented Mar 17, 2015

@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
Copy link

pires commented Mar 18, 2015

Got it, thanks ;-)

@pwais
Copy link

pwais commented Apr 1, 2015

+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
Copy link

hans-d commented Apr 14, 2015

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants