Skip to content

Commit

Permalink
Don't read from socket when data is pending in conn.block
Browse files Browse the repository at this point in the history
conn.block still reads data from the socket, but only after checking the result queue and waiting for readability first.
Reading data from the socket (PQconsumeInput) takes much more time than just checking available results (PQisBusy).
So doing it as a precaution to conn.block is a performance issue, now that we call block before each get_result in pg-1.3.x.

The test was introduced in commit 09bdc16 .
It is not particular specific to conn.block, so that we can check the error class on consume_input equaly.

Fixes #442
  • Loading branch information
larskanis committed Mar 9, 2022
1 parent 8ad8754 commit 7dd19cb
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 12 deletions.
10 changes: 1 addition & 9 deletions ext/pg_connection.c
Expand Up @@ -2339,21 +2339,12 @@ pg_rb_io_wait(VALUE io, VALUE events, VALUE timeout) {
static void *
wait_socket_readable( VALUE self, struct timeval *ptimeout, void *(*is_readable)(PGconn *))
{
VALUE socket_io;
VALUE ret;
void *retval;
struct timeval aborttime={0,0}, currtime, waittime;
VALUE wait_timeout = Qnil;
PGconn *conn = pg_get_pgconn(self);

socket_io = pgconn_socket_io(self);

/* Check for connection errors (PQisBusy is true on connection errors) */
if ( PQconsumeInput(conn) == 0 ) {
pgconn_close_socket_io(self);
rb_raise( rb_eConnectionBad, "PQconsumeInput() %s", PQerrorMessage(conn) );
}

if ( ptimeout ) {
gettimeofday(&currtime, NULL);
timeradd(&currtime, ptimeout, &aborttime);
Expand All @@ -2368,6 +2359,7 @@ wait_socket_readable( VALUE self, struct timeval *ptimeout, void *(*is_readable)

/* Is the given timeout valid? */
if( !ptimeout || (waittime.tv_sec >= 0 && waittime.tv_usec >= 0) ){
VALUE socket_io = pgconn_socket_io(self);
/* Wait for the socket to become readable before checking again */
ret = pg_rb_io_wait(socket_io, RB_INT2NUM(PG_RUBY_IO_READABLE), wait_timeout);
} else {
Expand Down
6 changes: 3 additions & 3 deletions spec/pg/connection_spec.rb
Expand Up @@ -1449,15 +1449,15 @@ def interrupt_thread(exc=nil)
conn.finish
end

it "block should raise ConnectionBad for a closed connection" do
it "consume_input should raise ConnectionBad for a closed connection" do
serv = TCPServer.new( '127.0.0.1', 54320 )
conn = described_class.connect_start( '127.0.0.1', 54320, "", "", "me", "xxxx", "somedb" )
while [PG::CONNECTION_STARTED, PG::CONNECTION_MADE].include?(conn.connect_poll)
sleep 0.1
end
serv.close
expect{ conn.block }.to raise_error(PG::ConnectionBad, /server closed the connection unexpectedly/)
expect{ conn.block }.to raise_error(PG::ConnectionBad, /can't get socket descriptor|connection not open/)
expect{ conn.consume_input }.to raise_error(PG::ConnectionBad, /server closed the connection unexpectedly/)
expect{ conn.consume_input }.to raise_error(PG::ConnectionBad, /can't get socket descriptor|connection not open/)
end

it "calls the block supplied to wait_for_notify with the notify payload if it accepts " +
Expand Down

0 comments on commit 7dd19cb

Please sign in to comment.