The watcher indexing listener didn't handle document level exceptions. #51466
Conversation
Prior to the change the watcher index listener didn't implement the `postIndex(ShardId, Engine.Index, Engine.IndexResult)` method. This caused document level exceptions like VersionConflictEngineException to be ignored. This commit fixes this. The watcher indexing listener did implement the `postIndex(ShardId, Engine.Index, Exception)` method, but that only handles engine level exceptions. This change also unmutes the SmokeTestWatcherTestSuiteIT#testMonitorClusterHealth test again. Relates to elastic#32299
|
Pinging @elastic/es-core-features (:Core/Features/Watcher) |
There was a problem hiding this comment.
@martijnvg - great catch ! LGTM
For my own knowledge is there a way to know which exception types are document related failures vs. engine level exceptions ?
Also, I assume that anytime someone overrides postIndex(ShardId shardId, Engine.Index index, Exception ex) they should probably also override postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult result).
I wonder if the postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult result) override defined here, should instead be the default implementation in the interface ? (in a different PR, that also includes postDelete)
The bases class The way I look at this is, that document level exceptions are failures related to an operation on a single doc, that doesn't have consequences for other documents or a shard. For example a version conflict or failure during index time text analysis. The engine level failures affect the entire shard and can cause a shard to fail. For example a corruption that occurs during indexing.
Not sure. Indexing operation listeners are very low level and this very much depends on the implementation. |
#51466) Prior to the change the watcher index listener didn't implement the `postIndex(ShardId, Engine.Index, Engine.IndexResult)` method. This caused document level exceptions like VersionConflictEngineException to be ignored. This commit fixes this. The watcher indexing listener did implement the `postIndex(ShardId, Engine.Index, Exception)` method, but that only handles engine level exceptions. This change also unmutes the SmokeTestWatcherTestSuiteIT#testMonitorClusterHealth test again. Relates to #32299
Prior to the change the watcher index listener didn't implement the
postIndex(ShardId, Engine.Index, Engine.IndexResult)method. Thiscaused document level exceptions like VersionConflictEngineException
to be ignored. This commit fixes this.
The watcher indexing listener did implement the
postIndex(ShardId, Engine.Index, Exception)method, but that only handles engine level exceptions.
This change also unmutes the SmokeTestWatcherTestSuiteIT#testMonitorClusterHealth test again.
Relates to #32299 (this should fix the watch count bug, which wasn't subtracted from 1 to 0 due to VersionConflictEngineException being thrown after watcher updating the watch after it was deleted)