Comment in snapbuild.c file
Hi all,
In snapbuild.c file, there is a comment as follows.
* NB: Because of that xmax can be lower than xmin, because we only
* increase xmax when a catalog modifying transaction commits. While odd
* looking, it's correct and actually more efficient this way since we hit
* fast paths in tqual.c.
*/
Maybe we can get rid of the second "because" in the first sentence?
Attached patch.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
fix_comment_in_snapbuild_c.patchtext/x-patch; charset=US-ASCII; name=fix_comment_in_snapbuild_c.patchDownload
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 153d171..55026ed 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1116,10 +1116,10 @@ SnapBuildProcessRunningXacts(SnapBuild *builder, XLogRecPtr lsn, xl_running_xact
* so, because we only need to do it for catalog transactions since we
* only ever look at those.
*
- * NB: Because of that xmax can be lower than xmin, because we only
- * increase xmax when a catalog modifying transaction commits. While odd
- * looking, it's correct and actually more efficient this way since we hit
- * fast paths in tqual.c.
+ * NB: Because of that xmax can be lower than xmin, we only increase xmax
+ * when a catalog modifying transaction commits. While odd looking, it's
+ * correct and actually more efficient this way since we hit fast paths
+ * in tqual.c.
*/
builder->xmin = running->oldestRunningXid;
Masahiko Sawada wrote:
Hi all,
In snapbuild.c file, there is a comment as follows.
* NB: Because of that xmax can be lower than xmin, because we only
* increase xmax when a catalog modifying transaction commits. While odd
* looking, it's correct and actually more efficient this way since we hit
* fast paths in tqual.c.
*/Maybe we can get rid of the second "because" in the first sentence?
I think the whole para needs to be rethought. I propose this:
* NB: We only increase xmax when a catalog-modifying transaction commits
* (see SnapBuildCommitTxn). Because of this, xmax can be lower than xmin,
* which looks odd but is correct and actually more efficient, since we hit
* fast paths in tqual.c.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Alvaro Herrera wrote:
Masahiko Sawada wrote:
Hi all,
In snapbuild.c file, there is a comment as follows.
* NB: Because of that xmax can be lower than xmin, because we only
* increase xmax when a catalog modifying transaction commits. While odd
* looking, it's correct and actually more efficient this way since we hit
* fast paths in tqual.c.
*/I think the whole para needs to be rethought.
Pushed, thanks.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Aug 13, 2017 at 12:28 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Alvaro Herrera wrote:
Masahiko Sawada wrote:
Hi all,
In snapbuild.c file, there is a comment as follows.
* NB: Because of that xmax can be lower than xmin, because we only
* increase xmax when a catalog modifying transaction commits. While odd
* looking, it's correct and actually more efficient this way since we hit
* fast paths in tqual.c.
*/I think the whole para needs to be rethought.
Pushed, thanks.
Sorry for the late response. I agreed with your proposal. Thank you
for committing!
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers