search

piskvorky / sqlitedict

Persistent dict, backed by sqlite3 and pickle, multithread-safe.
Apache License 2.0
1.16k stars 130 forks source link

Question regarding commit() #52

Open FlxVctr opened 8 years ago

FlxVctr commented 8 years ago

I don't understand this part in the documentation:

     |  commit(self, blocking=True)
     |      Persist all data to disk.
     |      
     |      When `blocking` is False, the commit command is queued, but the data is
     |      not guaranteed persisted (default implication when autocommit=True).

Does this mean blocking is False or True when using autocommit, e.g. can I be sure my data is persisted to disk?

Maybe it's me but as I understand the doc here, its meaning is ambiguous, also because the usage example says in the comments it's persisted to disk with autocommit=True.

fiendish commented 6 years ago

looking at the code, autocommit calls commit with its default arguments, so autocommit blocks. This may be a bug. Inline comments indicate that it shouldn't block.

piskvorky commented 6 years ago

What the docs mean is that with blocking=False, the commit command is only queued. If something happens before it's actually pulled from the queue (exception, user break, ...), the commit may not be executed.

I agree with @fiendish that with autocommit=True, it's always blocking=True. So the commit is always executed (not just queued!) after each command. Beware -- slow.

@fiendish which comments indicate a bug?

fiendish commented 6 years ago

@piskvorky https://github.com/RaRe-Technologies/sqlitedict/blob/f6456a3c280815c997487a9965cd76346c93879c/sqlitedict.py#L307

Everything in that comment before "we need..." should probably be removed.

piskvorky commented 6 years ago

Aha, thanks! That comment doesn't seem to match reality, yes.

piskvorky commented 5 years ago

You may be right, I don't remember that at all. Will need to investigate. @endlisnis can you have a look at the code too, to make sure?

endlisnis commented 5 years ago

My inspection of the code makes it look like "autocommit" is NON-BLOCKING. As part of setitem, it should check autocommit, and block on the commit, but that's non-trivial. You'll need a way to signal the completion of a commit back to the source thread with the realization that there may be multiple threads waiting for multiple different commits.

jonchun commented 4 years ago

As of commit 39e2020ade12aa3e248dcda907ea8a15a5e7d004, it looks like autocommit is BLOCKING. Can this issue be marked resolved?