activemq-artemis icon indicating copy to clipboard operation
activemq-artemis copied to clipboard
verified publisher icon apache

ARTEMIS-2618 Added optional retry handling on critical I/O Error

Open MrEasy opened this issue 4 years ago • 3 comments

This can be used if running in environments where external applications temporarily lock journal files (e.g. virus scanners). Disabled by default.

MrEasy avatar Oct 14 '21 10:10 MrEasy

This idea has merit, but there a couple problems:

  • The code isn't formatted properly so CheckStyle is failing.
  • The retry is limited to org.apache.activemq.artemis.core.io.nio.NIOSequentialFile#open(int, boolean), but there are several implementations of org.apache.activemq.artemis.core.io.SequentialFile#open(int, boolean) which might benefit from such a retry.
  • The configuration should be available from broker.xml rather than just a system property.

jbertram avatar Feb 07 '22 16:02 jbertram

Can't argue with that of course. As you can see in the issue though, it has its 2nd birthday in 2 days :-) - so would prefer not investing more time if that then would not result in sth. releasable. Do you think the points you mentioned are the ones that should complete it?

MrEasy avatar Feb 08 '22 07:02 MrEasy

Do you think the points you mentioned are the ones that should complete it?

I can't speak for anyone else, but as far as I'm concerned I would say yes at this point.

jbertram avatar Feb 15 '22 18:02 jbertram

the reason I never merged this, was because I was more afraid of side effects than anything else.

What if the Open did leave a file handle open somewhere in the OS, and still issued a warning? How to test it (as a matter of fact there are no tests on this PR, and even if we had it.. it would be difficult to validate it).

and the reason I never closed it... it was I wanted to be polite and find a middle term solution.. but I could never accommodate the request in a way I felt comfortable on merging.

@MrEasy I will close this now, and close the JIRA as a won't fix... just because of those reasons.

Sorry it took me 2 years to close it.. but at the time you open I tried to find a solution and I never came to it.

And as anything... 2021 was a weird year to me... it seems 2 years passed like yesterday.

clebertsuconic avatar Aug 30 '22 20:08 clebertsuconic

Thanks for the update @clebertsuconic - 2020 was yesterday from my feeling ;) I still consider it a valuable change improving robustness (helped several times in our environments), but totally ok if not merged.

MrEasy avatar Sep 13 '22 06:09 MrEasy