Restore disconnection-handling for PostgresqlAdapter #13397

Closed
wants to merge 4 commits into from

1 participant

@matthewd
Ruby on Rails member

Per #12867, #verify! is currently failing to recognise that the PostgreSQL connection has gone away.

The only way to truly confirm we can send a query is to do so, as was the case before 34c7e73.

So we need to put that behaviour back, while adding locking to protect against the reaper hitting a connection while it's in use.

Commits are currently split for easier discussion; this will obviously need to be squashed and changelogged before merge.

/cc @tenderlove

matthewd added some commits Dec 19, 2013
@matthewd matthewd Terminate the backend ourselves on PG 9.2+
This should make it harder to accidentally break this test.
37f833b
@matthewd matthewd Test checking #active? while a query is running
This is the behaviour that 34c7e73 was seeking to allow.
cd94d2b
@matthewd matthewd Restore functioning #active? for PostgreSQL
This reverts the substantive portion of 34c7e73.
03da376
@kainosnoema kainosnoema and 1 other commented on an outdated diff Dec 19, 2013
...tive_record/connection_adapters/postgresql_adapter.rb
- end
- self.client_min_messages = @config[:min_messages] || 'warning'
- self.schema_search_path = @config[:schema_search_path] || @config[:schema_order]
-
- # Use standard-conforming strings if available so we don't have to do the E'...' dance.
- set_standard_conforming_strings
-
- # If using Active Record's time zone support configure the connection to return
- # TIMESTAMP WITH ZONE types in UTC.
- # (SET TIME ZONE does not use an equals sign like other SET variables)
- if ActiveRecord::Base.default_timezone == :utc
- execute("SET time zone 'UTC'", 'SCHEMA')
- elsif @local_tz
- execute("SET time zone '#{@local_tz}'", 'SCHEMA')
- end
+ synchronize do

The only two methods calling #configure_connection (#connect and #reconnect!) are already synchronized, and the method comment recommends against calling this directly. Maybe it's okay to skip it here?

@matthewd
Ruby on Rails member

Well spotted; done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@matthewd matthewd Synchronize all access to the underlying PG connection
It's not thread-safe, and we need to safely issue queries from both the
owning thread and the reaper.
e60a1b8
@matthewd
Ruby on Rails member

I tried to allow the reaper to skip the lock-wait... but for some reason I'm yet to comprehend, that made things sad.

matthewd@1d14acd

@matthewd
Ruby on Rails member

We decided against using a lock

@matthewd matthewd closed this Mar 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment