Re: New vacuum option to do only freezing
Masahiko Sawada <sawada.mshk@gmail.com>
From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: Robert Haas <robertmhaas@gmail.com>, "Bossart,
Nathan" <bossartn@amazon.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2019-04-04T02:28:08Z
Lists: pgsql-hackers
Attachments
- v14-0001-Add-INDEX_CLEANUP-option-to-VACUUM-command.patch (application/octet-stream) patch v14-0001
On Thu, Apr 4, 2019 at 9:18 AM Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote: > > Hello. > > At Wed, 3 Apr 2019 11:55:00 -0400, Robert Haas <robertmhaas@gmail.com> wrote in <CA+Tgmoas581jpJ0TPaA38OhjXHgbLy8z1fuuHH7CaNkrboZJeA@mail.gmail.com> > > On Wed, Apr 3, 2019 at 1:32 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > Attached the updated version patches including the > > > DISABLE_PAGE_SKIPPING part (0003). > > > > I am confused about nleft_dead_tuples. It looks like it gets > > incremented whenever we set tupgone = true, regardless of whether we > > are doing index cleanup. But if we ARE doing index cleanup then the > > dead tuple will not be left. And if we are not doing index vacuum > > then we still don't need this for anything, because tups_vacuumed is > > counting the same thing. I may be confused. But if I'm not, then I > > think this should just be ripped out, and we should only keep > > nleft_dead_itemids. > > tups_vacuumed is including heap_page_prune()ed tuples, which > aren't counted as "tupgone". Yes. tup_vacuumed counts not only HOT pruned tuples but also tuples that became dead after heap_page_prune(). When index clenaup is disabled, the former leaves only itemid whereas the latter leaves itemid and heap tuple as we don't remove. nleft_dead_tuples counts only the latter to report precisely. I think nleft_dead_tuples should be incremented only when index cleanup is disabled, and the that part comment should be polished. > > > As far as VacOptTernaryValue, I think it would be safer to change this > > so that VACOPT_TERNARY_DEFAULT = 0. That way palloc0 will fill in the > > value that people are likely to want by default, which makes it less > > likely that people will accidentally write future code that doesn't > > clean up indexes. > > It's convincing. My compalint was enabled=0 and disabled=1 is > confusing so I'm fine with default=0, disabled=1, enabled=2. Okay, fixed. Attached the updated version patch. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Commits
-
Add toast-level reloption for vacuum_index_cleanup
- ce59b75d449d 12.0 landed
-
heap_prepare_freeze_tuple: Simplify coding
- 2bf372a4ae26 12.0 landed
-
Fix missing word.
- 6665305e17ed 12.0 landed
-
Allow VACUUM to be run with index cleanup disabled.
- a96c41feec6b 12.0 landed
-
Don't mark pages all-visible spuriously
- d2599ecfcc74 11.0 cited