Re: New vacuum option to do only freezing

On Sat, Feb 2, 2019 at 7:00 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> On 2019-Feb-01, Bossart, Nathan wrote:
>
> > IMHO we could document this feature at a slightly higher level without
> > leaving out any really important user-facing behavior.  Here's a quick
> > attempt to show what I am thinking:
> >
> >         With this option, VACUUM skips all index cleanup behavior and
> >         only marks tuples as "dead" without reclaiming the storage.
> >         While this can help reclaim transaction IDs faster to avoid
> >         transaction ID wraparound (see Section 24.1.5), it will not
> >         reduce bloat.
>
> Hmm ... don't we compact out the storage for dead tuples?  If we do (and
> I think we should) then this wording is not entirely correct.

Yeah, we remove tuple and leave the dead ItemId. So we actually
reclaim the almost tuple storage.

>
> >         Note that this option is ignored for tables
> >         that have no indexes.  Also, this option cannot be used in
> >         conjunction with the FULL option, since FULL requires
> >         rewriting the table.
>
> I would remove the "Also," in there, since it seems to me to give the
> wrong impression about those two things being similar, but they're not:
> one is convenient behavior, the other is a limitation.

Agreed.

>
> > +     /* Notify user that DISABLE_INDEX_CLEANUP option is ignored */
> > +     if (!vacrelstats->hasindex && (options & VACOPT_DISABLE_INDEX_CLEANUP))
> > +             ereport(NOTICE,
> > +                             (errmsg("DISABLE_INDEX_CLEANUP is ignored because table \"%s\" does not have index",
> > +                                             RelationGetRelationName(onerel))));
> >
> > It might make more sense to emit this in lazy_scan_heap() where we
> > determine the value of skip_index_vacuum.
>
> Why do we need a NOTICE here?  I think this case should be silent.
>

Okay, removed it.

On Fri, Feb 1, 2019 at 11:43 PM Bossart, Nathan <bossartn@amazon.com> wrote:
>
> +               if (skip_index_vacuum)
> +                       ereport(elevel,
> +                                       (errmsg("\"%s\": marked %.0f row versions as dead in %u pages",
> +                                                       RelationGetRelationName(onerel),
> +                                                       tups_vacuumed, vacuumed_pages)));
>
> IIUC tups_vacuumed will include tuples removed during HOT pruning, so
> it could be inaccurate to say all of these tuples have only been
> marked "dead."  Perhaps we could keep a separate count of tuples
> removed via HOT pruning in case we are using DISABLE_INDEX_CLEANUP.
> There might be similar problems with the values stored in vacrelstats
> that are used at the end of heap_vacuum_rel() (e.g. tuples_deleted).

Yeah, tups_vacuumed include such tuples so the message is inaccurate.
So I think that we should not change the message but we can report the
dead item pointers we left in errdetail. That's more accurate and
would help user.

postgres(1:1130)=# vacuum (verbose, disable_index_cleanup) test;
INFO:  vacuuming "public.test"
INFO:  "test": removed 49 row versions in 1 pages
INFO:  "test": found 49 removable, 51 nonremovable row versions in 1
out of 1 pages
DETAIL:  0 dead row versions cannot be removed yet, oldest xmin: 509
There were 0 unused item pointers.
Skipped 0 pages due to buffer pins, 0 frozen pages.
0 pages are entirely empty.
49 tuples are left as dead.
CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s.
VACUUM

Attached the updated patch and the patch for vacuumdb.

Regards,

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