Minor clarifying changes to abbreviated key abort code comments
Attached are a couple of patches that only change code comments. The
first (abort abbreviation) patch is recommended for backpatch to 9.5.
The second is a tiny tweak.
--
Peter Geoghegan
Attachments:
0002-Correct-tiny-inaccuracy-in-strxfrm-cache-comment.patchtext/x-patch; charset=US-ASCII; name=0002-Correct-tiny-inaccuracy-in-strxfrm-cache-comment.patchDownload+1-2
0001-Clarify-point-on-aborting-abbreviation.patchtext/x-patch; charset=US-ASCII; name=0001-Clarify-point-on-aborting-abbreviation.patchDownload+20-11
On Sat, Oct 31, 2015 at 3:42 PM, Peter Geoghegan <pg@heroku.com> wrote:
Attached are a couple of patches that only change code comments. The
first (abort abbreviation) patch is recommended for backpatch to 9.5.
The second is a tiny tweak.
This comment doesn't make sense to me:
+ * (TSS_BUILDRUNS state prevents control reaching here in any
+ * case).Unless I'm missing something, that's not actually true.
I've committed 0002.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Nov 3, 2015 at 5:47 AM, Robert Haas <robertmhaas@gmail.com> wrote:
This comment doesn't make sense to me:
+ * (TSS_BUILDRUNS state prevents control reaching here in any + * case).Unless I'm missing something, that's not actually true.
It is true. consider_abort_common() only actually considers aborting
when state is TSS_INITIAL (we're still doing an internal sort). The
only other pertinent state here is TSS_BUILDRUNS. The point is that
TSS_BUILDRUNS is a generic "point of no return" past which
abbreviation cannot be aborted. That is a little arbitrary.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Nov 3, 2015 at 12:36 PM, Peter Geoghegan <pg@heroku.com> wrote:
On Tue, Nov 3, 2015 at 5:47 AM, Robert Haas <robertmhaas@gmail.com> wrote:
This comment doesn't make sense to me:
+ * (TSS_BUILDRUNS state prevents control reaching here in any + * case).Unless I'm missing something, that's not actually true.
It is true. consider_abort_common() only actually considers aborting
when state is TSS_INITIAL (we're still doing an internal sort). The
only other pertinent state here is TSS_BUILDRUNS. The point is that
TSS_BUILDRUNS is a generic "point of no return" past which
abbreviation cannot be aborted. That is a little arbitrary.
OK, I see. Fixing comments in the back-branches is not always a
productive use of time, and in general I might like it if you pushed
for such things less frequently. But I've done it anyway in this
instance.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Nov 3, 2015 at 11:15 AM, Robert Haas <robertmhaas@gmail.com> wrote:
OK, I see. Fixing comments in the back-branches is not always a
productive use of time, and in general I might like it if you pushed
for such things less frequently. But I've done it anyway in this
instance.
I guess I favor doing it where the comment is actually wrong, which
does apply here -- we don't *rely* on anything. We could very well
abort when state is TSS_BUILDRUNS, but we elect not too, since
TSS_BUILDRUNS is taken to mean that it's too late for aborting to be
worth it.
Thanks
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Nov 3, 2015 at 2:19 PM, Peter Geoghegan <pg@heroku.com> wrote:
On Tue, Nov 3, 2015 at 11:15 AM, Robert Haas <robertmhaas@gmail.com> wrote:
OK, I see. Fixing comments in the back-branches is not always a
productive use of time, and in general I might like it if you pushed
for such things less frequently. But I've done it anyway in this
instance.I guess I favor doing it where the comment is actually wrong, which
does apply here -- we don't *rely* on anything. We could very well
abort when state is TSS_BUILDRUNS, but we elect not too, since
TSS_BUILDRUNS is taken to mean that it's too late for aborting to be
worth it.
Fair point.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers