Monitoring and re-establishing a PostgreSQL connection

Within a complicated Ruby project, I need to monitor the connection to a PostgreSQL database, as answered here. I use a thread for this purpose. Every few milliseconds, I invoke "consume_input" which will raise an exception of the database connection no longer exists. If might have been closed for a number of reasons, but there is no indication from the socket/connection why it is closed. If the server is restarted and is alive again within a certain time-frame, I reconnect and assume everything is fine. Otherwise, I notify another object via a callback that all is not well, and the thread exits normally.

I'm looking for tips and suggestions on following best practices and correctness. I need some general Ruby advice on making my code cleaner and more robust.

PG_MAX_FAILURES=10
PG_SLEEP_QUANTA=0.25
def monitor_start
log.info("db: #{__method__}")
_dbconn=nil
if @monitor.alive?
@monitor.kill
sleep PG_SLEEP_QUANTA
end
_failures=0
while true do
begin
if ! _dbconn.is_a?PG::Connection or _dbconn.status() != PG::CONNECTION_OK
log.info("Reconnecting...")
_dbconn = PG.connect( @dbconnstr )
log.info("Reconnected.")
else
_dbconn.consume_input()
_failures=0
end
sleep PG_SLEEP_QUANTA
_failures += 1
if _failures >= PG_MAX_FAILURES
@monitor_post_callback.call()
end
log.warn("Postgresql connection lost. Retry# #{_failures}...")
sleep PG_SLEEP_QUANTA
retry
rescue => ex
log.warn("Other exception #{ex.class} #{ex.message}")
end
end
}
end


What follows is just my subjective opinion; you can disagree.

1. Robust code is maintainable. You need to be able to change this code if you find a bug in it, or if the requirements change. It might even be that someone else may need to maintain this code. So you should probably use a regular indenting pattern (2 spaces is normal for Ruby). That would make it a lot easier to see what was going on.

2. Likewise, your code would be cleaner if it was split into separate functions. Making the contents of the thread.new block a separate function would be a good place to start, but you could (and maybe should) go further than that.

3. It seems to me that the sleep ... retry at the end of the first rescue block are redundant, since you are going to loop around again anyway. YMMV.

• If the read fails, there is no other point where sleep is called. But I see your point that retry is technically redundant. Two-space indentation noted, thank you. – Otheus May 29 '15 at 12:27
• Can't split into two functions here. The code preceding the Thread is to make sure that multiple instances of the thread aren't run concurrently. – Otheus Jun 3 '15 at 8:19
• @monitor = Thread.new { thread_body_function } ? – Andy Jones Jun 3 '15 at 12:54
• Hrm, that might make unit-testing easier. – Otheus Jun 3 '15 at 15:16
• Exactly. The more you break the code up, the easier it is to read (and therefore maintain) and the easier it is to test. And if you pass _dbconn into thread_body_function it gets even better: dependancy injection makes the tests even easier (mocks & stubs) plus it makes it easier to break the code up further. – Andy Jones Jun 4 '15 at 7:44