Re: Re: PATCH: Split stats file per database WAS: autovacuum stress-testing our system
От | Tom Lane |
---|---|
Тема | Re: Re: PATCH: Split stats file per database WAS: autovacuum stress-testing our system |
Дата | |
Msg-id | 8540.1464713975@sss.pgh.pa.us обсуждение исходный текст |
Ответ на | Re: Re: PATCH: Split stats file per database WAS: autovacuum stress-testing our system (Tomas Vondra <tomas.vondra@2ndquadrant.com>) |
Ответы |
Re: Re: PATCH: Split stats file per database WAS:
autovacuum stress-testing our system
(Tomas Vondra <tomas.vondra@2ndquadrant.com>)
|
Список | pgsql-hackers |
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes: > On 05/26/2016 10:10 PM, Tom Lane wrote: >> I posted a patch at >> https://www.postgresql.org/message-id/13023.1464213041@sss.pgh.pa.us >> which I think is functionally equivalent to what you have here, but >> it goes to some lengths to make the code more readable, whereas this >> is just adding another layer of complication to something that's >> already a mess (eg, the request_time field is quite useless as-is). >> So I'd like to propose pushing that in place of this patch ... do you >> care to review it first? > I've reviewed the patch today, and it seems fine to me - correct and > achieving the same goal as the patch posted to this thread (plus fixing > the issue with shared catalogs and fixing many comments). Thanks for reviewing! > FWIW do you still plan to back-patch this? Minimizing the amount of > changes was one of the things I had in mind when writing "my" patch, > which is why I ended up with parts that are less readable. Yeah, I think it's a bug fix and should be back-patched. I'm not in favor of making things more complicated just to reduce the number of lines a patch touches. > The one change I'm not quite sure about is the removal of clock skew > detection in pgstat_recv_inquiry(). You've removed the first check on > the inquiry, replacing it with this comment: > It seems sufficient to check for clock skew once per write round. > But the first check was comparing msg/req, while the second check looks > at dbentry/cur_ts. I don't see how those two clock skew check are > redundant - if they are, the comment should explain that I guess. I'm confused here --- are you speaking of having removed if (msg->cutoff_time > req->request_time) req->request_time = msg->cutoff_time; ? That is not a check for clock skew, it's intending to be sure that req->request_time reflects the latest request for this DB when we've seen more than one request. But since req->request_time isn't actually being used anywhere, it's useless code. I reformatted the actual check for clock skew, but I do not think I changed its behavior. > Another thing is that if you believe merging requests across databases > is a silly idea, maybe we should bite the bullet and replace the list of > requests with a single item. I'm not convinced about this, though. No, I don't want to do that either. We're not spending much code by having pending_write_requests be a list rather than a single entry, and we might eventually figure out a reasonable way to time the flushes so that we can merge requests. regards, tom lane
В списке pgsql-hackers по дате отправления: