search

elixir-sqlite / sqlitex

An Elixir wrapper around esqlite. Allows access to sqlite3 databases.
https://hex.pm/packages/sqlitex
120 stars 34 forks source link

Document/Fix race condition when using Sqlitex.Server with transactions #85

Open the-kenny opened 5 years ago

the-kenny commented 5 years ago

As far as I see, using begin and end together with Sqlitex.Server has a race condition when two processes are sending messages:

Serialized:

A -> Server: "begin"
A -> Server: "Insert into foo..."
A -> Server: "rollback"

B -> Server: "select * from foo"

Parallel:

A -> Server: "begin"
A -> Server: "Insert into ..."
B -> Server: "select * from foo"
A -> Server: "commit"

The correct result for the select * from foo of B would be [], but with this race condition, B will see the data A inserted in a transaction which gets canceled.

To me this looks like an inherent issue with the current implementation of Sqlitex.Server. However, we could work around this by moving it behind something more resembling a connection pool which will spawn a new instance for each process connecting to it, monitoring that process, and closing the connections when the process exits (or after some timeout).

Do you think this solution would work, or do you see any issues? If everything looks good, I might try to contribute a pool like this in the next few days.

the-kenny commented 5 years ago

Note that I started working on connection pooling support in https://github.com/the-kenny/sqlitex/tree/server-connection-pooling yesterday. Will keep you updated :-)

mmmries commented 5 years ago

@the-kenny I can see what you mean about the server. Because it acts as a shared connection to the database it interleaves all the calls from different clients as if they were one client.

Perhaps we should change the documentation for that module to specify that Server should not be used with transactions? That it's only useful to synchronize access to a single connection?

I'm not sure how sqlitex fits in with the rest of the elixir-sqlite tooling, so I'm not sure if database connection pooling has been tackled somewhere else. Have you tested that transactions work as expected if A and B each open their own connections?

the-kenny commented 5 years ago

@mmmries Yeah, I get the feeling that a whole connection pooling solution is a bit out of scope for this project. It's also easy to do as a separate library.

I just finished implementing another solution: Sqlitex.Server.with_transaction(server, fun). I'll open a Pull Request for review in a minute.