4 ms·

I'm a bit confused by this perception of the thread. There was one person doubtful the patch is the right approach (Tom), but still offered concrete review comm

I'm a bit confused by this perception of the thread. There was one person doubtful the patch is the right approach (Tom), but still offered concrete review comments (of actual issues that'd at the very least need to be documented in code comments). Others agreed that it's something that needs to be done. > Instead of saying "yeah, that's right", people go off in every random direction, with people arguing about your NFS mount options, what things can cause write to do that and whether they are possible[1] or they should care about them. Given that changing the mount option solved the immediate problem for the OP, I fail to see how it's random. > [1] spoiler alert: it doesn't matter. The API allows for this, you need to handle it. Sure, it's not like the return value was ignored before though. I agree retrying is the better response, but erroring out is a form of handling (and will also lead to retries in several places, just with a loop that's not tightly around the write()).

lugg 8y ago
You had me until you tried to justify bubbling the error instead of handling it at its source while possible.
- anarazel 8y ago
  Hm? I explicitly said that retries would be better?
- naasking 8y ago
  That can be a correct response, for instance, if you have some sort of async i/o in place so you don't want to wait for a write that's not ready because you'll block.
- deleted 8y ago
  [deleted]
jbverschoor 8y ago
They were not ewview comments. They were arguments about where it ends. He's totalyl unwilling, and says you shouldn't run of of NFS. But then other people are, and are talking about mount options, when the problem clearly lies in the code not following the specs.
- deleted 8y ago
  [deleted]
- anarazel 8y ago
  Why are: > 1. If writes need to be retried, why not reads? (No, that's not an invitation to expand the scope of the patch; it's a question about NFS implementation.) and > 4. As coded, the patch behaves incorrectly if you get a zero return on a retry. If we were going to do this, I think we'd need to absorb the errno-munging currently done by callers into the writeAll function. not review comments? > when the problem clearly lies in the code not following the specs. There were plenty of questions about how exactly the fix should look like below https://www.postgresql.org/message-id/AD4A13A5-8778-4D94-BBB5-AB6C13BF752A%40phlo.org https://www.postgresql.org/message-id/AD4A13A5-8778-4D94-BBB... How's that not review?
thewsc 8y ago
> Given that changing the mount option solved the immediate problem for the OP, I fail to see how it's random. It's random because pretty much anyone could volunteer that opinion. By diagnosing the problem, making a patch and reaching out the author has already made their intention of fixing the problem clear. Unless it has been established that the problem isn't valid a workaround isn't really relevant. Effective communication and exchange of ideas needs to have a good ratio between work and value. I used to frequent meetups where people would present projects with thousands of hours or work and priorities behind them. There would almost always be someone with less experience stating their "ideas" on what should be done instead. Eventually those people would end up talking among themselves where their ideas could flow freely without any restriction of actual work being done. Experienced people provide value. They try to understand the problem, add their own experience to it and validate the work that has already been done. They make the problem smaller and closer to a solution. They don't, or shouldn't, casually increase the scope for little reason.
- anarazel 8y ago
  > It's random because pretty much anyone could volunteer that opinion. By diagnosing the problem, making a patch and reaching out the author has already made their intention of fixing the problem clear. Unless it has been established that the problem isn't valid a workaround isn't really relevant. I'm baffled by this. Even if the fix had been immediately committed, the workaround of using nointr still would have been valuable, because a fixed version of postgres wouldn't immediately have been released. You seem to argue in a way that entirely counteract your own later comments.
  - thewsc 8y ago
    If someone spends x amount of work hours on something, that is what they want feedback on. They aren't looking for quick suggestions on other paths to explore. It isn't a brain storming session. It is work done being represented by an e-mail, code or a product. You are being presented with their theory for a solution. At some point something else might be relevant, but that isn't something to assume. The assumption should be that the person presenting have made their choices based on their situation. It is often the same with software. Good feedback on software isn't random ideas, suggestions or feature requests that adds hundreds of hours of work on a whim. It is feedback that considers the work that has already been done. Anyone can come up with something else, especially in theory and with a blank slate. It doesn't really require anything other than an opinion. Hacker News certainly is proof of that.
acqq 8y ago
> I agree retrying is the better response, but erroring out is a form of handling In this case it is “a form” but that specific handling is provably wrong. That is important: everybody could have tried and proved again. Therefore just dismissing the correct handling and keeping the wrong one is, let me repeat, also provably wrong.

lugg 8y ago

You had me until you tried to justify bubbling the error instead of handling it at its source while possible.

anarazel 8y ago
Hm? I explicitly said that retries would be better?
naasking 8y ago
That can be a correct response, for instance, if you have some sort of async i/o in place so you don't want to wait for a write that's not ready because you'll block.
deleted 8y ago
[deleted]

jbverschoor 8y ago

They were not ewview comments. They were arguments about where it ends. He's totalyl unwilling, and says you shouldn't run of of NFS. But then other people are, and are talking about mount options, when the problem clearly lies in the code not following the specs.

deleted 8y ago
[deleted]
anarazel 8y ago
Why are: > 1. If writes need to be retried, why not reads? (No, that's not an invitation to expand the scope of the patch; it's a question about NFS implementation.) and > 4. As coded, the patch behaves incorrectly if you get a zero return on a retry. If we were going to do this, I think we'd need to absorb the errno-munging currently done by callers into the writeAll function. not review comments? > when the problem clearly lies in the code not following the specs. There were plenty of questions about how exactly the fix should look like below https://www.postgresql.org/message-id/AD4A13A5-8778-4D94-BBB5-AB6C13BF752A%40phlo.org https://www.postgresql.org/message-id/AD4A13A5-8778-4D94-BBB... How's that not review?

thewsc 8y ago

> Given that changing the mount option solved the immediate problem for the OP, I fail to see how it's random. It's random because pretty much anyone could volunteer that opinion. By diagnosing the problem, making a patch and reaching out the author has already made their intention of fixing the problem clear. Unless it has been established that the problem isn't valid a workaround isn't really relevant. Effective communication and exchange of ideas needs to have a good ratio between work and value. I used to frequent meetups where people would present projects with thousands of hours or work and priorities behind them. There would almost always be someone with less experience stating their "ideas" on what should be done instead. Eventually those people would end up talking among themselves where their ideas could flow freely without any restriction of actual work being done. Experienced people provide value. They try to understand the problem, add their own experience to it and validate the work that has already been done. They make the problem smaller and closer to a solution. They don't, or shouldn't, casually increase the scope for little reason.

anarazel 8y ago
> It's random because pretty much anyone could volunteer that opinion. By diagnosing the problem, making a patch and reaching out the author has already made their intention of fixing the problem clear. Unless it has been established that the problem isn't valid a workaround isn't really relevant. I'm baffled by this. Even if the fix had been immediately committed, the workaround of using nointr still would have been valuable, because a fixed version of postgres wouldn't immediately have been released. You seem to argue in a way that entirely counteract your own later comments.
- thewsc 8y ago
  If someone spends x amount of work hours on something, that is what they want feedback on. They aren't looking for quick suggestions on other paths to explore. It isn't a brain storming session. It is work done being represented by an e-mail, code or a product. You are being presented with their theory for a solution. At some point something else might be relevant, but that isn't something to assume. The assumption should be that the person presenting have made their choices based on their situation. It is often the same with software. Good feedback on software isn't random ideas, suggestions or feature requests that adds hundreds of hours of work on a whim. It is feedback that considers the work that has already been done. Anyone can come up with something else, especially in theory and with a blank slate. It doesn't really require anything other than an opinion. Hacker News certainly is proof of that.

acqq 8y ago

> I agree retrying is the better response, but erroring out is a form of handling In this case it is “a form” but that specific handling is provably wrong. That is important: everybody could have tried and proved again. Therefore just dismissing the correct handling and keeping the wrong one is, let me repeat, also provably wrong.