Horton

Re: doc review for v14

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org, Bruce Momjian <bruce@momjian.us>, Thomas Munro <thomas.munro@gmail.com>
Date: 2021-01-23T10:15:40Z
Lists: pgsql-hackers 
Hi Justin,

On Sun, Dec 27, 2020 at 02:26:05PM -0600, Justin Pryzby wrote:
> Thank you.

I have been looking at 0005, the patch dealing with the docs of the
replication stats, and have some comments.

        <para>
         Number of times transactions were spilled to disk while decoding changes
-        from WAL for this slot. Transactions may get spilled repeatedly, and
-        this counter gets incremented on every such invocation.
+        from WAL for this slot. A given transaction may be spilled multiple times, and
+        this counter is incremented each time.
       </para></entry>
The original can be a bit hard to read, and I don't think that the new
formulation is an improvement.  I actually find confusing that this
mixes in the same sentence that a transaction can be spilled multiple
times and increment this counter each time.  What about splitting that
into two sentences?  Here is an idea:
"This counter is incremented each time a transaction is spilled.  The
same transaction may be spilled multiple times."

-        Number of transactions spilled to disk after the memory used by
-        logical decoding of changes from WAL for this slot exceeds
+        Number of transactions spilled to disk because the memory used by
+        logical decoding of changes from WAL for this slot exceeded
What does "logical decoding of changes from WAL" mean?  Here is an
idea to clarify all that:
"Number of transactions spilled to disk once the memory used by
logical decoding to decode changes from WAL has exceeded
logical_decoding_work_mem."

         Number of in-progress transactions streamed to the decoding output plugin
-        after the memory used by logical decoding of changes from WAL for this
-        slot exceeds <literal>logical_decoding_work_mem</literal>. Streaming only
+        because the memory used by logical decoding of changes from WAL for this
+        slot exceeded <literal>logical_decoding_work_mem</literal>. Streaming only
         works with toplevel transactions (subtransactions can't be streamed
-        independently), so the counter does not get incremented for subtransactions+        independently), so the counter is not incremented for subtransactions.
I have the same issue here with "by logical decoding of changes from
WAL".  I'd say "after the memory used by logical decoding to decode
changes from WAL for this slot has exceeded logical_decoding_work_mem".

         output plugin while decoding changes from WAL for this slot. Transactions
-        may get streamed repeatedly, and this counter gets incremented on every
-        such invocation.
+        may be streamed multiple times, and this counter is incremented each time.
I would split this stuff into two sentences:
"This counter is incremented each time a transaction is streamed.  The
same transaction may be streamed multiple times.

          Resets statistics to zero for a single replication slot, or for all
-         replication slots in the cluster.  The argument can be either the name
-         of the slot to reset the stats or NULL.  If the argument is NULL, all
-         counters shown in the <structname>pg_stat_replication_slots</structname>
-         view for all replication slots are reset.
+         replication slots in the cluster.  The argument can be either NULL or the name
+         of a slot for which stats are to be reset.  If the argument is NULL, all
+         counters in the <structname>pg_stat_replication_slots</structname>
+         view are reset for all replication slots.
Here also, I find rather confusing that this paragraph tells multiple
times that NULL resets the stats for all the replication slots.  NULL
should use a <literal> markup, and it is cleaner to use "statistics"
rather than "stats" IMO.  So I guess we could simplify things as
follows:
"Resets statistics of the replication slot defined by the argument. If
the argument is NULL, resets statistics for all the replication
slots."
--
Michael

Commits

  1. Fix typos and grammar in comments and docs

  2. Fix typos and grammar in documentation and code comments

  3. Move pg_stat_statements query jumbling to core.

  4. doc: Improve description of data checksums

  5. Fix some typos, grammar and style in docs and comments

  6. doc: Improve wording of section for repslot statistics

  7. Implement support for bulk inserts in postgres_fdw

  8. psql \dX: list extended statistics objects

  9. Add documentation chapter about checksums

  10. Simplify some comments in xml.c

  11. Allow decoding at prepare time in ReorderBuffer.

  12. Extend the output plugin API to allow decoding of prepared xacts.

  13. doc: Improve some grammar and sentences

  14. doc: Improve description of min_dynamic_shared_memory

  15. Fix typos and grammar in docs and comments

  16. Multirange datatypes

  17. Move per-agg and per-trans duplicate finding to the planner.

  18. Allow pg_rewind to use a standby server as the source system.

  19. snapshot scalability: cache snapshots using a xact completion counter.

  20. Add tests for tuplesort.c.

  21. Document piecemeal construction of partitioned indexes

  22. Report progress of CREATE INDEX operations

  23. Fix misc typos, mostly in comments.

  24. Improve pageinspect module

  25. Revamp the WAL record format.

  26. Fix assorted bugs in CREATE/DROP INDEX CONCURRENTLY.