coleifer / peewee

a small, expressive orm -- supports postgresql, mysql, sqlite and cockroachdb
http://docs.peewee-orm.com/
MIT License
11.06k stars 1.37k forks source link

Regression of < used on connections #2901

Closed antoyo closed 3 months ago

antoyo commented 3 months ago

Hi. This commit seemed to have reintroduced the bug #1883. I haven't really tested the previous commit, but version 3.17.0 works for me. Thanks to fix this issue.

coleifer commented 3 months ago

I'm at a bit of a loss how to replicate this. Where are you seeing the traceback, when calling close() to check the connection back in? Is it possible that the timestamps are identical? The only change I can think that would affect you is the removal of the random "jitter" added to the timestamps.

For example, this runs 100 threads and over 1000 repeated runs I cannot replicate any issues:

import threading
import time

from peewee import *
from playhouse.pool import *

db = PooledSqliteDatabase('/tmp/test.db', max_connections=100, check_same_thread=False)

for test in range(1000):
    # Test this 1000 times.
    nthreads = 100

    def work():
        for i in range(10):
            db.connect()
            for i in range(10):
                db.execute_sql('select * from foo;').fetchall()
            time.sleep(0.00001)
            db.close()

    ts = [threading.Thread(target=work) for t in range(nthreads)]
    for t in ts: t.start()
    for t in ts: t.join()
coleifer commented 3 months ago

The only way I could replicate was by monkey-patching time.time() to return the same timestamp. I suppose if the operating system time resolution is seconds, e.g. per Python docs:

Note that even though the time is always returned as a floating point number, not all systems provide time with a better precision than 1 second.

It is possible this would occur with much greater likelihood. I wonder if that's the case on the system you're using.

Instead of using randomness, I've just inserted into the heap a comparable no-op. That should fix things fairly robustly.

antoyo commented 3 months ago

Here's what time() returns on my system:

>>> time()
1716389563.1667016

I'll try this commit and see if that fixes my issue.

coleifer commented 3 months ago

Can you please provide some indication, then, how you encounter or replicate this?