Labib,
> it is not that I am too keen on my code being included, but I am still > glad > to get some response. > Maybe I didn't make clear what the intent of my patches was: > I didn't want to add any feature. I just wanted to change JAMES a little > bit > to behave friendlier even in unexpected conditions. I think I understand the point of your changes, I just don't necessarily agree with them. I like some of your changes, I don't like all of them. I don't think the sleeps make the system any friendlier. > > i) I don't think there should be a hardcoded 2 sec delay for > > the bounce > > window. > The reason is that I wanted to forbid rebounces happening without user > interaction or at least they should happen slowly (2 s). This is rather > lax > since one might argue that rebounces don't make sense at all. > Maybe you have got a better solution? However, if you don't like this > protection against rebouncing: leave it. As my email makes clear, I don't object to the bounce window itself, I just want it to be configurable. That is, a hard coded 2 second parameter is not acceptable for an enterprise piece of software. It's not flexible. I think the bounce window is a nice addition, so long as you can adjust the window. There is nothing special about 2 seconds. > > iii) There is no javadoc provided for the new methods in NotifySender > > and NotifyPostmaster contrary to Jakarta code standards. > yeah. you are right. sorry. I will add some docs and repost the code. btw. > i No worries. > did diff the files with 'cvs diff'. thats not okay? Use 'cvs diff -u' > > I'm less crazy about the sleeps, not because I think they'll have a > > disastrous effect on performance, but rather because they > > seem like the > > wrong solution to the stated problem. Basically I have no idea what > > makes these particular exceptions merit sleep calls, while others > > throughout the system do not. Yes, they're in loops, but > > that's hardly > > unusual. > Hmm, maybe you don't like this principle of mine. In these loops all > exceptions are caught. You can try to determine every possible cause and > still forget something, like it happend. The bug itself is minor in my > reckoning. But its effect can be disastrous! At least on a machine where > JAMES runs with root rights like it normally will. On our machine it > doesn't, but we don't have quotas enabled. Soon the whole machine will > crash > or at least hang. > Thus after dealing with the foreseen exceptions you should in an endless > loop at least sleep after a not foreseen exception occurred before you try > executing the same code again. The spots where I added the sleeps should > never be reached as I understand the code. Nope, I don't like that principle at all. As I said, there isn't much special about the particular points where you're inserting the sleeps. Moreover, the above principle makes an assumption that I really don't like - that there are no side effects associated with the exception. For example (and there are a couple of examples of this in the current code base), I could catch an exception, deal with the troublesome issue (remove it from the spool, etc.) and re-throw the exception to notify the error handler up above. In that case you'd have the thread sleep unnecessarily for two seconds. Waste of resources. This kind of change really doesn't seem to me to improve product robustness in the face of error. It doesn't really do anything but make the logs smaller before you can hit Ctrl-C. It doesn't do anything to i) notify someone there's been an unforeseen error or ii) modify the situation to resolve the error condition. The change seems to tacitly assume that this problem occurs while someone is sitting there looking at the logs. In my experience that's not when unexpected problems arise. > > It seems like the way to solve this is by handling the > > NullPointerException in the initphase - not by adding a sleep > > to the run > > that makes it easier to kill the system once it's running and throwing > > errors. > Yes. But see my comments above. See my comments above. > > And again, on the general principle that hard coded timeouts > > are bad, if > > we're going to put in these sleep calls I don't see why they'd be > > hardcoded to a fixed 2 sec. > These calls are kind of "infunctional". One would never hope them to be > called. They are just a bit of garbage in the class file to protect you > against something unkown. There is no need to argue about the value or its > configurability. I disagree. If we're going to add the sleeps then these values have to be configurable. They're system level parameters that fundamentally affect the error handling. You can't just hard code them to some magic value that works in one environment. --Peter -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
