Re: Teaching users how they can get the most out of HOT in Postgres 14

On Thu, Jun 17, 2021 at 10:54 AM Peter Geoghegan  wrote:
>
> On Sun, May 30, 2021 at 6:30 PM Masahiko Sawada  wrote:
> > We need to accept "yes" and "no" too? Currently, the parsing of a
> > boolean type reloption accepts those words.
>
> Added those in the attached revision, version 2. This is much closer
> to being commitable than v1 was. I plan on committing this in the next
> several days.
>
> I probably need to polish the documentation some more, though.
>
> > It seems to me that it's better to have INDEX_CLEANUP option of VACUUM
> > command support AUTO for consistency. Do you have any concerns about
> > supporting it?
>
> v2 sorts out the mess with VacOptTernaryValue by just adding a new
> enum constant to VacOptTernaryValue, called VACOPT_TERNARY_AUTO -- the
> enum still has a distinct VACOPT_TERNARY_DEFAULT value. v2 also adds a
> new reloption-specific enum, StdRdOptIndexCleanup, which is the
> datatype that we actually use inside the StdRdOptions struct. So we
> are now able to specify "VACUUM (INDEX_CLEANUP AUTO)" in v2 of the
> patch.
>
> v2 also adds a new option to vacuumdb, --force-index-cleanup. This
> seemed to make sense because we already have a --no-index-cleanup
> option.
>
> > > And does StdRdOptions.vacuum_truncate now need
> > > to become a VacOptTernaryValue field too, for consistency with the new
> > > definition of StdRdOptions.vacuum_index_cleanup?
> >
> > We don't have the bypass optimization for heap truncation, unlike
> > index vacuuming. So I think we can leave both vacuum_truncate
> > reloption and TRUNCATE option as boolean parameters.
>
> Actually FWIW we do have a bypass optimization for TRUNCATE -- it too
> has an always-on dynamic behavior -- so it really is like the index
> vacuuming thing. In theory it might make sense to have the same "auto,
> on, off" thing, just like with index vacuuming in the patch. However,
> I haven't done that in the patch because in practice it's a bad idea.
> If we offered users the option of truly forcing truncation, then
> lazy_truncate_heap() could just insist on truncating. It would have to
> just wait for an AEL, no matter how long it took. That would probably
> be dangerous because waiting for an AEL without backing out in VACUUM
> just isn't a great idea.

I agree that it doesn't make sense to force heap truncation.

Thank you for updating the patch! Here are comments on v2 patch:

 typedef enum VacOptTernaryValue
 {
    VACOPT_TERNARY_DEFAULT = 0,
+   VACOPT_TERNARY_AUTO,
    VACOPT_TERNARY_DISABLED,
    VACOPT_TERNARY_ENABLED,
 } VacOptTernaryValue;

VacOptTernaryValue is no longer a ternary value. Can we rename it
something like VacOptValue?

---
+           if (vacopts->force_index_cleanup)
            {
-               /* INDEX_CLEANUP is supported since v12 */
+               /*
+                * "INDEX_CLEANUP TRUE" has been supported since v12. Though
+                * the --force-index-cleanup vacuumdb option was only added in
+                * v14, it still works in the same way on v12+.
+                */
                Assert(serverVersion >= 120000);
+               Assert(!vacopts->no_index_cleanup);
                appendPQExpBuffer(sql, "%sINDEX_CLEANUP FALSE", sep);
                sep = comma;
            }

We should specify TRUE instead.

---
--force-index-cleanup option isn't shown in the help message.

---
I think we also improve the tab completion for INDEX_CLEANUP option.

---
@@ -32,7 +32,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [
boolean ]
     DISABLE_PAGE_SKIPPING [ boolean ]
     SKIP_LOCKED [ boolean ]
-    INDEX_CLEANUP [ boolean ]
+    INDEX_CLEANUP [ enum ]
     PROCESS_TOAST [ boolean ]
     TRUNCATE [ boolean ]
     PARALLEL integer

How about listing the available values of INDEX_CLEANUP here instead
of enum? For example, we do a similar thing in the description of
FORMAT option of EXPLAIN command. It would be easier to perceive all
available values.

---
+     
+      
+      
+       

It should be --force-index-cleanup.

Regards,

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