piskvorky / sqlitedict

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

Allow to change autocommit attribute #168

Open presidento opened 1 year ago

presidento commented 1 year ago

If autocommit changed, it was not propagated to SqliteMultithread, since its autocommit value was set only in initialization phase. According to the documentation we can temporarily disable autocommit if we need performance, but the performance was still the same as with keeping autocommit enabled.

Test case 1

If autocommit is False, then the dict should not be saved if we forget to call commit. (This is the case if we initialize SqliteDict with autocommit=False)

my_dict = sqlitedict.SqliteDict(filename="autocommit-test.sqlite", autocommit=True)
my_dict.autocommit = False
my_dict["a"] = 12
del my_dict

my_dict = sqlitedict.SqliteDict(filename="autocommit-test.sqlite")
print("a" in my_dict) # Should be False, but it is True (commit was called)
my_dict.terminate()

Test case 2

Performance measusements with this file:

import sqlitedict
import time
import contextlib

@contextlib.contextmanager
def measure(name):
    start = time.time()
    yield
    end = time.time()
    print(f"Elapsed for {name}: {end-start:.1f} seconds")

def fill_values(in_dict):
    for index in range(1_000):
        in_dict[index] = index

with measure("autocommit=False -> True"):
    my_dict = sqlitedict.SqliteDict(filename="mytest.sqlite", autocommit=False)
    my_dict.autocommit = True
    fill_values(my_dict)
    my_dict.commit()
    my_dict.terminate()

with measure("autocommit=True"):
    my_dict = sqlitedict.SqliteDict(filename="mytest.sqlite", autocommit=True)
    fill_values(my_dict)
    my_dict.commit()
    my_dict.terminate()

with measure("autocommit=False"):
    my_dict = sqlitedict.SqliteDict(filename="mytest.sqlite", autocommit=False)
    fill_values(my_dict)
    my_dict.commit()
    my_dict.terminate()

with measure("autocommit=True -> False"):
    my_dict = sqlitedict.SqliteDict(filename="mytest.sqlite", autocommit=True)
    my_dict.autocommit = False
    fill_values(my_dict)
    my_dict.commit()
    my_dict.terminate()

Results:

# Original
❯ python .\sqlitedict-test.py
Elapsed for autocommit=False -> True: 1.9 seconds
Elapsed for autocommit=True: 2.1 seconds
Elapsed for autocommit=False: 0.1 seconds
Elapsed for autocommit=True -> False: 1.5 seconds # <------

# Fixed version
❯ python .\sqlitedict-test.py
Elapsed for autocommit=False -> True: 1.9 seconds
Elapsed for autocommit=True: 2.2 seconds
Elapsed for autocommit=False: 0.1 seconds
Elapsed for autocommit=True -> False: 0.2 seconds # <------

You can be inspired with this pull request and reject this one, or I may can change it to follow the contributing guides. For me using WAL journal mode with autocommit was a perfect solution, I just wanted to let you know that it did not work as I expected.

mpenkov commented 1 year ago

Sorry for the late reply. Looks good, but is it possible to add tests?

presidento commented 1 year ago

Sorry, I moved forward. You can consider this as a bug report with a hint how to fix.