Re: 64-bit XIDs in deleted nbtree pages

On Fri, Feb 26, 2021 at 9:58 AM Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Thu, Feb 25, 2021 at 5:42 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > btvacuumcleanup()  has been playing two roles: recycling deleted pages
> > and collecting index statistics.
>
> Right.
>
> I pushed the VACUUM VERBOSE "index pages newly deleted"
> instrumentation patch earlier - it really isn't complicated or
> controversial, so I saw no reason to delay with that.

Thanks!

I think we can improve bloom indexes in a separate patch so that they
use pages_newly_deleted.

>
> Attached is v7, which now only has the final patch -- the optimization
> that makes it possible for VACUUM to recycle pages that were newly
> deleted during the same VACUUM operation.  Still no real changes.
> Again, I just wanted to keep CFBot happy. I haven't thought about or
> improved this final patch recently, and it clearly needs more work to
> be ready to commit.

I've looked at the patch. The patch is straightforward and I agreed
with the direction.

Here are some comments on v7 patch.

---
+   /* Allocate _bt_newly_deleted_pages_recycle related information */
+   vstate.ndeletedspace = 512;

Maybe add a #define for the value 512?

----
+   for (int i = 0; i < vstate->ndeleted; i++)
+   {
+       BlockNumber blkno = vstate->deleted[i].blkno;
+       FullTransactionId safexid = vstate->deleted[i].safexid;
+
+       if (!GlobalVisCheckRemovableFullXid(heapRel, safexid))
+           break;
+
+       RecordFreeIndexPage(rel, blkno);
+       stats->pages_free++;
+   }

Should we use 'continue' instead of 'break'? Or can we sort
vstate->deleted array by full XID and leave 'break'?

---
Currently, the patch checks only newly-deleted-pages if they are
recyclable at the end of btvacuumscan. What do you think about the
idea of checking also pages that are deleted by previous vacuums
(i.g., pages already marked P_ISDELETED() but not
BTPageIsRecyclable())? There is still a little hope that such pages
become recyclable when we reached the end of btvacuumscan. We will end
up checking such pages twice (during btvacuumscan() and the end of
btvacuumscan()) but if the cost of collecting and checking pages is
not high it probably could expand the chance of recycling pages.

I'm going to reply to the discussion vacuum_cleanup_index_scale_factor
in a separate mail. Or maybe it's better to start a new thread for
that so as get opinions from other hackers. It's no longer related to
the subject.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/