Re: New vacuum option to do only freezing

On Sat, Feb 2, 2019 at 7:00 PM Alvaro Herrera <alvherre(at)2ndquadrant(dot)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(at)amazon(dot)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