Thread: [HACKERS] Fix GetOldestXmin comment
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
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
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