Re: New docs chapter on Transaction Management and related changes - Mailing list pgsql-hackers

On Wed, 2022-11-09 at 09:16 -0500, Robert Treat wrote:
> On Mon, Nov 7, 2022 at 5:04 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
> > Some comments:
> > 
> <snip>
> > > --- a/doc/src/sgml/ref/release_savepoint.sgml
> > > +++ b/doc/src/sgml/ref/release_savepoint.sgml
> > > @@ -34,23 +34,16 @@ RELEASE [ SAVEPOINT ] <replaceable>savepoint_name</replaceable>
> > >    <title>Description</title>
> > > 
> > >    <para>
> > > -   <command>RELEASE SAVEPOINT</command> destroys a savepoint previously defined
> > > -   in the current transaction.
> > > +   <command>RELEASE SAVEPOINT</command> will subcommit the subtransaction
> > > +   established by the named savepoint, if one exists. This will release
> > > +   any resources held by the subtransaction. If there were any
> > > +   subtransactions of the named savepoint, these will also be subcommitted.
> > >    </para>
> > > 
> > >    <para>
> > 
> > "Subtransactions of the named savepoint" is somewhat confusing; how about
> > "subtransactions of the subtransaction established by the named savepoint"?
> > 
> > If that is too long and explicit, perhaps "subtransactions of that subtransaction".
> 
> Personally, I think these are more confusing.

Perhaps.  I was worried because everywhere else, the wording makes a clear distinction
between a savepoint and the subtransaction created by a savepoint.  But perhaps some
sloppiness is better to avoid such word cascades.

> > > --- a/doc/src/sgml/ref/rollback.sgml
> > > +++ b/doc/src/sgml/ref/rollback.sgml
> > > @@ -56,11 +56,14 @@ ROLLBACK [ WORK | TRANSACTION ] [ AND [ NO ] CHAIN ]
> > >      <term><literal>AND CHAIN</literal></term>
> > >      <listitem>
> > >       <para>
> > > -      If <literal>AND CHAIN</literal> is specified, a new transaction is
> > > +      If <literal>AND CHAIN</literal> is specified, a new unaborted transaction is
> > >        immediately started with the same transaction characteristics (see <xref
> > >        linkend="sql-set-transaction"/>) as the just finished one.  Otherwise,
> > >        no new transaction is started.
> > 
> > I don't think that is an improvement.  "Unaborted" is an un-word.  A new transaction
> > is always "unaborted", isn't it?
> 
> I thought about this as well when reviewing it, but I do think
> something is needed for the case where you have a transaction which
> has suffered an error and then you issue "rollback and chain"; if you
> just say "a new transaction is immediately started with the same
> transaction characteristics" it might imply to some the new
> transaction has some kind of carry over of the previous broken
> transaction... the use of the word unaborted makes it clear that the
> new transaction is 100% functional.

A new transaction is never aborted in my understanding.  Being aborted is not a
characteristic of a transaction, but a state.

> > 
> 
> 
> > > +    The internal transaction ID type <type>xid</type> is 32-bits wide
> > 
> > There should be no hyphen in "32 bits wide", just as in "3 years old".
> 
> Minor aside, we should clean up glossary.sgml as well.

Right, it has this:

     The numerical, unique, sequentially-assigned identifier that each
     transaction receives when it first causes a database modification.
     Frequently abbreviated as <firstterm>xid</firstterm>.
     When stored on disk, xids are only 32-bits wide, so only
     approximately four billion write transaction IDs can be generated;
     to permit the system to run for longer than that,
     <firstterm>epochs</firstterm> are used, also 32 bits wide.

Which reminds me that I should have suggested <firstterm> rather than
<quote> where I complained about the use of <literal>.

> > 
> > +                                                                   Up to
> > +    64 open subxids are cached in shared memory for each backend; after
> > +    that point, the overhead increases significantly since we must look
> > +    up subxid entries in <filename>pg_subtrans</filename>.
> > 
> > Comma before "since".  Perhaps you should mention that this means disk I/O.
> 
> ISTR that you only use a comma before since in cases where the
> preceding thought contains a negative.

Not being a native speaker, I'll leave that to those who are; I went by feeling.

> In any case, are you thinking something like this:
> 
> " 64 open subxids are cached in shared memory for each backend; after
>  that point the overhead increases significantly due to additional disk I/O
>  from looking up subxid entries in <filename>pg_subtrans</filename>."

Yes.

Yours,
Laurenz Albe