Logical replication launcher uses wal_retrieve_retry_interval

On 14/04/17 12:57, Masahiko Sawada wrote:

Hi,

I noticed that the logical replication launcher uses
wal_retrieve_retry_interval as a interval of launching logical
replication worker process. This behavior is not documented and I
guess this is no longer consistent with what its name means.

Yes that was done based on reviews (and based on general attitude of not
adding more knobs that are similar in meaning). It is briefly documented
in the replication config section. Same is true for wal_receiver_timeout
btw.

I think that we should either introduce a new GUC parameter (say
logical_replication_retry_interval?) for this or update the
description of wal_retrieve_retry_interval. IMO the former is better.

I am not quite sure adding more GUCs is all that great option. When
writing the patches I was wondering if we should perhaps rename the
wal_receiver_timeout and wal_retrieve_retry_interval to something that
makes more sense for both physical and logical replication though.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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

On Fri, Apr 14, 2017 at 9:19 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

I am not quite sure adding more GUCs is all that great option. When
writing the patches I was wondering if we should perhaps rename the
wal_receiver_timeout and wal_retrieve_retry_interval to something that
makes more sense for both physical and logical replication though.

It seems to me that you should really have a different GUC,
wal_retrieve_retry_interval has been designed to work in the startup
process, and I think that it should still only behave as originally
designed. And at some point I think that it would make as well sense
to be able to make this parameter settable at worker-level.
--
Michael

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

On Fri, Apr 14, 2017 at 9:19 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

On 14/04/17 12:57, Masahiko Sawada wrote:

Hi,

I noticed that the logical replication launcher uses
wal_retrieve_retry_interval as a interval of launching logical
replication worker process. This behavior is not documented and I
guess this is no longer consistent with what its name means.

Yes that was done based on reviews (and based on general attitude of not
adding more knobs that are similar in meaning). It is briefly documented
in the replication config section. Same is true for wal_receiver_timeout
btw.

Thank you for the comment!

These two parameters are classed as a standby server parameter in the
document. We might want to fix it.

I think that we should either introduce a new GUC parameter (say
logical_replication_retry_interval?) for this or update the
description of wal_retrieve_retry_interval. IMO the former is better.

I am not quite sure adding more GUCs is all that great option. When
writing the patches I was wondering if we should perhaps rename the
wal_receiver_timeout and wal_retrieve_retry_interval to something that
makes more sense for both physical and logical replication though.

It would work but IMO having multiple different behaviors for one GUC
parameter is not good design.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

On 14/04/17 14:30, Michael Paquier wrote:

On Fri, Apr 14, 2017 at 9:19 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

I am not quite sure adding more GUCs is all that great option. When
writing the patches I was wondering if we should perhaps rename the
wal_receiver_timeout and wal_retrieve_retry_interval to something that
makes more sense for both physical and logical replication though.

It seems to me that you should really have a different GUC,
wal_retrieve_retry_interval has been designed to work in the startup
process, and I think that it should still only behave as originally
designed.

Ah yeah I am actually confusing it with wal_receiver_timeout which
behaves same for wal_receiver and subscription worker. So yeah it makes
sense to have separate GUC (I wonder if we then need yet another one for
tablesync though since both of those will be controlling restarts of
subscription workers after crash).

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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

On Fri, Apr 14, 2017 at 9:59 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

On 14/04/17 14:30, Michael Paquier wrote:

On Fri, Apr 14, 2017 at 9:19 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

I am not quite sure adding more GUCs is all that great option. When
writing the patches I was wondering if we should perhaps rename the
wal_receiver_timeout and wal_retrieve_retry_interval to something that
makes more sense for both physical and logical replication though.

It seems to me that you should really have a different GUC,
wal_retrieve_retry_interval has been designed to work in the startup
process, and I think that it should still only behave as originally
designed.

Ah yeah I am actually confusing it with wal_receiver_timeout which
behaves same for wal_receiver and subscription worker. So yeah it makes
sense to have separate GUC

Attached two patches add new GUCs apply_worker_timeout and
apply_worker_launch_interval which are used instead of
wal_receiver_timeout and wal_retrieve_retry_timeout. These new
parameters are not settable at worker-level so far.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On 4/16/17 22:40, Masahiko Sawada wrote:

Attached two patches add new GUCs apply_worker_timeout and
apply_worker_launch_interval which are used instead of
wal_receiver_timeout and wal_retrieve_retry_timeout. These new
parameters are not settable at worker-level so far.

Under what circumstances are these needed? Does anyone ever set these?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

On 18/04/17 16:24, Peter Eisentraut wrote:

On 4/16/17 22:40, Masahiko Sawada wrote:

Attached two patches add new GUCs apply_worker_timeout and
apply_worker_launch_interval which are used instead of
wal_receiver_timeout and wal_retrieve_retry_timeout. These new
parameters are not settable at worker-level so far.

Under what circumstances are these needed? Does anyone ever set these?

Personally I don't see need for apply_worker_timeout, no idea why that
can't use wal_receiver_timeout, the mechanics are exactly same, and it's
IMHO only needed because default tcp keepalive settings are usually too
generous. As for apply_worker_launch_interval, I think we want different
name so that it can be used for tablesync rate limiting as well.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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

On 4/18/17 12:00, Petr Jelinek wrote:

As for apply_worker_launch_interval, I think we want different
name so that it can be used for tablesync rate limiting as well.

But that's a mechanism we don't have yet, so maybe we should design that
when we get there?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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