Add connection retrying to ensure that the connection is automatically restored

[sdogruyol]

No description provided.

[shosti]

I'm confused as to what the original problem was and why this fixes it. Why would doing 2 retries be better than 1 in this case? (One should have been enough...) Or is it just PGError vs PG::Error?

[sdogruyol]

To validate that we don't create 2 transactions, I've used the following approach

Created a MockTransactionApi which is wrapped by ActiveRecord::Base.transaction and has a dummy endpoint like

get '/' do
    sleep 5
    DummyJob.do(1)
end

The DummyJob would be something like

class DummyJOb < QueueClassicPlus::Base
  @queue = :dummy
  lock!

  def self.perform(cf_job_id)
    # do_something
  end
end

PG has select txid_current(); to get the current transaction id from the connection. In the following log hitting the endpoint shows that QC also uses the the same transaction acquired by ActiveRecord::Base.transaction to execute, which means we don't break the connection with this change 👍

TRANSACTIONS COUNT
1
TRANSACTION ID
771444
QC INSIDE TRANSACTIONS COUNT
1
QC INSIDE TRANSACTION ID
771444

Adding a sleep 5 statement to execution before enquing and restarting pg shows that the wrapping ActiveRecord::Base.transaction raises.

PG::ConnectionBad: PQsocket() can't get socket descriptor: ROLLBACK

The enqueued QC job fails within this transaction then on the retry (which this PR adds) establishes a new connection with a new tx id.

*First Transaction*
TRANSACTIONS COUNT
1
TRANSACTION ID
771451
QC INSIDE TRANSACTIONS COUNT
1
FATAL:  terminating connection due to administrator command

*QC retry*
QC INSIDE TRANSACTIONS COUNT
WARNING:  there is no transaction in progress
QC INSIDE TRANSACTION ID
771452

Note that WARNING: there is no transaction in progress shows that on retry we get a connection which does not start a ActiveRecord::Base.transaction.

This mean that we're safe regarding transactions 👍

About PG::Error and PGError.

[1] pry(main)> PG::Error == PGError
=> true

Seems like they're aliases, however while testing I've seen that rescue PGError was not rescuing every exception 🤔

[shosti]

I'm unsure as to whether this is actually testing what I'm worried about. What I'd like to see is something along the lines of:

ActiveRecord::Base.transaction do
  puts "ActiveRecord Transaction"
  puts ActiveRecord::Base.connection.execute('select txid_current()').to_a
  puts "QC Transaction"
  puts QC.default_conn_adapter.connection.exec('select txid_current()').to_a
end

after the reconnect (during a request) and verifying that they are the same.