[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#32234: [PATCH 2/2] database: Serialize all database accesses in a th
From: |
Clément Lassieur |
Subject: |
bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread. |
Date: |
Mon, 27 Aug 2018 17:05:03 +0200 |
User-agent: |
mu4e 1.0; emacs 26.1 |
Thank you for the explanation Danny.
Indeed I didn't fix what you described. That could be done easily by
wrapping the handler with WITH-DB-CRITICAL-SECTION. I'm not sure about
the consequences in terms of performance, given that this will send a
huge function to a channel, and that all the work will be done in the
same thread. If you think it's worth it, don't hesitate to send a
patch.
Clément
Danny Milosavljevic <address@hidden> writes:
> Hi Clément,
>
> I've read through the patch and it seems to handle the case I mean fine
> because
> you support an arbitrary number of queries per db critical section - so I
> agree
> that this patchset is fine.
>
> Keep in mind this is only fine if the critical section is held over an entire
> http
> request handler and not only over a single database query (as far as I can see
> the former is the case in the patch - OK).
>
> Much longer explanation follows:
>
> On Mon, 27 Aug 2018 15:18:09 +0200
> Clément Lassieur <address@hidden> wrote:
>
>> Danny Milosavljevic <address@hidden> writes:
>>
>> > Hi Clément,
>> >
>> > in the future I plan on making the actual bin/evaluate use another
>> > database connection
>> > in order for the web interface to be isolated while it's querying.
>>
>> I don't understand... bin/evaluate doesn't query the database at all. I
>> don't know why it would need to.
>
> Yeah, it has moved. Sorry.
>
> But I mean the part that changes the values in the database (on behalf of
> bin/evaluate).
> So now it's the procedure "evaluate" in src/cuirass/base.scm .
>
>> > Otherwise - as it is now in master - it can happen that while you are
>> > querying one
>> > page, half of the things have different values than you requested - which
>> > is really
>> > weird.
>> >
>> > For example right now you could query for a row with status=42 and get
>> > back data with
>> > status=43 (because it has been changed in the mean time).
>>
>> Could you please show an example that I can reproduce? I don't
>> understand what you mean.
>
> Right now something like this happens (simplified to make it easier to follow
> - finding
> the problem by debugging the Javascript frontend (wrongly) was much more
> difficult):
>
> Connection 1:
>
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
>
> Connection 2:
>
> ... Wait some time until the user sends a request...
> Query: SELECT x FROM a
> Result: Nondeterministic number
> Query: SELECT x FROM a
> Result: Nondeterministic possibly different number (WTF!!!!!)
>
> This is especially bad if you request extra data from other tables in an extra
> query and the join condition suddenly doesn't match (and thus the row isn't
> returned!).
>
>
> Better would be if it acted like this:
>
> Connection 1:
>
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
> Statement: UPDATE a SET x = x + 1
>
> Connection 2:
>
> ... Wait some time until the user sends a request...
> Statement: BEGIN TRANSACTION
> Statement: SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
> Query: SELECT x FROM a
> Result: Some number
> Query: SELECT x FROM a
> Result: The same number
> ... wait however long you want
> Query: SELECT x FROM a
> Result: The same number
> Statement: ROLLBACK TRANSACTION or COMMIT TRANSACTION
>
> loop
>
> Statement: BEGIN TRANSACTION
> Statement: SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
> Query: SELECT x FROM a
> Result: Some possibly different number xxx
> Query: SELECT x FROM a
> Result: The same number xxx as in the previous query
> Query: SELECT x FROM a
> Result: The same number xxx as in the previous query
> ...
>
>> With that patch, database queries are serialized, which means that if
>> query1, query2 and query3 are sent in that order, they will be executed
>> in that order, one after the other.
>
> It depends on what exactly that means. As long as it means that the
> entire HTTP request handler is ONE query that is ordered such, that's fine.
>
> Otherwise not.
>
> If there are more complicated multiple queries done by the web interface
> on behalf of the user because of one HTTP request, we have to make sure
> that those queries execute without any interleaving by some writer.
>
> As a stopgap, this database query serializer should let the user batch
> the queries/statements and run each batch in its own transaction.
> I think that would be quite okay as a solution, actually, as long as
> there are no other shadow clients of the database.
>
>> Currently, there is only one connection that is shared by the writers
>> and readers. Having one 'read connection' and one 'write connection'
>> would be possible and make sense if both of them live in a separate
>> thread and use the SQLite multithreading feature so that writing and
>> reading proceed concurrently. Is that what you mean?
>
> No.
- bug#32234: [PATCH 1/2] utils: Avoid deadlock when WITH-CRITICAL-SECTION calls are nested., Clément Lassieur, 2018/08/06
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread., Clément Lassieur, 2018/08/06
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread., Clément Lassieur, 2018/08/06
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread., Ludovic Courtès, 2018/08/19
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread., Clément Lassieur, 2018/08/26
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread., Ludovic Courtès, 2018/08/26
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread., Clément Lassieur, 2018/08/27
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread., Danny Milosavljevic, 2018/08/27
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread., Clément Lassieur, 2018/08/27
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread., Danny Milosavljevic, 2018/08/27
- bug#32234: [PATCH 2/2] database: Serialize all database accesses in a thread.,
Clément Lassieur <=