On Fri, Mar 1, 2019 at 4:41 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > FWIW, I notice that the logic that appears after the > > _bt_lock_branch_parent() call to _bt_getstackbuf() anticipates that it > > must defend against interrupted splits in at least the > > grandparent-of-leaf page, and maybe even the parent, so it's probably > > not unworkable to not finish the split: > > -ETOOMANYNEGATIVES ... can't quite parse your point here?
Sorry. :-) My point was that it actually seems feasible to not do the split, making the quoted paragraph from nbtree README correct as-is. But, since we're happy to continue to finish the occasional interrupted internal page split within VACUUM anyway, that isn't an important point. > I think that code is there to deal with the possibility of finding > an old half-dead page. Don't know that it's safe to remove it yet. I don't know that it is either. My first instinct is to assume that it's fine to remove the code, since, as I said, we're treating internal pages that are half-dead as being ipso facto corrupt -- we'll throw an error before too long anyway. However, "internal + half dead" was a valid state for an nbtree page prior to 9.4, and we make no distinction about that (versioning nbtree indexes to deal with cross-version incompatibilities came only in Postgres v11). Trying to analyze whether or not it's truly safe to just not do this seems very difficult, and I don't think that it's performance critical. This is a problem only because it's distracting and confusing. I favor keeping the test, but having it throw a ERRCODE_INDEX_CORRUPTED error, just like _bt_pagedel() does already. A comment could point out that the test is historical/defensive, and probably isn't actually necessary. What do you think of that idea? -- Peter Geoghegan
