Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug on commit() #108

Open
acortiana opened this issue Mar 6, 2020 · 5 comments
Open

bug on commit() #108

acortiana opened this issue Mar 6, 2020 · 5 comments

Comments

@acortiana
Copy link

I'm using sqlitedict version 1.6.0 and Python version 3.6.9
If I do this

mydict = SqliteDict('./my_db.sqlite', autocommit=False)
mydict['some_key'] = 'some_value'
mydict.commit()

and the commit fails for some reason, the
mydict.commit()
line hangs and never returns.
I think that the problem is here, maybe a try/except is needed, with some error handling code:
https://github.com/RaRe-Technologies/sqlitedict/blob/39e8dced2f56498df6e0d58a0addf42297917d3f/sqlitedict.py#L404

I tried to add a try/except and, in my case, I got this error:

2020-03-06 14:56:45 ERROR    Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/sqlitedict.py", line 405, in run
    conn.commit()
sqlite3.OperationalError: database is locked
@jonchun
Copy link

jonchun commented Mar 10, 2020

I just tested the same code and it works perfectly fine on my end.

sqlite3.OperationalError: database is locked

is most likely being thrown because you have a different process/thread trying to commit to the same sqlite db at the same time. Try completely restarting your computer (to make sure any orphaned/backgrounded tasks get killed) and see if you can reproduce the issue consistently.

Additionally see if you can reproduce the issue even after changing the db file name so you're 100% sure nothing else can be writing to the db

@acortiana
Copy link
Author

The problem is not to understand why I'm getting the exception.
In my use case, I have a lot of processes accessing the DB in the same time, and this is by design.

The problem is that sqlitedict is not responding in the right way when there is an exception on the underlying commit operation.
To do a test in the right way, you have to make the underlying commit
https://github.com/RaRe-Technologies/sqlitedict/blob/39e8dced2f56498df6e0d58a0addf42297917d3f/sqlitedict.py#L404
raise an exception.
IMHO, the expected behavior is to make that exception "propagate" from the thread to the main process and be raised by
mydict.commit()
call

The current behavior is to hang indefinitely without ever returning and this cannot be the right behavior, right?

@shantanoo
Copy link

Quick fix could be adding timeout to sqlite connection.

https://stackoverflow.com/questions/2598801/python-sqlite-database-locked-despite-large-timeouts#3659261

@ed2050
Copy link

ed2050 commented Dec 28, 2020

Was anything ever done to fix this? The "quick fix" doesn't appear possible because there doesn't seem to be a way to pass sqlite connection params to sqlitedict. The connection happens inside sqlitedict.

Maybe what's needed is another arg to sqlitedict constructor. Something like (dict) connect_params, a dictionary of arguments for sqlite3.connect. Then pass it to connect as kwargs.

I don't see any code in sqlitedict.commit () that would swallow an exception. My guess is that acortiana did something more besides just wrapping conn.commit () in try / except. Although it seems he's using the multithread class based on line number, so perhaps it's a thread issue (exception gets raised and silently swallowed, or error message output is ignored, in a secondary thread, but error propagates all the way to the top when testing with one thread).

@acortiana
Copy link
Author

I did this test to better demonstrate the problem:

  1. added lines
if hasattr(self, 'test123'):
    raise NameError('HiThere')
self.test123 = 1

just after this line:
https://github.com/RaRe-Technologies/sqlitedict/blob/39e8dced2f56498df6e0d58a0addf42297917d3f/sqlitedict.py#L404

  1. run exactly this code
from sqlitedict import SqliteDict
mydict = SqliteDict('./my_db.sqlite', autocommit=False)
mydict['some_key'] = 'some_value'
mydict.commit()

I added the "hasattr" condition to skip the first commit from the "HiThere" exception.
This because when the mydict object is created, it does an implicit (first) commit operation to create the table.

Executing the code, you can see that the Thread-1 raises the exception but you can also see that mydict.commit() command remains stuck without ever returning.
IMHO the right behavior is that mydict.commit() also must raise an exception, without remaining stuck forever.

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

No branches or pull requests

5 participants