On Mon, May 28, 2018 at 4:52 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: > Hi, > > Review comments on commit 857f9c36: > 1. > @@ -2049,6 +2055,10 @@ _bt_newroot(Relation rel, Buffer lbuf, Buffer rbuf) > metapg = BufferGetPage(metabuf); > metad = BTPageGetMeta(metapg); > > + /* upgrade metapage if needed */ > + if (metad->btm_version < BTREE_VERSION) > + _bt_upgrademetapage(metapg); > + > > The metapage upgrade should be performed under critical section. > > 2. > @@ -191,6 +304,10 @@ _bt_getroot(Relation rel, int access) > LockBuffer(metabuf, BUFFER_LOCK_UNLOCK); > LockBuffer(metabuf, BT_WRITE); > > + /* upgrade metapage if needed */ > + if (metad->btm_version < BTREE_VERSION) > + _bt_upgrademetapage(metapg); > + > > Same as above. > > In other cases like _bt_insertonpg, the upgrade is done inside the > critical section. It seems the above cases are missed. > > 3. > + TransactionId btm_oldest_btpo_xact; /* oldest btpo_xact among of > + * deleted pages */ > > /among of/among all > > Attached patch to fix the above comments. >
Thank you for reviewing and the patch. All changes looks good to me. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
