On Wed, Nov 16, 2016 at 11:14 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > Hm. Thinking about that again, having a GUC to control if orphaned > temp tables in autovacuum is an overkill (who is going to go into this > level of tuning!?) and that we had better do something more aggressive > as there have been cases of users complaining about dangling temp > tables. I suspect the use case where people would like to have a look > at orphaned temp tables after a backend crash is not that wide, at > least a method would be to disable autovacuum after a crash if such a > monitoring is necessary. Tom has already stated upthread that the > patch to remove wildly locks is not acceptable, and he's clearly > right. > > So the best move would be really to make the removal of orphaned temp > tables more aggressive, and not bother about having a GUC to control > that. The patch sent in > https://www.postgresql.org/message-id/cab7npqsrywaz1i12mpkh06_roo3ifgcgr88_aex1oeg2r4o...@mail.gmail.com > does so, so I am marking the CF entry as ready for committer for this > patch to attract some committer's attention on the matter.
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. 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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
