Re: Use of RangeVar for partitioned tables in autovacuum

Thanks Michael for working on this.

On 2017/09/27 11:28, Michael Paquier wrote:
> Hi all,
>
> I have been looking more closely at the problem in $subject, that I
> have mentioned a couple of times, like here:
> https://www.postgresql.org/message-id/CAB7nPqQa37OUne_rJzpmWC4EXQaLX9f27-Ma_-rSFL_3mj+HFQ@mail.gmail.com
>
> As of HEAD, the RangeVar defined in calls of vacuum() is used for
> error reporting, only in two cases: for autoanalyze and autovacuum
> when reporting to users that a lock cannot be taken on a relation. The
> thing is that autovacuum has the good idea to call vacuum() on only
> one relation at the same time, with always a relation OID and a
> RangeVar defined, so the code currently on HEAD is basically fine with
> its error reporting, because get_rel_oids() will always work on the
> relation OID with its RangeVar, and because code paths with manual
> VACUUMs don't use the RangeVar for any reports.

Yeah, autovacuum_do_vac_analyze will never pass partitioned table OIDs to
vacuum, for example, because they're not RELKIND_RELATION relations.

> While v10 is safe, I think that this is wrong in the long-term and
> that may be a cause of bugs if people begin to generate reports for
> manual VACUUMs, which is plausible in my opinion. The patch attached,
> is what one solution would look like if we would like to make things
> more robust as long as manual VACUUM commands can only specify one
> relation at the same time. I have found that tracking the parent OID
> by tweaking a bit get_rel_oids() was the most elegant solution.

The patch basically looks good to me, FWIW.

> Please
> note that this range of problems is something that I think is better
> solved with the infrastructure that support for VACUUM with multiple
> relations includes (last version here
> https://www.postgresql.org/message-id/766556DD-AA3C-42F7-ACF4-5DC97F41F151@amazon.com)
> So I don't think that the patch attached should be integrated, still I
> am attaching it to satisfy the curiosity of anybody looking at this
> message.

Yeah, after looking at the code a bit, it seems that the problem won't
really occur for the case we're trying to apply this patch for.

> In conclusion, I think that the open item of $subject should be
> removed from the list, and we should try to get the multi-VACUUM patch
> in to cover any future problems. I'll do so if there are no
> objections.

+1 to this, taking the previous +1 back [1]. :)

> One comment on top of vacuum() is wrong by the way: in the case of a
> manual VACUUM or ANALYZE, a NULL RangeVar is used if no relation is
> specified in the command.

Yep, but it doesn't ever get accessed per our observations.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/8d810dd9-5f64-a5f3-c016-a81f05528fa8%40lab.ntt.co.jp