DEV Community

Discussion on: TIL: Using Different Database Connection with ActiveRecord Transactions

Collapse
rhymes profile image
rhymes • Edited

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

Collapse
amrrbakry profile image
Amr El-Bakry Author

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:

  ActiveRecord::Base.transaction do
    record1.update!(some_data) # want to rollback this
    external_service_response = Communicator.call!(record1, save_error: true) # dependes on the previous line to make the call
    record2.update!(external_service_response)
  end

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:

It will also handle cases in which there are more threads than connections: if all connections have been checked out, and a thread tries to checkout a connection anyway, then ConnectionPool will wait until some other thread has checked in a connection.

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!

Collapse
rhymes profile image
rhymes

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

That's true, I don't know why but I understood that you said Rails was creating a new pool for each thread 😅

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!

What about moving using an explicit transaction only for the first two statements? Let's see:

  • if update #1 breaks, you catch the error, the write wasn't done anyway, the call to the external service won't be made and you're fine
  • if the call to the external service returns an error response you explicitly rollback the transaction, the first update gets undone, but you still have the error response
  • update #2 checks if the error response is populated, if so it writes on disk, otherwise it just doesn't do anything.

Something like:

external_service_response = nil
ActiveRecord::Base.transaction do
  record1.update!(some_data)
  external_service_response = Communicator.call!(record1, save_error: true)
end
record2.update!(external_service_response) if external_service_response

would that do?

Thread Thread
amrrbakry profile image
Amr El-Bakry Author

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?

Thread Thread
rhymes profile image
rhymes

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:

# update record 1
# call the service with record 1
# did the call went fine? yes? move on
# did the call raise an error? store the error in a variable
##### rollback the transaction
##### save the error outside the transaction bloc

Does it make sense?

Thread Thread
amrrbakry profile image
Amr El-Bakry Author

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?

yes, exactly.

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.

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!

Thread Thread
rhymes profile image
rhymes

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.

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 ;)

Thread Thread
amrrbakry profile image
Amr El-Bakry Author • Edited

yeah, maybe another service could handle dealing with the response and whatever we want to do with it :D