Re: New vacuum option to do only freezing

On Tue, Mar 5, 2019 at 8:01 PM Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
>
> Hello, I have some other comments.
>

Thank you for the comment!

On Tue, Mar 5, 2019 at 8:01 PM Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
>
>
> +                   nleft;            /* item pointers we left */
>
> The name seems to be something other, and the comment doesn't
> makes sense at least.. for me.. Looking below,
>
> +                                    "%.0f tuples are left as dead.\n",
> +                                    nleft),
> +                     nleft);
>
> How about "nleft_dead; /* iterm pointers left as dead */"?

Fixed.

>
>
>
> In this block:
>
> -        if (nindexes == 0 &&
> +        if ((nindexes == 0 || skip_index_vacuum) &&
>              vacrelstats->num_dead_tuples > 0)
>          {
>
> Is it right that vacuumed_pages is incremented and FSM is updated
> while the page is not actually vacuumed?

Good catch. I think the FSM stuff is right because we actually did HOT
pruning but the increment of vacuumed_page seems wrong to me. I think
we should not increment it and not report "removed XX row version in
YY pages" message.

>
>
>          tups_vacuumed += heap_page_prune(onerel, buf, OldestXmin, false,
> -                                         &vacrelstats->latestRemovedXid);
> +                                         &vacrelstats->latestRemovedXid,
> +                                         &tups_pruned);
>
> tups_pruned looks as "HOT pruned tuples". It is named "unused" in
> the function's parameters. (But I think it is useless. Please see
> the details below.)
>
>
> I tested it with a simple data set.
>
> (autovacuum = off)
> drop table if exists t;
> create table t with (fillfactor=50) as select a, a % 3 as b from generate_series(1, 9) a;
> create index on t(a);
> update t set a = -a where b = 0;
> update t set b = b + 1 where b = 1;
>
> We now have 9 tuples, 15 versions and 3 out of 6 "old" tuples are
> to be "left dead" by DISABLE_INDEX_CLEANUP vacuum. It means,
> three tuples ends with "left dead", three tuples are removed and
> 12 tuples will survive the vacuum below.
>
> vacuum (verbose, freeze ,disable_index_cleanup, disable_page_skipping) t;
>
> > INFO:  "t": removed 0 row versions in 1 pages
> > INFO:  "t": found 0 removable, 9 nonremovable row versions in 1 out of 1 pages
> > DETAIL:  0 dead row versions cannot be removed yet, oldest xmin: 925
> > There were 0 unused item pointers.
> > Skipped 0 pages due to buffer pins, 0 frozen pages.
> > 0 pages are entirely empty.
> > 3 tuples are left as dead.
>
> Three tuple versions have vanished. Actually they were removed
> but not shown in the message.
>
> heap_prune_chain() doesn't count a live root entry of a chain as
> "unused (line pointer)" since it is marked as "redierected". As
> the result the vanished tuples are counted in tups_vacuumed, not
> in tups_pruned. Maybe the name tups_vacuumed is confusing.  After
> removing tups_pruned code it works correctly.
>
> > INFO:  "t": removed 6 row versions in 1 pages
> > INFO:  "t": found 6 removable, 9 nonremovable row versions in 1 out of 1 pages
> > DETAIL:  0 dead row versions cannot be removed yet, oldest xmin: 932
> > There were 0 unused item pointers.
> > Skipped 0 pages due to buffer pins, 0 frozen pages.
> > 0 pages are entirely empty.
> > 3 tuples are left as dead.
>
> I see two choices of the second line above.
>
> 1> "t": found 6 removable, 9 nonremovable row versions in 1 out of 1 pages
>
>   "removable" includes "left dead" tuples.
>
> 2> "t": found 3 removable, 12 nonremovable row versions in 1 out of 1 pages
>
>   "removable" excludes "left dead" tuples.
>
> If you prefer the latter, removable and nonremoveable need to be
> corrected using nleft.

I think that the first vacuum should report the former message because
it's true that the table has 6 removable tuples. We remove 6 tuples
but leave 3 item pointers. So in the second vacuum, it should be
"found 0 removable, 9 nonremovable row versions ..." and "3 tuples are
left as dead". But to report more precisely it'd be better to report
"0 tuples and 3 item identifiers are left as dead" here.

Attached updated patch incorporated all of comments. Also I've added
new reloption vacuum_index_cleanup as per discussion on the "reloption
to prevent VACUUM from truncating empty pages at the end of relation"
thread. Autovacuums also can skip index cleanup when the reloption is
set to false. Since the setting this to false might lead some problems
I've made autovacuums report the number of dead tuples and dead
itemids we left.

Regards,

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