From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
Cc: | Noah Misch <noah(at)leadboat(dot)com>, pgsql-hackers(at)postgresql(dot)org, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Subject: | Re: Re: PATCH: Split stats file per database WAS: autovacuum stress-testing our system |
Date: | 2016-05-31 16:59:35 |
Message-ID: | 8540.1464713975@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2016-05-31 17:03:21 | Re: Rename max_parallel_degree? |
Previous Message | Tom Lane | 2016-05-31 16:47:58 | Re: Parallel safety tagging of extension functions |