Re: timeout of pg_receivexlog --status-interval

From: <furuyao(at)pm(dot)nttdata(dot)co(dot)jp>
To: <sawada(dot)mshk(at)gmail(dot)com>, <masao(dot)fujii(at)gmail(dot)com>
Cc: <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: timeout of pg_receivexlog --status-interval
Date: 2014-07-23 06:55:26
Message-ID: A9C510524E235E44AE909CD4027AE196BF7C70D168@MBX-MSG-SV03.msg.nttdata.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> >> 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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kalai R 2014-07-23 06:59:53 Re:
Previous Message John R Pierce 2014-07-23 06:50:55 Re: