Fix GetOldestXmin comment

Started by Masahiko Sawadaover 8 years ago3 messages
#1Masahiko Sawada
sawada.mshk@gmail.com
1 attachment(s)

Hi,

While reading source code, I realized that comment of GetOldestXmin mentions;

* if rel = NULL and there are no transactions running in the current
* database, GetOldestXmin() returns latestCompletedXid.

However, in that case if I understand correctly GetOldestXmin()
actually returns latestCompletedXid + 1 as follows;

/*
* We initialize the MIN() calculation with latestCompletedXid + 1. This
* is a lower bound for the XIDs that might appear in the ProcArray later,
* and so protects us against overestimating the result due to future
* additions.
*/
result = ShmemVariableCache->latestCompletedXid;
Assert(TransactionIdIsNormal(result));
TransactionIdAdvance(result);

Attached patch fixes the top comment of GetOldestXmin.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

getoldestxmin_comment.patchapplication/octet-stream; name=getoldestxmin_comment.patchDownload
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 8a71536..a645283 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -1292,7 +1292,7 @@ TransactionIdIsActive(TransactionId xid)
  * anything older is definitely not considered as running by anyone anymore,
  * but the exact value calculated depends on a number of things. For example,
  * if rel = NULL and there are no transactions running in the current
- * database, GetOldestXmin() returns latestCompletedXid. If a transaction
+ * database, GetOldestXmin() returns latestCompletedXid + 1. If a transaction
  * begins after that, its xmin will include in-progress transactions in other
  * databases that started earlier, so another call will return a lower value.
  * Nonetheless it is safe to vacuum a table in the current database with the
#2Amit Kapila
amit.kapila16@gmail.com
In reply to: Masahiko Sawada (#1)
Re: Fix GetOldestXmin comment

On Tue, May 30, 2017 at 6:32 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Hi,

While reading source code, I realized that comment of GetOldestXmin mentions;

* if rel = NULL and there are no transactions running in the current
* database, GetOldestXmin() returns latestCompletedXid.

However, in that case if I understand correctly GetOldestXmin()
actually returns latestCompletedXid + 1 as follows;

Isn't there another gotcha in above part of the comment, shouldn't it
say rel != NULL? AFAICS, when rel is NULL, it considers all databases
not only current database.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Amit Kapila (#2)
Re: Fix GetOldestXmin comment

On Tue, May 30, 2017 at 11:07 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, May 30, 2017 at 6:32 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Hi,

While reading source code, I realized that comment of GetOldestXmin mentions;

* if rel = NULL and there are no transactions running in the current
* database, GetOldestXmin() returns latestCompletedXid.

However, in that case if I understand correctly GetOldestXmin()
actually returns latestCompletedXid + 1 as follows;

Isn't there another gotcha in above part of the comment, shouldn't it
say rel != NULL? AFAICS, when rel is NULL, it considers all databases
not only current database.

Hmm it could return latestCompletedXid in that case. I think I
understood now, Thank you!

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