On 2018-04-23 14:43:48 -0700, Matthew Wilcox wrote: > On Mon, Apr 23, 2018 at 01:57:30PM -0700, Andres Freund wrote: > > I've never really looked at this code in any depth before, but won't > > this potentially lead to the same error being reported on multiple FDs? > > Imagine two fds (potentially in different processes) getting the 0 > > returned by errseq_sample() because it's not ERRSEQ_SEEN. Afaict > > file_check_and_advance_wb_err() will return an error that's always > > unlike 0 in that case, and thus the error will returned on both fds? > > > > I'm personally perfectly fine with that, but it's not necessarily what's > > described as desired in your email?. > > That's what I was trying to say with this paragraph: > > > > This patch restores that behaviour by reporting errors to file descriptors > > > which are opened after the error occurred, but before it was reported > > > to any file descriptor. > > Maybe it was a little unclear? I'd appreciate a suggestion on making > it clearer. I think I was thinking of the following paragraph from your commit message: > Before errseq_t, a writeback error would be reported exactly once (as > long as the inode remained in memory), so Postgres could open a file, > call fsync() and find out whether there had been a writeback error on > that file from another process. Note the "exactly once", making "that behaviour" in the following paragraph potentially refer to exactly once: > This patch restores that behaviour by reporting errors to file descriptors > which are opened after the error occurred, but before it was reported > to any file descriptor. Just adding a sentence here saying something like "This means that the error might be reported to more fds than before." or such would address that potential ambiguity? > I think this behaviour is perfectly justifiable. I agree. Greetings, Andres Freund
