Re: [HACKERS] timeout of pg_receivexlog --status-interval

2014-07-24 Thread Fujii Masao 
On Wed, Jul 16, 2014 at 2:29 AM, Sawada Masahiko sawada.m...@gmail.com wrote:
 On Tue, Jul 15, 2014 at 7:38 PM, Fujii Masao masao.fu...@gmail.com wrote:
 On Thu, Jul 10, 2014 at 11:10 PM, Sawada Masahiko sawada.m...@gmail.com 
 wrote:
 Hi,

 At 1047 line of receivelog.c:CopyStreamPoll(), we set NULL to
 timeoutptr variable.
 if the value of timeoutprt is set NULL then the process will wait
 until can read socket using by select() function as following.

 if (timeout_ms  0)
 timeoutptr = NULL;
 else
 {
 timeout.tv_sec = timeout_ms / 1000L;
 timeout.tv_usec = (timeout_ms % 1000L) * 1000L;
 timeoutptr = timeout;
 }

 ret = select(PQsocket(conn) + 1, input_mask, NULL, NULL, timeoutptr);

 But the 1047 line of receivelog.c is never executed because the value
 of timeout_ms is NOT allowed less than 0 at CopyStreamReceive which is
 only one function calls CopyStreamPoll().
 The currently code, if we specify -s to 0 then CopyStreamPoll()
 function is never called.
 And the pg_receivexlog will be execute PQgetCopyData() and failed, in
 succession.

 Thanks for reporting this! Yep, this is a problem.

 I think that it is contradiction, and should execute select() function
 with NULL of fourth argument.
 the attached patch allows to execute select() with NULL, i.g.,
 pg_receivexlog.c will wait until can  read socket without timeout, if
 -s is specified to 0.

 Your patch changed the code so that CopyStreamPoll is called even
 when the timeout is 0. I don't agree with this change because the
 timeout=0 basically means that the caller doesn't request to block and
 there is no need to call CopyStreamPoll in this case. So I'm thinking to
 apply the attached patch. Thought?


 Thank you for the response.
 I think this is  better.

 One another point about select() function, I think that they are same
 behavior between the fifth argument is NULL and 0(i.g. 0 sec).

No, per manpage of select(2), select(2) with timeout=0 behaves differently
from select(2) with timeout=NULL. So I don't think your suggestion is
acceptable...

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] timeout of pg_receivexlog --status-interval

2014-07-23 Thread furuyao 
  Hi,
 
  At 1047 line of receivelog.c:CopyStreamPoll(), we set NULL to
  timeoutptr variable.
  if the value of timeoutprt is set NULL then the process will wait
  until can read socket using by select() function as following.
 
  if (timeout_ms  0)
  timeoutptr = NULL;
  else
  {
  timeout.tv_sec = timeout_ms / 1000L; timeout.tv_usec =
  (timeout_ms % 1000L) * 1000L;
  timeoutptr = timeout;
  }
 
  ret = select(PQsocket(conn) + 1, input_mask, NULL, NULL,
  timeoutptr);
 
  But the 1047 line of receivelog.c is never executed because the value
  of timeout_ms is NOT allowed less than 0 at CopyStreamReceive which
  is only one function calls CopyStreamPoll().
  The currently code, if we specify -s to 0 then CopyStreamPoll()
  function is never called.
  And the pg_receivexlog will be execute PQgetCopyData() and failed,
 in
  succession.
 
  Thanks for reporting this! Yep, this is a problem.
 
  I think that it is contradiction, and should execute select()
  function with NULL of fourth argument.
  the attached patch allows to execute select() with NULL, i.g.,
  pg_receivexlog.c will wait until can  read socket without timeout,
 if
  -s is specified to 0.
 
  Your patch changed the code so that CopyStreamPoll is called even when
  the timeout is 0. I don't agree with this change because the
  timeout=0 basically means that the caller doesn't request to block and
  there is no need to call CopyStreamPoll in this case. So I'm thinking
  to apply the attached patch. Thought?
 
 
 Thank you for the response.
 I think this is  better.
 
 One another point about select() function, I think that they are same
 behavior between the fifth argument is NULL and 0(i.g. 0 sec).
 so I think that it's better to change the CopyStreamPoll() as followings.
 
 @@ -1043,7 +1043,7 @@ CopyStreamPoll(PGconn *conn, long timeout_ms)
 FD_ZERO(input_mask);
 FD_SET(PQsocket(conn), input_mask);
 
 -   if (timeout_ms  0)
 +   if (timeout_ms = 0)
 timeoutptr = NULL;
 else
 {
 
 Please give me feed back.

I have no problem with either of the suggestions, if we specify -s to 0.

However, the fix of CopyStreamPoll(), I can't choose the route which doesn't 
carry out select().

I have proposed a patch that was in reference to walreceiver, 
there is a logic to continuously receive messages as walreceiver in that patch, 
and the route which doesn't carry out select() is necessary for it.

I think that a condition change of CopyStreamReceive() is better from 
expansibility. Thought?

Regards,

--
Furuya Osamu


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] timeout of pg_receivexlog --status-interval

2014-07-15 Thread Fujii Masao 
On Thu, Jul 10, 2014 at 11:10 PM, Sawada Masahiko sawada.m...@gmail.com wrote:
 Hi,

 At 1047 line of receivelog.c:CopyStreamPoll(), we set NULL to
 timeoutptr variable.
 if the value of timeoutprt is set NULL then the process will wait
 until can read socket using by select() function as following.

 if (timeout_ms  0)
 timeoutptr = NULL;
 else
 {
 timeout.tv_sec = timeout_ms / 1000L;
 timeout.tv_usec = (timeout_ms % 1000L) * 1000L;
 timeoutptr = timeout;
 }

 ret = select(PQsocket(conn) + 1, input_mask, NULL, NULL, timeoutptr);

 But the 1047 line of receivelog.c is never executed because the value
 of timeout_ms is NOT allowed less than 0 at CopyStreamReceive which is
 only one function calls CopyStreamPoll().
 The currently code, if we specify -s to 0 then CopyStreamPoll()
 function is never called.
 And the pg_receivexlog will be execute PQgetCopyData() and failed, in
 succession.

Thanks for reporting this! Yep, this is a problem.

 I think that it is contradiction, and should execute select() function
 with NULL of fourth argument.
 the attached patch allows to execute select() with NULL, i.g.,
 pg_receivexlog.c will wait until can  read socket without timeout, if
 -s is specified to 0.

Your patch changed the code so that CopyStreamPoll is called even
when the timeout is 0. I don't agree with this change because the
timeout=0 basically means that the caller doesn't request to block and
there is no need to call CopyStreamPoll in this case. So I'm thinking to
apply the attached patch. Thought?

Regards,

-- 
Fujii Masao
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 32afc8d..44b9fcc 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -1094,7 +1094,7 @@ CopyStreamReceive(PGconn *conn, long timeout, char **buffer)
 		 * No data available. Wait for some to appear, but not longer than
 		 * the specified timeout, so that we can ping the server.
 		 */
-		if (timeout  0)
+		if (timeout != 0)
 		{
 			int		ret;
 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] timeout of pg_receivexlog --status-interval

2014-07-15 Thread Sawada Masahiko 
On Tue, Jul 15, 2014 at 7:38 PM, Fujii Masao masao.fu...@gmail.com wrote:
 On Thu, Jul 10, 2014 at 11:10 PM, Sawada Masahiko sawada.m...@gmail.com 
 wrote:
 Hi,

 At 1047 line of receivelog.c:CopyStreamPoll(), we set NULL to
 timeoutptr variable.
 if the value of timeoutprt is set NULL then the process will wait
 until can read socket using by select() function as following.

 if (timeout_ms  0)
 timeoutptr = NULL;
 else
 {
 timeout.tv_sec = timeout_ms / 1000L;
 timeout.tv_usec = (timeout_ms % 1000L) * 1000L;
 timeoutptr = timeout;
 }

 ret = select(PQsocket(conn) + 1, input_mask, NULL, NULL, timeoutptr);

 But the 1047 line of receivelog.c is never executed because the value
 of timeout_ms is NOT allowed less than 0 at CopyStreamReceive which is
 only one function calls CopyStreamPoll().
 The currently code, if we specify -s to 0 then CopyStreamPoll()
 function is never called.
 And the pg_receivexlog will be execute PQgetCopyData() and failed, in
 succession.

 Thanks for reporting this! Yep, this is a problem.

 I think that it is contradiction, and should execute select() function
 with NULL of fourth argument.
 the attached patch allows to execute select() with NULL, i.g.,
 pg_receivexlog.c will wait until can  read socket without timeout, if
 -s is specified to 0.

 Your patch changed the code so that CopyStreamPoll is called even
 when the timeout is 0. I don't agree with this change because the
 timeout=0 basically means that the caller doesn't request to block and
 there is no need to call CopyStreamPoll in this case. So I'm thinking to
 apply the attached patch. Thought?


Thank you for the response.
I think this is  better.

One another point about select() function, I think that they are same
behavior between the fifth argument is NULL and 0(i.g. 0 sec).
so I think that it's better to change the CopyStreamPoll() as followings.

@@ -1043,7 +1043,7 @@ CopyStreamPoll(PGconn *conn, long timeout_ms)
FD_ZERO(input_mask);
FD_SET(PQsocket(conn), input_mask);

-   if (timeout_ms  0)
+   if (timeout_ms = 0)
timeoutptr = NULL;
else
{

Please give me feed back.

Regards,

---
Sawada Masahiko


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers