On 2016-04-25 16:29:36 -0400, Robert Haas wrote: > On Mon, Apr 25, 2016 at 2:05 PM, Andres Freund <and...@anarazel.de> wrote: > > Well, I posted a patch. I'd have applied it too (after addressing your > > comments obviously), except that there's some interdependencies with the > > nsmg > 0 thread (some of my tests fail spuriously without that > > fixed). Early last week I waited for a patch on that thread, but when > > that didn't materialize by Friday I switched to work on that . With > > both fixes applied I can't reproduce any problems anymore. > > OK. What are the interdependencies? You've said that a few times but > I am not clear on what the nature of those interdependencies is.
I added checks to smgr/md.c that verify that the smgr state is consistent with the on-file state. Without the two issues in  fixed, these tests fail in a standby, while running regression tests. Adding those tests made me notice a bug in an unreleased version of the patch, so it seems they're worthwhile to run. > > About the delay: Sure, it'd be nicer if I'd addressed this > > immediately. But priority-wise it's an issue that's damned hard to hit, > > and where the worst known consequence is having to reconnect; that's not > > earth shattering. So spending time to work on the, imo more severe > > performance regressions, seemed to be more important; maybe I was wrong > > in priorizing things that way. > > Do you mean performance regression related to this patch, or to other > patches like the atomic buffer pin/unpin stuff? Other patches, I can't see this having measurable impact. > >> We have a patch, and that's good, and I have reviewed it and > >> Thom has tested it, and that's good, too. But it is not clear whether > >> you feel confident to commit it or when you might be planning to do > >> that, so I asked. Given that this is the open item of longest tenure > >> and that we're hoping to ship beta soon, why is that out of line? > > > > Well, if you'd asked it that way, then I'd not responded the way I have. > > Fair enough, but keep in mind that a few more mildly-phrased emails > from Noah didn't actually get us to the point where this is fixed, or > even to a clear explanation of when it might be fixed or why it wasn't > getting fixed sooner. Hrmpf. I'd initially missed the first email in the onslought, and the second email I responded to and posted a patch in the timeframe I'd promised. I didn't forsee, after we'd initially dismissed a correlation, that there'd be a connection to the other patch. > >> That commit introduced a new way to write to blocks that might have in > >> the meantime been removed, and it failed to make that safe. > > > > There's no writing of any blocks involved - the problem is just about > > opening segments which might or might not exist. > > As I understand it, that commit introduced a backend-private queue; > when it fills, we issue flush requests for particular blocks. By the > time the flush requests are issued, the blocks might have been > truncated away. So it's not writing blocks in the sense of write(), > but persuading the OS to write those blocks to stable storage. Right. > >> And in fact I think it's a regression that can be > >> expected to be a significant operational problem for people if not > >> fixed, because the circumstances in which it can happen are not very > >> obscure. You just need to hold some pending flush requests in your > >> backend-local queue while some other process truncates the relation, > >> and boom. > > > > I found it quite hard to come up with scenarios to reproduce it. Entire > > segments have to be dropped, the writes to the to-be-dropped segment > > have to result in fully dead rows, only few further writes outside the > > dropped can happen, invalidations may not fix the problem up. But > > obviously it should be fixed. > > It's probably a bit tricky to produce a workload where the > truncated-away block is in some backend's private queue, because the > queues are very small and it's not exactly clear what sort of workload > will produce regular relation truncations, and you need both of those > things to hit this. However, I think it's pretty optimistic to think > that there won't be people in that situation, partly because we had a > customer find a bug in an EDB proprietary feature a very similar > whose profile is very similar to this feature less than 2 years ago. I'm obviously not arguing that we shouldn't fix this. > >> I assume you will be pretty darned unhappy if we end up at #2, so I am > >> trying to figure out if we can achieve #1. OK? > > > > Ok. > > I'm still not clear on the answer to this. I think the answer is that > you think that this patch is OK to commit with some editing, but the > situation with the nmsgs > 0 patch is not so clear yet because it > hasn't had any review yet. And they depend on each other somehow. Am > I close? You are. I'd prefer pushing this after fixes for the two invalidation issues, because it makes testing easier. But I guess the timeframe there is unclear enough, that it makes sense to test this patch on top of the the preliminary fixes, and then push without those. - Andres -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers