Re: Fun fact about autovacuum and orphan temp tables

On Thu, Nov 17, 2016 at 7:37 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> The whole point of the review process is to get an opinion from
> somebody other than the original author on the patch in question. If
> you write a patch and then mark your own patch Ready for Committer,
> you're defeating the entire purpose of this process. I think that's
> what you did here, I think you have done it on other occasions in the
> not-too-distant patch, and I wish you'd stop. I understand perfectly
> well that there are times when a committer needs to involve themselves
> directly in a patch even when nobody else has reviewed it, because
> that just has to happen, and I try to keep an eye out for such
> scenarios, but it's frustrating to clear time to work on the RfC
> backlog and then discover patches that have just been marked that way
> without discussion or agreement.

OK, sorry about that, I switched it back to "Needs Review" FWIW. For
my defense, after a review of the first set of patches proposed, I
suggested one way to go which is the second patch I sent. Bruce and
yourself mentioned that nuking them more aggressively would be better
to have. Tom mentioned that doing so and having a GUC would be worth
it. In short this sounded like an agreement to me, which is why I
proposed a new patch to make the discussion move on. And the patch is
not that complicated. When looking at it I asked myself about
potential timing issues regarding fact of doing this cleanup more
aggressively but discarded them at the end.

> Now, on the substance of this issue, I think your proposed change to
> clean up temporary tables immediately rather than waiting until they
> become old enough to threaten wraparound is a clear improvement, and
> IMHO we should definitely do it. However, I think your proposed
> documentation changes are pretty well inadequate. If somebody
> actually does want to get at the temporary tables of a crashed
> backend, they will need to do a lot more than what you mention in that
> update. Even if they set autovacuum=off, they will be vulnerable to
> having those tables removed by another backend with the same backend
> ID that reinitializes the temporary schema. And even if that doesn't
> happen, they won't be able to select from those tables because they'll
> get an error about reading temporary tables of another session. To
> fix that they'd have to move the temporary tables into some other
> schema AND change the filenames on disk. And then on top of that, by
> the time anybody even found this in the documentation, it would be far
> too late to shut off autovacuum in the first place because it runs
> every minute (unless you have raised autovacuum_naptime, which you
> shouldn't). I think we just shouldn't document this. The proposed
> behavior change is basically switching from an extremely conservative
> behavior with surprising consequences to what people would expect
> anyway, and anyone who needs to dig information out of another
> session's temporary tables is going to need to know a lot more about
> the server internals than we normally explain in the documentation.

Okay, let's remove the documentation then. What do you think about the
updated version attached?
(Even this patch enters into "Needs Review" state).
--
Michael