I came upon an interesting problem a few days ago at work. I had some code inside an active record transaction that updated some record in the db, called an external service, and then updated another record based on the response from that external service. It was something like this:
def call
ActiveRecord::Base.transaction do
record1.update!(some_data)
external_service_response = Communicator.call!(record1, save_error: true) # raises exception if response is not a success
record2.update!(external_service_response)
end
end
The communicator class uses record1
data to build a request and call an external service. It also handles authentication and response parsing; if the response is a success, it parses and returns it. otherwise, it updates a record in the db with the error response if the save_error
flag is set to true
, and finally raises an exception, causing the active record transaction to roll back everything.
The Problem
the issue was that the error response was not saved to the db. Do you see why?! Because it's inside the transaction; so when the communicator class raises an exception, the transaction rolls back everything, including the operation to save the error message to the database. well, duh.
The Solution
the most obvious solution and the one you should go for in most cases is to move the communicator call outside the transaction, and the save error operation won't be rolled back. However, I couldn't do that so I had to do more thinking and googling...
I came across another neat solution and definitely learned something new. It uses the fact that an active record transaction "acts on a single database connection"; which basically means that if we use a different connection -from the one the transaction is using- to update a record, the transaction won't cover the update operation, and it won't be rolled back in case of an exception.
So, to solve my problem, I need to make a new thread (since each thread will use a different database connection), obtain a connection, and update the record with the error response.
ActiveRecord::Base.connection_pool
and to achieve all that, and to manage the database connections properly and make sure it's thread safe, we have ActiveRecord::Base.connection_pool
From the docs:
A connection pool synchronizes thread access to a limited number of database connections. The basic idea is that each thread checks out a database connection from the pool, uses that connection, and checks the connection back in. ConnectionPool is completely thread-safe, and will ensure that a connection cannot be used by two threads at the same time, as long as ConnectionPool's contract is correctly followed.
and so the final solution becomes:
# communicator class
#
# in case of error response
def save_error_response(record)
Thread.new do
ActiveRecord::Base.connection_pool.with_connection do
record.update(external_service_error_response)
end
end.join
end
the with_connection
method will check out a connection, yield to the block (update the record), and check in the connection again to the pool after finishing. We also need to join
the thread to make sure the main thread will wait for it to finish before exiting.
That's it! Now, the error response will be saved to the database and won't be rolled back in case of an exception as it's in a different thread and different connection from the transaction.
I hope my explanation of the problem and the solution was clear.
this post was also published on medium.
Top comments (14)
Hi Amr! I didn't know about this. There's one thing that leaves me doubtful though.
You said you couldn't move the update operation outside the transaction, but why is that? Because the solution you employed might affect performance if you have many requests (not because of the short lived thread but because you're basically spawning connections in N + 1 pools where N is the number of times you call that function concurrently).
This way you have the main pool, plus a new connection pool (albeit short lived) for every request that calls
save_error_response
, if I understood correctly what you're saying and what Rails is doing, that is.This means that instead of pooling the fixed number of connections the main pool has you're potentially instantiating a connection to the DB per each error response.
In some cases, you might not see it if you're lucky, but you could potentially overwhelm the DB because you're writing with too many connections at once.
An alternative, if your DB supports it, is to use savepoints: basically, inside the initial transaction, you call the first update, set a save point (kind like a bookmark), call the other two functions that can rollback, if they rollback they do it until the save point, instead of voiding the entire transaction. A sub-transaction in a way.
How do you achieve that? With nested transaction (only if you have a recent version of MySQL or PostgreSQL). You'll notice in the logs that they are using savepoints for each nested transaction you set.
You can read how to do employ them with Rails here: Nested transactions.
Using a sub transaction doesn't increase the number of transactions running in any given time, you're still respecting the limits set by the pool.
Hope this is a valid alternative for you
Hey rhymes,
thank you for taking the time to write this. I couldn't move the communicator call outside the transaction because then I'd have to also move the first update operation in the transaction too because the communicator depends on it:
it's not great design but it's what i'm working with.
Regarding your concern about performance, I'm not actually spawning N + 1 connections; there's a fixed number of connections in the pool that you can set (defaults to 5), and
ActiveRecord::Base.connection_pool
handles the case when there are more threads than connections as per the docmentation:so, for example, if we set the max connections in the pool to 5 and we have 10 threads, 5 connections will be used by 5 threads and when one of these threads finishes using a connection, it will be checked in to the pool and ready to be used by one of the other 5 threads and so on.
I really like the idea of savepoints. I'd even prefer it over my current solution, but I'm not sure how to make it work in this case because the statement I don't want to roll back is sandwhiched between two statements I want to roll back :D Thank you for telling me about this, though!
That's true, I don't know why but I understood that you said Rails was creating a new pool for each thread 😅
What about moving using an explicit transaction only for the first two statements? Let's see:
Something like:
would that do?
hmm, good thinking, but I don't think it would work :D the save error operation inside the communicator class would still be rolled back.
What do you think?
I don't understand what you really want to accomplish then :D
The logic seems to be too complicated for these three lines of code.
Update 1 should be rolled back if there's an error in
Communicator
but the save inside communicator should happen regardless of there being an error and update 2 should happen only if communicator went along fine?If you have control on the
Communicator
object you should probably keep the logic of calling the external service and remove the one that's giving you pain (which shouldn't be there in theory) which is the part about saving the error in the DB.If the communicator only communicates you can then leave all the DB logic in your "main transaction" and decide what to do. Something like:
Does it make sense?
yes, exactly.
it is required to save the error response to the db, but maybe it could be done outside of communicator. I'll have to think about this some more.
thank you again, rhymes, for taking the time to discuss this!
See it like this. The service object should be responsible for one thing only. In the future you might want to do N different things with the response. You might want to save it on the DB, you might want to log it on an external service, you might want to extract info from it. Separating the two steps should simplify the logic. In an extreme case you might even save the error later and asynchronously ;)
yeah, maybe another service could handle dealing with the response and whatever we want to do with it :D
Maybe you are using Rails 4, because in Rails 5 this method just locked my whole rails server. It seems the new way is to use the ThreadPoolExecutor as described in Wrapping Application Code
Just thought I'd share this with those that had the same problem I did. I was trying to cache an API response inside of an ActiveRecord validation method. Now I do it like this:
Yes, I was using Rails 4. Thank you for sharing this, Eric!
I am almost sure you can achieve the same goal with a rescue clause.
Your communicator class should not be responsible for updating the record1 in case of failures, that's your app logic.
hmm, will the exception raised outside the transaction block cause the transaction to roll back?
Hmm no, the error raised inside the transaction block will force the transaction to rollback, but ActiveRecord will re-raise the same error, here I'm rescuing after the rollback has occurred, then re-raising again. If you don't need to re-raise the error again you can simply omit it.
api.rubyonrails.org/classes/Active...
that is a great solution. I don't know why I haven't thought of that :D Thank you so much, Edgar!