Horton

Thread

Commits

  1. Fix invalid value of synchronous_commit in description of flush_lag

  1. Typo in description of replay_lag attribute in pg_stat_replication view

    Maksim Milyutin <milyutinma@gmail.com> — 2018-12-03T15:18:30Z

    Hello!


I have noticed that in description of *flush_lag* in pg_stat_replication 
view 
(https://www.postgresql.org/docs/current/monitoring-stats.html#PG-STAT-REPLICATION-VIEW) 
there exists unknown value of synchronous_commit parameter - 
*remote_flush*. I think it was meant to use the value *on*.

Small patch is attached.

-- 
Regards, Maksim Milyutin

  2. Re: Typo in description of replay_lag attribute in pg_stat_replication view

    Michael Paquier <michael@paquier.xyz> — 2018-12-04T00:18:02Z

    On Mon, Dec 03, 2018 at 06:18:30PM +0300, Maksim Milyutin wrote:
> I have noticed that in description of *flush_lag* in pg_stat_replication
> view (https://www.postgresql.org/docs/current/monitoring-stats.html#PG-STAT-REPLICATION-VIEW)
> there exists unknown value of synchronous_commit parameter - *remote_flush*.
> I think it was meant to use the value *on*.

Yes, you are right.  It should be "on" as "remote_flush" is not a valid
value.  remote_flush is listed in SyncCommitLevel though, so this makes
me wonder if we should also introduce a new value for this purpose
available for users.  The fix you propose looks good to me.  Any
opinions from others?
--
Michael

  3. Re: Typo in description of replay_lag attribute in pg_stat_replication view

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-12-04T00:28:15Z

    On Tue, Dec 4, 2018 at 1:18 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Dec 03, 2018 at 06:18:30PM +0300, Maksim Milyutin wrote:
> > I have noticed that in description of *flush_lag* in pg_stat_replication
> > view (https://www.postgresql.org/docs/current/monitoring-stats.html#PG-STAT-REPLICATION-VIEW)
> > there exists unknown value of synchronous_commit parameter - *remote_flush*.
> > I think it was meant to use the value *on*.
>
> Yes, you are right.  It should be "on" as "remote_flush" is not a valid
> value.  remote_flush is listed in SyncCommitLevel though, so this makes
> me wonder if we should also introduce a new value for this purpose
> available for users.  The fix you propose looks good to me.  Any
> opinions from others?

+1 for the patch.

As for introducing remote_flush as the true name of the level, this
was discussed but somehow went off-course and never made it to the
finish line:

https://www.postgresql.org/message-id/flat/CAEepm%3D3FFaanSS4sugG%2BApzq2tCVjEYCO2wOQBod2d7GWb%3DDvA%40mail.gmail.com

-- 
Thomas Munro
http://www.enterprisedb.com

  4. Re: Typo in description of replay_lag attribute in pg_stat_replication view

    Michael Paquier <michael@paquier.xyz> — 2018-12-04T02:19:25Z

    On Tue, Dec 04, 2018 at 01:28:15PM +1300, Thomas Munro wrote:
> On Tue, Dec 4, 2018 at 1:18 PM Michael Paquier <michael@paquier.xyz> wrote:
>> Yes, you are right.  It should be "on" as "remote_flush" is not a valid
>> value.  remote_flush is listed in SyncCommitLevel though, so this makes
>> me wonder if we should also introduce a new value for this purpose
>> available for users.  The fix you propose looks good to me.  Any
>> opinions from others?
> 
> +1 for the patch.

Thanks for confirming, Thomas.  I'll go apply hopefully tomorrow if
nobody has objections.

> As for introducing remote_flush as the true name of the level, this
> was discussed but somehow went off-course and never made it to the
> finish line:
> 
> https://www.postgresql.org/message-id/flat/CAEepm%3D3FFaanSS4sugG%2BApzq2tCVjEYCO2wOQBod2d7GWb%3DDvA%40mail.gmail.com

Oh, I forgot this one.  We may want to revive that...  remote_flush is
more meaningful than on, especially since there are more and more
possible values for synchronous_commit.
--
Michael

  5. Re: Typo in description of replay_lag attribute in pg_stat_replication view

    Maksim Milyutin <milyutinma@gmail.com> — 2018-12-04T22:24:22Z

    04.12.2018 5:19, Michael Paquier wrote:

> On Tue, Dec 04, 2018 at 01:28:15PM +1300, Thomas Munro wrote:
>> On Tue, Dec 4, 2018 at 1:18 PM Michael Paquier <michael@paquier.xyz> wrote:
>>> Yes, you are right.  It should be "on" as "remote_flush" is not a valid
>>> value.  remote_flush is listed in SyncCommitLevel though, so this makes
>>> me wonder if we should also introduce a new value for this purpose
>>> available for users.  The fix you propose looks good to me.  Any
>>> opinions from others?
>> +1 for the patch.
> Thanks for confirming, Thomas.  I'll go apply hopefully tomorrow if
> nobody has objections.
>
>> As for introducing remote_flush as the true name of the level, this
>> was discussed but somehow went off-course and never made it to the
>> finish line:
>>
>> https://www.postgresql.org/message-id/flat/CAEepm%3D3FFaanSS4sugG%2BApzq2tCVjEYCO2wOQBod2d7GWb%3DDvA%40mail.gmail.com
> Oh, I forgot this one.  We may want to revive that...  remote_flush is
> more meaningful than on, especially since there are more and more
> possible values for synchronous_commit.


Yeah, I think the notion *remote_flush level* is more appropriate 
especially in the context of sync replication. Within this context maybe 
it makes sense to replace the word *level* to *value* in description of 
*flush_lag*?

-- 
Regards,
Maksim Milyutin

  6. Re: Typo in description of replay_lag attribute in pg_stat_replication view

    Michael Paquier <michael@paquier.xyz> — 2018-12-05T01:04:55Z

    On Wed, Dec 05, 2018 at 01:24:22AM +0300, Maksim Milyutin wrote:
> Yeah, I think the notion *remote_flush level* is more appropriate especially
> in the context of sync replication. Within this context maybe it makes sense
> to replace the word *level* to *value* in description of *flush_lag*?

I am not sure that this is an improvement.  Anyway, I have committed
your original patch as that's clearly a mistake and back-patched down to
v10.
--
Michael

  7. Re: Typo in description of replay_lag attribute in pg_stat_replication view

    Maksim Milyutin <milyutinma@gmail.com> — 2018-12-05T07:52:27Z

    05.12.2018 4:04, Michael Paquier wrote:

> On Wed, Dec 05, 2018 at 01:24:22AM +0300, Maksim Milyutin wrote:
>> Yeah, I think the notion *remote_flush level* is more appropriate especially
>> in the context of sync replication. Within this context maybe it makes sense
>> to replace the word *level* to *value* in description of *flush_lag*?
> I am not sure that this is an improvement.  Anyway, I have committed
> your original patch as that's clearly a mistake and back-patched down to
> v10.


Ok, thanks.

-- 
Regards,
Maksim Milyutin