Skip to content

Commit d128188

Browse files
dnhatndnhatn
committed
Return seq_no and primary_term in noop update (#44603)
With this change, we will return primary_term and seq_no of the current document if an update is detected as a noop. We already return the version; hence we should also return seq_no and primary_term. Relates #42497
1 parent aebfdf1 commit d128188

File tree

12 files changed

+136
-89
lines changed

12 files changed

+136
-89
lines changed

‎client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/bulk/RestNoopBulkAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
8787

8888
private static class BulkRestBuilderListener extends RestBuilderListener<BulkRequest> {
8989
private final BulkItemResponse ITEM_RESPONSE = new BulkItemResponse(1, DocWriteRequest.OpType.UPDATE,
90-
new UpdateResponse(new ShardId("mock", "", 1), "mock_type", "1", 1L, DocWriteResponse.Result.CREATED));
90+
new UpdateResponse(new ShardId("mock", "", 1), "mock_type", "1", 0L, 1L, 1L, DocWriteResponse.Result.CREATED));
9191

9292
private final RestRequest request;
9393

‎client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/bulk/TransportNoopBulkAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
public class TransportNoopBulkAction extends HandledTransportAction<BulkRequest, BulkResponse> {
3636
private static final BulkItemResponse ITEM_RESPONSE = new BulkItemResponse(1, DocWriteRequest.OpType.UPDATE,
37-
new UpdateResponse(new ShardId("mock", "", 1), "mock_type", "1", 1L, DocWriteResponse.Result.CREATED));
37+
new UpdateResponse(new ShardId("mock", "", 1), "mock_type", "1", 0L, 1L, 1L, DocWriteResponse.Result.CREATED));
3838

3939
@Inject
4040
public TransportNoopBulkAction(TransportService transportService, ActionFilters actionFilters) {

‎docs/reference/docs/update.asciidoc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ the request was ignored.
194194
"_type": "_doc",
195195
"_id": "1",
196196
"_version": 7,
197+
"_primary_term": 1,
198+
"_seq_no": 6,
197199
"result": "noop"
198200
}
199201
--------------------------------------------------

‎modules/reindex/src/test/java/org/elasticsearch/index/reindex/AsyncBulkByScrollActionTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -885,8 +885,8 @@ void doExecute(ActionType<Response> action, Request request, ActionListener<Resp
885885
true);
886886
} else if (item instanceof UpdateRequest) {
887887
UpdateRequest update = (UpdateRequest) item;
888-
response = new UpdateResponse(shardId, update.type(), update.id(),
889-
randomIntBetween(0, Integer.MAX_VALUE), Result.CREATED);
888+
response = new UpdateResponse(shardId, update.type(), update.id(), randomNonNegativeLong(),
889+
randomIntBetween(1, Integer.MAX_VALUE), randomIntBetween(0, Integer.MAX_VALUE), Result.CREATED);
890890
} else if (item instanceof DeleteRequest) {
891891
DeleteRequest delete = (DeleteRequest) item;
892892
response =

‎rest-api-spec/src/main/resources/rest-api-spec/test/update/16_noop.yml‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
"Noop":
3+
- skip:
4+
version: " - 7.3.99"
5+
reason: "Noop does not return seq_no and primary_term until 7.4.0"
6+
- do:
7+
index:
8+
index: test_1
9+
type: test
10+
id: 1
11+
body: { foo: bar }
12+
13+
- match: { _seq_no: 0 }
14+
- match: { _version: 1 }
15+
- match: { _primary_term: 1 }
16+
- match: { result: created }
17+
18+
- do:
19+
update:
20+
index: test_1
21+
type: test
22+
id: 1
23+
body:
24+
doc: { foo: bar }
25+
26+
- match: { _seq_no: 0 }
27+
- match: { _version: 1 }
28+
- match: { _primary_term: 1 }
29+
- match: { result: noop }
30+
31+
- do:
32+
update:
33+
index: test_1
34+
type: test
35+
id: 1
36+
body:
37+
doc: { foo: bar }
38+
detect_noop: false
39+
40+
- match: { _seq_no: 1 }
41+
- match: { _primary_term: 1 }
42+
- match: { _version: 2 }
43+
- match: { result: updated }

‎server/src/main/java/org/elasticsearch/action/DocWriteResponse.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,8 @@ public abstract static class Builder {
376376
protected Result result = null;
377377
protected boolean forcedRefresh;
378378
protected ShardInfo shardInfo = null;
379-
protected Long seqNo = UNASSIGNED_SEQ_NO;
380-
protected Long primaryTerm = UNASSIGNED_PRIMARY_TERM;
379+
protected long seqNo = UNASSIGNED_SEQ_NO;
380+
protected long primaryTerm = UNASSIGNED_PRIMARY_TERM;
381381

382382
public ShardId getShardId() {
383383
return shardId;
@@ -419,11 +419,11 @@ public void setShardInfo(ShardInfo shardInfo) {
419419
this.shardInfo = shardInfo;
420420
}
421421

422-
public void setSeqNo(Long seqNo) {
422+
public void setSeqNo(long seqNo) {
423423
this.seqNo = seqNo;
424424
}
425425

426-
public void setPrimaryTerm(Long primaryTerm) {
426+
public void setPrimaryTerm(long primaryTerm) {
427427
this.primaryTerm = primaryTerm;
428428
}
429429

‎server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import org.elasticsearch.index.IndexNotFoundException;
6363
import org.elasticsearch.index.IndexSettings;
6464
import org.elasticsearch.index.VersionType;
65+
import org.elasticsearch.index.seqno.SequenceNumbers;
6566
import org.elasticsearch.index.shard.ShardId;
6667
import org.elasticsearch.indices.IndexClosedException;
6768
import org.elasticsearch.ingest.IngestService;
@@ -685,7 +686,8 @@ void markCurrentItemAsDropped() {
685686
new BulkItemResponse(currentSlot, indexRequest.opType(),
686687
new UpdateResponse(
687688
new ShardId(indexRequest.index(), IndexMetaData.INDEX_UUID_NA_VALUE, 0),
688-
indexRequest.type(), indexRequest.id(), indexRequest.version(), DocWriteResponse.Result.NOOP
689+
indexRequest.type(), indexRequest.id(), SequenceNumbers.UNASSIGNED_SEQ_NO, SequenceNumbers.UNASSIGNED_PRIMARY_TERM,
690+
indexRequest.version(), DocWriteResponse.Result.NOOP
689691
)
690692
)
691693
);

‎server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ Result prepareUpsert(ShardId shardId, UpdateRequest request, final GetResult get
140140
break;
141141
case NONE:
142142
UpdateResponse update = new UpdateResponse(shardId, getResult.getType(), getResult.getId(),
143-
getResult.getVersion(), DocWriteResponse.Result.NOOP);
143+
getResult.getSeqNo(), getResult.getPrimaryTerm(), getResult.getVersion(), DocWriteResponse.Result.NOOP);
144144
update.setGetResult(getResult);
145145
return new Result(update, DocWriteResponse.Result.NOOP, upsertResult.v2(), XContentType.JSON);
146146
default:
@@ -194,7 +194,7 @@ Result prepareUpdateIndexRequest(ShardId shardId, UpdateRequest request, GetResu
194194
// where users repopulating multi-fields or adding synonyms, etc.
195195
if (detectNoop && noop) {
196196
UpdateResponse update = new UpdateResponse(shardId, getResult.getType(), getResult.getId(),
197-
getResult.getVersion(), DocWriteResponse.Result.NOOP);
197+
getResult.getSeqNo(), getResult.getPrimaryTerm(), getResult.getVersion(), DocWriteResponse.Result.NOOP);
198198
update.setGetResult(extractGetResult(request, request.index(), getResult.getSeqNo(), getResult.getPrimaryTerm(),
199199
getResult.getVersion(), updatedSourceAsMap, updateSourceContentType, getResult.internalSourceRef()));
200200
return new Result(update, DocWriteResponse.Result.NOOP, updatedSourceAsMap, updateSourceContentType);
@@ -257,7 +257,7 @@ Result prepareUpdateScriptRequest(ShardId shardId, UpdateRequest request, GetRes
257257
default:
258258
// If it was neither an INDEX or DELETE operation, treat it as a noop
259259
UpdateResponse update = new UpdateResponse(shardId, getResult.getType(), getResult.getId(),
260-
getResult.getVersion(), DocWriteResponse.Result.NOOP);
260+
getResult.getSeqNo(), getResult.getPrimaryTerm(), getResult.getVersion(), DocWriteResponse.Result.NOOP);
261261
update.setGetResult(extractGetResult(request, request.index(), getResult.getSeqNo(), getResult.getPrimaryTerm(),
262262
getResult.getVersion(), updatedSourceAsMap, updateSourceContentType, getResult.internalSourceRef()));
263263
return new Result(update, DocWriteResponse.Result.NOOP, updatedSourceAsMap, updateSourceContentType);

‎server/src/main/java/org/elasticsearch/action/update/UpdateResponse.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.common.xcontent.XContentBuilder;
2626
import org.elasticsearch.common.xcontent.XContentParser;
2727
import org.elasticsearch.index.get.GetResult;
28-
import org.elasticsearch.index.seqno.SequenceNumbers;
2928
import org.elasticsearch.index.shard.ShardId;
3029
import org.elasticsearch.rest.RestStatus;
3130

@@ -50,8 +49,8 @@ public UpdateResponse(StreamInput in) throws IOException {
5049
* Constructor to be used when a update didn't translate in a write.
5150
* For example: update script with operation set to none
5251
*/
53-
public UpdateResponse(ShardId shardId, String type, String id, long version, Result result) {
54-
this(new ShardInfo(0, 0), shardId, type, id, SequenceNumbers.UNASSIGNED_SEQ_NO, 0, version, result);
52+
public UpdateResponse(ShardId shardId, String type, String id, long seqNo, long primaryTerm, long version, Result result) {
53+
this(new ShardInfo(0, 0), shardId, type, id, seqNo, primaryTerm, version, result);
5554
}
5655

5756
public UpdateResponse(
@@ -152,10 +151,10 @@ public void setGetResult(GetResult getResult) {
152151
@Override
153152
public UpdateResponse build() {
154153
UpdateResponse update;
155-
if (shardInfo != null && seqNo != null) {
154+
if (shardInfo != null) {
156155
update = new UpdateResponse(shardInfo, shardId, type, id, seqNo, primaryTerm, version, result);
157156
} else {
158-
update = new UpdateResponse(shardId, type, id, version, result);
157+
update = new UpdateResponse(shardId, type, id, seqNo, primaryTerm, version, result);
159158
}
160159
if (getResult != null) {
161160
update.setGetResult(new GetResult(update.getIndex(), update.getType(), update.getId(),

‎server/src/test/java/org/elasticsearch/action/bulk/TransportShardBulkActionTests.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@
4747
import org.elasticsearch.index.engine.VersionConflictEngineException;
4848
import org.elasticsearch.index.mapper.Mapping;
4949
import org.elasticsearch.index.mapper.MetadataFieldMapper;
50-
import org.elasticsearch.index.seqno.SequenceNumbers;
5150
import org.elasticsearch.index.shard.IndexShard;
5251
import org.elasticsearch.index.shard.IndexShardTestCase;
5352
import org.elasticsearch.index.shard.ShardId;
@@ -129,7 +128,7 @@ public void testShouldExecuteReplicaItem() throws Exception {
129128
equalTo(ReplicaItemExecutionMode.FAILURE));
130129
// NOOP requests should not be replicated
131130
DocWriteRequest<UpdateRequest> updateRequest = new UpdateRequest("index", "type", "id");
132-
response = new UpdateResponse(shardId, "type", "id", 1, DocWriteResponse.Result.NOOP);
131+
response = new UpdateResponse(shardId, "type", "id", 0, 1, 1, DocWriteResponse.Result.NOOP);
133132
request = new BulkItemRequest(0, updateRequest);
134133
request.setPrimaryResponse(new BulkItemResponse(0, DocWriteRequest.OpType.UPDATE,
135134
response));
@@ -481,8 +480,7 @@ public void testNoopUpdateRequest() throws Exception {
481480
.doc(Requests.INDEX_CONTENT_TYPE, "field", "value");
482481
BulkItemRequest primaryRequest = new BulkItemRequest(0, writeRequest);
483482

484-
DocWriteResponse noopUpdateResponse = new UpdateResponse(shardId, "_doc", "id", 0,
485-
DocWriteResponse.Result.NOOP);
483+
DocWriteResponse noopUpdateResponse = new UpdateResponse(shardId, "_doc", "id", 0, 2, 1, DocWriteResponse.Result.NOOP);
486484

487485
IndexShard shard = mock(IndexShard.class);
488486

@@ -514,7 +512,7 @@ public void testNoopUpdateRequest() throws Exception {
514512
equalTo(DocWriteResponse.Result.NOOP));
515513
assertThat(bulkShardRequest.items().length, equalTo(1));
516514
assertEquals(primaryRequest, bulkShardRequest.items()[0]); // check that bulk item was not mutated
517-
assertThat(primaryResponse.getResponse().getSeqNo(), equalTo(SequenceNumbers.UNASSIGNED_SEQ_NO));
515+
assertThat(primaryResponse.getResponse().getSeqNo(), equalTo(0L));
518516
}
519517

520518
public void testUpdateRequestWithFailure() throws Exception {

0 commit comments

Comments
 (0)