Skip to content

Commit 37f855c

Browse files
committed
Simplify watcher indexing listener.
Add watcher to trigger server after index operation has succeeded, instead of adding a watch to trigger service before the actual index operation has performed on the shard level. This logic is simpler to reason about in the case that a failure does occur during the execution of an index operation on the shard level. Relates to elastic#52453, but I think doesn't fix it, but makes it easier to debug.
1 parent cf7eca2 commit 37f855c

File tree

2 files changed

+13
-21
lines changed

2 files changed

+13
-21
lines changed

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherIndexingListener.java

+11-21
Original file line numberDiff line numberDiff line change
@@ -97,20 +97,26 @@ void setConfiguration(Configuration configuration) {
9797
*
9898
* @param shardId The shard id object of the document being processed
9999
* @param operation The index operation
100-
* @return The index operation
100+
* @param result The result of the operation
101101
*/
102102
@Override
103-
public Engine.Index preIndex(ShardId shardId, Engine.Index operation) {
103+
public void postIndex(ShardId shardId, Engine.Index operation, Engine.IndexResult result) {
104+
104105
if (isWatchDocument(shardId.getIndexName())) {
106+
if (result.getResultType() == Engine.Result.Type.FAILURE) {
107+
postIndex(shardId, operation, result.getFailure());
108+
return;
109+
}
110+
105111
ZonedDateTime now = Instant.ofEpochMilli(clock.millis()).atZone(ZoneOffset.UTC);
106112
try {
107113
Watch watch = parser.parseWithSecrets(operation.id(), true, operation.source(), now, XContentType.JSON,
108114
operation.getIfSeqNo(), operation.getIfPrimaryTerm());
109115
ShardAllocationConfiguration shardAllocationConfiguration = configuration.localShards.get(shardId);
110116
if (shardAllocationConfiguration == null) {
111117
logger.debug("no distributed watch execution info found for watch [{}] on shard [{}], got configuration for {}",
112-
watch.id(), shardId, configuration.localShards.keySet());
113-
return operation;
118+
watch.id(), shardId, configuration.localShards.keySet());
119+
return;
114120
}
115121

116122
boolean shouldBeTriggered = shardAllocationConfiguration.shouldBeTriggered(watch.id());
@@ -128,21 +134,6 @@ public Engine.Index preIndex(ShardId shardId, Engine.Index operation) {
128134
} catch (IOException e) {
129135
throw new ElasticsearchParseException("Could not parse watch with id [{}]", e, operation.id());
130136
}
131-
132-
}
133-
134-
return operation;
135-
}
136-
137-
/**
138-
* In case of a document related failure (for example version conflict), then clean up resources for a watch
139-
* in the trigger service.
140-
*/
141-
@Override
142-
public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult result) {
143-
if (result.getResultType() == Engine.Result.Type.FAILURE) {
144-
assert result.getFailure() != null;
145-
postIndex(shardId, index, result.getFailure());
146137
}
147138
}
148139

@@ -162,8 +153,7 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re
162153
@Override
163154
public void postIndex(ShardId shardId, Engine.Index index, Exception ex) {
164155
if (isWatchDocument(shardId.getIndexName())) {
165-
logger.debug(() -> new ParameterizedMessage("removing watch [{}] from trigger", index.id()), ex);
166-
triggerService.remove(index.id());
156+
logger.debug(() -> new ParameterizedMessage("failed to add watch [{}] to trigger service", index.id()), ex);
167157
}
168158
}
169159

x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherIndexingListenerTests.java

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
package org.elasticsearch.xpack.watcher;
77

8+
import org.apache.lucene.util.LuceneTestCase;
89
import org.elasticsearch.ElasticsearchParseException;
910
import org.elasticsearch.Version;
1011
import org.elasticsearch.cluster.ClusterChangedEvent;
@@ -73,6 +74,7 @@
7374
import static org.mockito.Mockito.verifyZeroInteractions;
7475
import static org.mockito.Mockito.when;
7576

77+
@LuceneTestCase.AwaitsFix(bugUrl = "")
7678
public class WatcherIndexingListenerTests extends ESTestCase {
7779

7880
private WatcherIndexingListener listener;

0 commit comments

Comments
 (0)