On Thu, Jul 28, 2016 at 9:08 PM, Fujii Masao <[email protected]> wrote:
> On Thu, Jul 28, 2016 at 6:52 PM, Kyotaro HORIGUCHI
> <[email protected]> wrote:
>> Hello,
>>
>> While testing replication for 9.5, we found that repl-master can
>> ignore wal_sender_timeout and seemingly waits for TCP
>> retransmission timeout for the case of sudden power-off of a
>> standby.
>>
>> My investigation told me that the immediate cause could be that
>> secure_write() is called with *blocking mode* (that is,
>> port->noblock = false) under *pq_putmessage_noblock* macro called
>> from XLogSendPhysical().
>>
>> libpq.h of 9.5 and newer defines it as the following,
>>
>>> #define pq_putmessage(msgtype, s, len) \
>>> (PqCommMethods->putmessage(msgtype, s, len))
>>> #define pq_putmessage_noblock(msgtype, s, len) \
>>> (PqCommMethods->putmessage(msgtype, s, len))
>>
>> which is apparently should be the following.
>>
>>> #define pq_putmessage_noblock(msgtype, s, len) \
>>> (PqCommMethods->putmessage_noblock(msgtype, s, len))
>>
>> The attached patch fixes it.
>
> Good catch! Barring objection, I will push this to both master and 9.5.
Regarding this patch, while reading pqcomm.c, I found the following things.
1. socket_comm_reset() calls pq_endcopyout().
I think that socket_endcopyout() should be called, instead.
2. socket_putmessage_noblock() calls pq_putmessage().
I think that socket_putmessage() should be called, instead.
3. Several source comments in pqcomm.c have not been updated.
Some comments still use the old function name like pq_putmessage().
Attached patch fixes the above issues.
Regards,
--
Fujii Masao
*** a/src/backend/libpq/pqcomm.c
--- b/src/backend/libpq/pqcomm.c
***************
*** 18,24 ****
* NOTE: generally, it's a bad idea to emit outgoing messages directly with
* pq_putbytes(), especially if the message would require multiple calls
* to send. Instead, use the routines in pqformat.c to construct the message
! * in a buffer and then emit it in one call to pq_putmessage. This ensures
* that the channel will not be clogged by an incomplete message if execution
* is aborted by ereport(ERROR) partway through the message. The only
* non-libpq code that should call pq_putbytes directly is old-style COPY OUT.
--- 18,24 ----
* NOTE: generally, it's a bad idea to emit outgoing messages directly with
* pq_putbytes(), especially if the message would require multiple calls
* to send. Instead, use the routines in pqformat.c to construct the message
! * in a buffer and then emit it in one call to socket_putmessage. This ensures
* that the channel will not be clogged by an incomplete message if execution
* is aborted by ereport(ERROR) partway through the message. The only
* non-libpq code that should call pq_putbytes directly is old-style COPY OUT.
***************
*** 44,50 ****
* StreamClose - Close a client/backend connection
* TouchSocketFiles - Protect socket files against /tmp cleaners
* pq_init - initialize libpq at backend startup
! * pq_comm_reset - reset libpq during error recovery
* pq_close - shutdown libpq at backend exit
*
* low-level I/O:
--- 44,50 ----
* StreamClose - Close a client/backend connection
* TouchSocketFiles - Protect socket files against /tmp cleaners
* pq_init - initialize libpq at backend startup
! * socket_comm_reset - reset libpq during error recovery
* pq_close - shutdown libpq at backend exit
*
* low-level I/O:
***************
*** 53,68 ****
* pq_getmessage - get a message with length word from connection
* pq_getbyte - get next byte from connection
* pq_peekbyte - peek at next byte from connection
! * pq_putbytes - send bytes to connection (not flushed until pq_flush)
! * pq_flush - flush pending output
! * pq_flush_if_writable - flush pending output if writable without blocking
* pq_getbyte_if_available - get a byte if available without blocking
*
* message-level I/O (and old-style-COPY-OUT cruft):
! * pq_putmessage - send a normal message (suppressed in COPY OUT mode)
! * pq_putmessage_noblock - buffer a normal message (suppressed in COPY OUT)
! * pq_startcopyout - inform libpq that a COPY OUT transfer is beginning
! * pq_endcopyout - end a COPY OUT transfer
*
*------------------------
*/
--- 53,68 ----
* pq_getmessage - get a message with length word from connection
* pq_getbyte - get next byte from connection
* pq_peekbyte - peek at next byte from connection
! * pq_putbytes - send bytes to connection (not flushed until socket_flush)
! * socket_flush - flush pending output
! * socket_flush_if_writable - flush pending output if writable without blocking
* pq_getbyte_if_available - get a byte if available without blocking
*
* message-level I/O (and old-style-COPY-OUT cruft):
! * socket_putmessage - send a normal message (suppressed in COPY OUT mode)
! * socket_putmessage_noblock - buffer a normal message (suppressed in COPY OUT)
! * socket_startcopyout - inform libpq that a COPY OUT transfer is beginning
! * socket_endcopyout - end a COPY OUT transfer
*
*------------------------
*/
***************
*** 109,115 **** static List *sock_paths = NIL;
* Buffers for low-level I/O.
*
* The receive buffer is fixed size. Send buffer is usually 8k, but can be
! * enlarged by pq_putmessage_noblock() if the message doesn't fit otherwise.
*/
#define PQ_SEND_BUFFER_SIZE 8192
--- 109,115 ----
* Buffers for low-level I/O.
*
* The receive buffer is fixed size. Send buffer is usually 8k, but can be
! * enlarged by socket_putmessage_noblock() if the message doesn't fit otherwise.
*/
#define PQ_SEND_BUFFER_SIZE 8192
***************
*** 223,229 **** socket_comm_reset(void)
/* Do not throw away pending data, but do reset the busy flag */
PqCommBusy = false;
/* We can abort any old-style COPY OUT, too */
! pq_endcopyout(true);
}
/* --------------------------------
--- 223,229 ----
/* Do not throw away pending data, but do reset the busy flag */
PqCommBusy = false;
/* We can abort any old-style COPY OUT, too */
! socket_endcopyout(true);
}
/* --------------------------------
***************
*** 1302,1308 **** pq_getmessage(StringInfo s, int maxlen)
/* --------------------------------
! * pq_putbytes - send bytes to connection (not flushed until pq_flush)
*
* returns 0 if OK, EOF if trouble
* --------------------------------
--- 1302,1308 ----
/* --------------------------------
! * pq_putbytes - send bytes to connection (not flushed until socket_flush)
*
* returns 0 if OK, EOF if trouble
* --------------------------------
***************
*** 1444,1450 **** internal_flush(void)
}
/* --------------------------------
! * pq_flush_if_writable - flush pending output if writable without blocking
*
* Returns 0 if OK, or EOF if trouble.
* --------------------------------
--- 1444,1450 ----
}
/* --------------------------------
! * socket_flush_if_writable - flush pending output if writable without blocking
*
* Returns 0 if OK, or EOF if trouble.
* --------------------------------
***************
*** 1542,1548 **** fail:
}
/* --------------------------------
! * pq_putmessage_noblock - like pq_putmessage, but never blocks
*
* If the output buffer is too small to hold the message, the buffer
* is enlarged.
--- 1542,1548 ----
}
/* --------------------------------
! * socket_putmessage_noblock - like socket_putmessage, but never blocks
*
* If the output buffer is too small to hold the message, the buffer
* is enlarged.
***************
*** 1563,1569 **** socket_putmessage_noblock(char msgtype, const char *s, size_t len)
PqSendBuffer = repalloc(PqSendBuffer, required);
PqSendBufferSize = required;
}
! res = pq_putmessage(msgtype, s, len);
Assert(res == 0); /* should not fail when the message fits in
* buffer */
}
--- 1563,1569 ----
PqSendBuffer = repalloc(PqSendBuffer, required);
PqSendBufferSize = required;
}
! res = socket_putmessage(msgtype, s, len);
Assert(res == 0); /* should not fail when the message fits in
* buffer */
}
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
