Pass through all exceptions in IndicesLifecycleListeners #9330
Conversation
clintongormley
added
:Core/Infra/Core
|
I wonder if we should only rethrow exceptions of a certain exception type(s). I think the original idea was to protect against exceptions in listener code from causing an unneeded engine failure. Maybe we can even add this as a throws clause to the interface and document it as an official way to block index creation. Not sure, but I think it's worth considering. @dakrone what is your opinion? |
|
@bleskes yeah I talked to @kimchy about that and we both agreed passing all exceptions seemed better. Passing all exceptions through I think is cleaner in that there isn't a hidden element of a certain type of exception. Additionally listeners should handle exceptions themselves with a try-catch rather than bubbling them up so that they have to consider the failure case (ie, if this listener fails, do I really want to fail the shard) @kimchy did you have any other reasons for preferring the re-throw versus certain exception type solution? |
| try { | ||
| client().admin().indices().prepareCreate("failed").setSettings(SETTING_NUMBER_OF_SHARDS, 1, "index.fail", true).get(); | ||
| fail("should have thrown an exception"); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
catch ElasticsearchException instead?
|
left a super minor comment, LGTM although I am not super sure about potential side effects that this change might have (not too familiar with this part of es) |
|
@dakrone yea, you and me are on the same page. Its very easy to comprehend when you are implementing the listener, and it becomes the responsibility of the listener to catch exceptions/failures that should not cause the failure to bubble up |
dakrone
force-pushed
the
fail-engine-from-listener
branch
from
b88813a
to
283a467
Compare
This allows a plugin or user that registers a listener to be able to stop actions like creating an index or starting a shard by throwing an exception. Previously all exceptions were logged without being rethrown.
This allows a plugin or user that registers a listener to be able to
stop actions like creating an index or starting a shard by throwing an
exception. Previously all exceptions were logged without being rethrown.