On Mon, Jan 9, 2023 at 5:59 PM Peter Geoghegan <p...@bowt.ie> wrote: > On Mon, Jan 9, 2023 at 12:51 PM Robert Haas <robertmh...@gmail.com> wrote: > > I feel that you should at least have a reproducer for these problems > > posted to the thread, and ideally a regression test, before committing > > things. I think it's very hard to understand what the problems are > > right now. > > Hard to understand relative to what, exactly? We're talking about a > very subtle race condition here. > > I'll try to come up with a reproducer, but I *utterly* reject your > assertion that it's a hard requirement, sight unseen. Why should those > be the parameters of the discussion? > > For one thing I'm quite confident that I'm right, with or without a > reproducer. And my argument isn't all that hard to follow, if you have > relevant expertise, and actually take the time.
Look, I don't want to spend time arguing about what seem to me to be basic principles of good software engineering. When I don't put test cases into my patches, people complain at me and tell me that I'm a bad software engineer because I didn't include test cases. Your argument here seems to be that you're such a good software engineer that you don't need any test cases to know what the bug is or that you've fixed it correctly. That seems like a surprising argument, but even if it's true, test cases can have considerable value to future code authors, because it allows them to avoid reintroducing bugs that have previously been fixed. In my opinion, it's not worth trying to have automated test cases for absolutely every bug we fix, because many of them would be really hard to develop and executing all of them every time we do anything would be unduly time-consuming. But I can't remember the last time before this that someone wanted to commit a patch for a data corruption issue without even providing a test case that other people can run manually. If you think that is or ought to be standard practice, I can only say that I disagree. I don't particularly appreciate the implication that I either lack relevant or expertise or don't actually take time, either. I spent an hour yesterday looking at your patches yesterday and didn't feel I was very close to understanding 0002 in that time. I feel that if the patches were better-written, with relevant comments and test cases and really good commit messages and a lack of extraneous changes, I believe I probably would have gotten a lot further in the same amount of time. There is certainly an alternate explanation, which is that I am stupid. I'm inclined to think that's not the correct explanation, but most stupid people believe that they aren't, so that doesn't really prove anything. > But even this is > unlikely to matter much. Even if I somehow turn out to have been > completely wrong about the race condition, it is still self-evident > that the problem of uselessly WAL logging non-changes to the VM > exists. That doesn't require any concurrent access at all. It's a > natural consequence of calling visibilitymap_set() with > VISIBILITYMAP_ALL_FROZEN-only flags. You need only look at the code > for 2 minutes to see it. Apparently not, because I spent more time than that. -- Robert Haas EDB: http://www.enterprisedb.com
