Typo in description of replay_lag attribute in pg_stat_replication view
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
Attachments:
flush_lag_fix.patchtext/x-patch; name=flush_lag_fix.patchDownload+1-1
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
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:
/messages/by-id/CAEepm=3FFaanSS4sugG+Apzq2tCVjEYCO2wOQBod2d7GWb=DvA@mail.gmail.com
--
Thomas Munro
http://www.enterprisedb.com
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:/messages/by-id/CAEepm=3FFaanSS4sugG+Apzq2tCVjEYCO2wOQBod2d7GWb=DvA@mail.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
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:/messages/by-id/CAEepm=3FFaanSS4sugG+Apzq2tCVjEYCO2wOQBod2d7GWb=DvA@mail.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
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
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