Re: BUG #14941: Vacuum crashes

From: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Lyes Ameddah <lyes(dot)amd(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #14941: Vacuum crashes
Date: 2017-12-28 22:46:18
Message-ID: 5B80D67E-A664-495A-A0BE-CEA847676164@amazon.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-bugs pgsql-hackers

On 12/21/17, 11:07 PM, "Masahiko Sawada" <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On the latest patch, it looks good to me except for a following comment.
>
> - * If the RangeVar is not defined, we do not have
> enough information
> - * to provide a meaningful log statement. Chances are that
> - * vacuum_rel's caller has intentionally not provided
> this information
> - * so that this logging is skipped, anyway.
> + * If relname is NULL, we do not have enough
> information to provide a
> + * meaningful log statement. Chances are that this
> information was
> + * intentionally not provided so that this logging is
> skipped, anyway.
>
> This comment looks odd to me because we ourselves didn't set relname
> just before. For example, we can move the sentence to above comment
> block as follows. Thoughts?
>
> /*
> * If relation is NULL, we do not have enough information to provide a
> * meaningful log statement. Chances are that this information was
> * intentionally not provided so that this logging is skipped, anyway.
> * However, iff VACOPT_NOWAIT is specified, we always try to emit
> * a log statement even if relation is NULL.
> */
>
> (snip)
>
> /*
> * Determine the log level.
> *
> * For autovacuum logs, we emit a LOG if
> * log_autovacuum_min_duration is not diabled. For manual VACUUM, we
> * emit a WARNING to match the log statements in the permission
> * checks.
> */

I agree, this makes more sense. I've made this change in v3 of 0003.

Nathan

Attachment Content-Type Size
0003_add_nowait_vacuum_option_v3.patch application/octet-stream 11.9 KB
0001_fix_unparenthesized_vacuum_grammar_v2.patch application/octet-stream 1.9 KB
0002_add_parenthesized_analyzed_syntax_v2.patch application/octet-stream 3.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2017-12-28 23:16:20 Re: Condition variable live lock
Previous Message Tom Lane 2017-12-28 22:43:58 Re: plpgsql function startup-time improvements

Browse pgsql-bugs by date

  From Date Subject
Next Message XM 2017-12-29 03:16:01 help
Previous Message Logan Owen 2017-12-28 20:53:10 Re: Server postgres_fdw "fetch_size" option cannot be updated