Remove most uses of boolean success from IndexWriter and ReadersAndUpdates#14668
Remove most uses of boolean success from IndexWriter and ReadersAndUpdates#14668ChrisHegarty merged 3 commits intoapache:mainfrom
Conversation
|
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog-check label to it and you will stop receiving this reminder on future updates to the PR. |
|
I've taken the opportunity to add the exception to logged messages, now it's accessible at the point the message is created. I'm not sure if this is something that should be in those messages though. |
| } | ||
| if (infoStream.isEnabled("IW")) { | ||
| infoStream.message("IW", "hit exception updating document: " + t); | ||
| } |
There was a problem hiding this comment.
I've tried to review this but the number of possibilities is, ahem, large. Some of these changes are not strictly equivalent, even if it's highly theoretical to hit such paths. For example, in the change above, if tragicEvent throws an exception (info stream may cause it), previously it'd still run finally, now it won't. If there is a regression somewhere, it'll be fun debugging it.
There was a problem hiding this comment.
I've modified onTragicEvent to ensure it doesn't throw, and any throwables are suppressed (if possible). This does leave the case that an assertion failure when tragedy is null will throw an AssertionError that now doesn't call the cleanup - but that assertion should never be hit outside of tests with new modifications to existing code.
|
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog-check label to it and you will stop receiving this reminder on future updates to the PR. |
ee9a65b to
39c25df
Compare
|
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog-check label to it and you will stop receiving this reminder on future updates to the PR. |
|
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog-check label to it and you will stop receiving this reminder on future updates to the PR. |
Followon from #14633 - refactor uses of
successandsuccess2booleans