Skip to content

Commit

Permalink
Save connection options for conn.reset
Browse files Browse the repository at this point in the history
Commit 016c17c introduced a regression, that leads to a host list duplication every time conn.reset is used.

Since the hosts are duplicated when resolve_hosts is called, the conn.coninfo_hash can not be used.
So the fix is to store the original hash of connection options for later use in conn.reset .

Fixes #586
  • Loading branch information
larskanis committed Sep 5, 2024
1 parent a2a3e6d commit bf7d57f
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 7 deletions.
1 change: 1 addition & 0 deletions ext/pg_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ pgconn_s_allocate( VALUE klass )
RB_OBJ_WRITE(self, &this->decoder_for_get_copy_data, Qnil);
RB_OBJ_WRITE(self, &this->trace_stream, Qnil);
rb_ivar_set(self, rb_intern("@calls_to_put_copy_data"), INT2FIX(0));
rb_ivar_set(self, rb_intern("@iopts_for_reset"), Qnil);

return self;
}
Expand Down
7 changes: 6 additions & 1 deletion lib/pg/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,9 @@ def encrypt_password( password, username, algorithm=nil )
# Resets the backend connection. This method closes the
# backend connection and tries to re-connect.
def reset
iopts = conninfo_hash.compact
# Use connection options from PG::Connection.new to reconnect with the same options but with renewed DNS resolution.
# Use conninfo_hash as a fallback when connect_start was used to create the connection object.
iopts = @iopts_for_reset || conninfo_hash.compact
if iopts[:host] && !iopts[:host].empty? && PG.library_version >= 100000
iopts = self.class.send(:resolve_hosts, iopts)
end
Expand Down Expand Up @@ -825,6 +827,7 @@ def new(*args)
iopts = PG::Connection.conninfo_parse(option_string).each_with_object({}){|h, o| o[h[:keyword].to_sym] = h[:val] if h[:val] }
iopts = PG::Connection.conndefaults.each_with_object({}){|h, o| o[h[:keyword].to_sym] = h[:val] if h[:val] }.merge(iopts)

iopts_for_reset = iopts
if iopts[:hostaddr]
# hostaddr is provided -> no need to resolve hostnames

Expand All @@ -838,6 +841,8 @@ def new(*args)

raise PG::ConnectionBad, conn.error_message if conn.status == PG::CONNECTION_BAD

# save the connection options for conn.reset
conn.instance_variable_set(:@iopts_for_reset, iopts_for_reset)
conn.send(:async_connect_or_reset, :connect_poll)
conn
end
Expand Down
4 changes: 2 additions & 2 deletions spec/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -678,8 +678,8 @@ def with_env_vars(**kwargs)
end

# Append or change 'rubypg_test' host entry in /etc/hosts to a given IP address
def set_etc_hosts(hostaddr)
system "sudo --non-interactive sed -i '/.* rubypg_test$/{h;s/.*/#{hostaddr} rubypg_test/};${x;/^$/{s//#{hostaddr} rubypg_test/;H};x}' /etc/hosts" or skip("unable to change /etc/hosts file")
def set_etc_hosts(hostaddr, hostname)
system "sudo --non-interactive sed -i '/.* #{hostname}$/{h;s/.*/#{hostaddr} #{hostname}/};${x;/^$/{s//#{hostaddr} #{hostname}/;H};x}' /etc/hosts" or skip("unable to change /etc/hosts file")
end
end

Expand Down
21 changes: 21 additions & 0 deletions spec/pg/connection_async_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,25 @@ def interrupt_thread(exc=nil)
end
end

it "doesn't duplicate hosts in conn.reset", :without_transaction, :ipv6, :postgresql_10 do
set_etc_hosts "::1", "rubypg_test2 rubypg_test_ipv6"
set_etc_hosts "127.0.0.1", "rubypg_test2 rubypg_test_ipv4"
conn = described_class.connect( "postgres://rubypg_test2/test" )
conn.exec("select 1")
expect( conn.conninfo_hash[:host] ).to eq( "rubypg_test2,rubypg_test2" )
expect( conn.conninfo_hash[:hostaddr] ).to eq( "::1,127.0.0.1" )
expect( conn.conninfo_hash[:port] ).to eq( "#{@port},#{@port}" )
expect( conn.host ).to eq( "rubypg_test2" )
expect( conn.hostaddr ).to eq( "::1" )
expect( conn.port ).to eq( @port )

conn.reset
conn.exec("select 2")
expect( conn.conninfo_hash[:host] ).to eq( "rubypg_test2,rubypg_test2" )
expect( conn.conninfo_hash[:hostaddr] ).to eq( "::1,127.0.0.1" )
expect( conn.conninfo_hash[:port] ).to eq( "#{@port},#{@port}" )
expect( conn.host ).to eq( "rubypg_test2" )
expect( conn.hostaddr ).to eq( "::1" )
expect( conn.port ).to eq( @port )
end
end
8 changes: 4 additions & 4 deletions spec/pg/connection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1728,15 +1728,15 @@
end

it "refreshes DNS address while conn.reset", :without_transaction, :ipv6 do
set_etc_hosts "::1"
conn = described_class.connect( "postgres://rubypg_test/test" )
set_etc_hosts "::1", "rubypg_test1"
conn = described_class.connect( "postgres://rubypg_test1/test" )
conn.exec("select 1")

set_etc_hosts "127.0.0.1"
set_etc_hosts "127.0.0.1", "rubypg_test1"
conn.reset
conn.exec("select 1")

set_etc_hosts "::2"
set_etc_hosts "::2", "rubypg_test1"
expect do
conn.reset
conn.exec("select 1")
Expand Down

0 comments on commit bf7d57f

Please sign in to comment.