Comments (8)
Well Diesel's prevents that ^^
I think that should be restricted to diesel::Connection::transaction
, as it is easily possible to just open a transaction via diesel::connection::TransactionManager::begin
without closing it later. Nevertheless I think it would be really helpful to have additional methods in CustomizeConnection
that allows users to perform actions on connection checkout/checkin. Maybe even allow to control modify the is_valid()
/has_broken()
functions there. As far as I can tell these methods can be added without breaking change, as we could add default implementations can just call the existing functions. If you are interested in this I can open a PR next week.
from r2d2.
I am a bit confused as to how Diesel simultaneously guarantees that transactions will always be committed and needs the ability to forcibly roll back transactions when a pooled connection is returned.
In any case though, it seems reasonable to add those methods to CustomizeConnection.
from r2d2.
I am a bit confused as to how Diesel simultaneously guarantees
I realize my previous messages may have been lacking explanation to be understandable by anyone who isn't quite familiar with diesel and its issue history ^^
I'm going to try and give more details :)
that transactions will always be committed
It does not precisely "guarantees that transactions will always be committed", however it does provide a way for users to "make sure to never forget to make the commit call".
If you were to have a guard-based transaction API, it would probably look something like this:
let transaction = conn.begin_transaction()?;
query.execute(&mut transaction.connection);
transaction.commit()?;
Now this has several issues:
- If you rollback on dropping the guard, you don't really know what to do with errors that happen when attempting rollback.
- If you forget to make write the line with the commit call, the transaction would get rolled back (or at least attempted to be rolled back), and it's hard to check that you won't ever forget to write it.
(What I call "forget to make the commit call" is specifically "forgetting to write that last line, resulting in transaction never getting committed".)
On the other hand, Diesel's API is based on a Connection::transaction
function:
conn.transaction(|conn| {
query.execute(&mut conn);
Ok(())
})?;
Notice several things:
- Rollback errors are easily returned by the
transaction
function - It's impossible to "just forget" to specify whether to rollback or commit the transaction: that is specified the closure's
Ok
orErr
result, and the code will obviously fail to compile if it contains neither.
Hence indeed, "always using the Connection::transaction
when dealing with transactions" constitutes "a way for users to make sure to never forget to make the commit call"
Anyway - I realize now this is irrelevant to this issue, for reasons described later.
needs the ability to forcibly roll back transactions when a pooled connection is returned
Now there's actually two ways that this could be useful:
- people not using the
transaction
abstraction for some reason - There's actually another way we can exit the
transaction
function, besidesOk
andErr
(last one I promise!): by panicking out of it.
For that second case we have several design choices:
- Catch the panic and attempt a rollback
- blocking and possibly time consuming
- delaying the panic unwind
- is useless if the actual connection (not pooled) is gonna get dropped right after due to the panic)
- possibly silently failing
- Not do anything and let the user deal with it if they want to
Notice in particular that attempting rollback only ever makes sense if the connection is pooled - otherwise it's probably fine to just let the panic unwind and drop the actual connection, which will not be reused.
Diesel went with the second approach, which in particular allows users to, depending on their use-case:
- just let the connection get dropped/closed because the program will quit soon enough that all connections will be closed hence rolled back by the database
- attempt rollback themselves, and actually log the error somewhere if it happens
- do whatever else they see fit
Now if the user's will happens to be "I know this panic in my request handling code will be catched later, and the server will continue its life normally besides that, so I want to attempt rollback and report the error to e.g. Sentry if it fails" (spoiler: that's what we do), then the current only possible approach is
having custom wrappers around
PooledConnection
andPool
, which prevents from always using r2d2 directly (and requires reimplementing a whole bunch of traits on the wrappers).
which having this would solve.
Side note: I realize now that the "no-rollback-attempt on panic" behavior and "don't forget to commit" behavior are pretty untied: it's possible to obtain both "no-rollback-attempt on panic" and "rollback-attempt on panic" in both designs, by checking whether std::thread::panicking
in the guard's Drop
impl, and using std::panic::catch_unwind
in closure-based interface.
That makes the only relevant question between "no-rollback-attempt on panic" and "don't forget to commit" with regards to this issue the "no-rollback-attempt on panic" one:
Because we don't want a hardcoded "rollback-attempt on panic" for their previously cited downsides, it would be very handy to let users parametrize how they want to handle not-rolled-back transactions that come back to the pool.
Not to negate of course the other uses this may have, like managing a pool of connections dedicated to tests, where we would open a transaction everytime we get it from the pool, and roll it back every time it comes back, which is very close to our original issue.
from r2d2.
2. have the issues described here)
Those issues are described in the context of an async API, but r2d2 works with blocking connections.
from r2d2.
Those issues are described in the context of an async API, but r2d2 works with blocking connections.
I think the points about "how would one handle connection errors in the Drop implementation?" (also mentionned here) are still valid in a synchronous context.
(In addition, how do you make sure to never forget to make the commit call with a such API?)
Anway, that is just to justify that it isn't easy or even necessarily desirable to "just move to an RAII transaction API" that would dodge these transaction-management-on-pooled-connection-drop issues by other means.
from r2d2.
(In addition, how do you make sure to never forget to make the commit call with a such API?)
How would you make sure to never forget a commit call in any API?
from r2d2.
How would you make sure to never forget a commit call in any API?
Well Diesel's prevents that ^^
from r2d2.
I've opened #131 for this.
from r2d2.
Related Issues (20)
- Is there a way to not keep idle connections? HOT 8
- `get_timeout` exceeds timeout HOT 11
- Can be used in the future such as tokio, async_std? HOT 1
- Use `compare_exchange` or `compare_exchange_weak` instead
- Is there a way to drop or close an actual connection when I'm using PooldConnection? HOT 4
- "Named" connections HOT 3
- Why example use `let pool = pool.clone()` in a for loop ? HOT 2
- Document relationship between `min_idle`, `idle_timeout` and `max_lifetime` HOT 1
- How to set timezone when linking HOT 1
- Release? HOT 1
- Using behind proxychains HOT 1
- Seg fault with Postgres HOT 14
- Remove parking_lot dependency now that std's Mutex is faster than before HOT 3
- [Bug] CI test failure on arm HOT 4
- r2d2::Error should derive PartialEq and Clone HOT 4
- Pool building freezes when connections drop during initial setup HOT 4
- Does it support ssh2 sessions?
- Add a PooledConnection::pool() method HOT 3
- Fail Fast for Certain Errors
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from r2d2.