Skip to content

Commit 6be57f8

Browse files
dnhatndnhatn
authored
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 f3ddd36 commit 6be57f8

File tree

12 files changed

+135
-88
lines changed

12 files changed

+135
-88
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.9.99"
5+
reason: "Noop does not return seq_no and primary_term until 8.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
@@ -368,8 +368,8 @@ public abstract static class Builder {
368368
protected Result result = null;
369369
protected boolean forcedRefresh;
370370
protected ShardInfo shardInfo = null;
371-
protected Long seqNo = UNASSIGNED_SEQ_NO;
372-
protected Long primaryTerm = UNASSIGNED_PRIMARY_TERM;
371+
protected long seqNo = UNASSIGNED_SEQ_NO;
372+
protected long primaryTerm = UNASSIGNED_PRIMARY_TERM;
373373

374374
public ShardId getShardId() {
375375
return shardId;
@@ -411,11 +411,11 @@ public void setShardInfo(ShardInfo shardInfo) {
411411
this.shardInfo = shardInfo;
412412
}
413413

414-
public void setSeqNo(Long seqNo) {
414+
public void setSeqNo(long seqNo) {
415415
this.seqNo = seqNo;
416416
}
417417

418-
public void setPrimaryTerm(Long primaryTerm) {
418+
public void setPrimaryTerm(long primaryTerm) {
419419
this.primaryTerm = primaryTerm;
420420
}
421421

‎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;
@@ -681,7 +682,8 @@ void markCurrentItemAsDropped() {
681682
new BulkItemResponse(currentSlot, indexRequest.opType(),
682683
new UpdateResponse(
683684
new ShardId(indexRequest.index(), IndexMetaData.INDEX_UUID_NA_VALUE, 0),
684-
indexRequest.type(), indexRequest.id(), indexRequest.version(), DocWriteResponse.Result.NOOP
685+
indexRequest.type(), indexRequest.id(), SequenceNumbers.UNASSIGNED_SEQ_NO, SequenceNumbers.UNASSIGNED_PRIMARY_TERM,
686+
indexRequest.version(), DocWriteResponse.Result.NOOP
685687
)
686688
)
687689
);

‎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: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import org.elasticsearch.index.engine.VersionConflictEngineException;
4747
import org.elasticsearch.index.mapper.Mapping;
4848
import org.elasticsearch.index.mapper.MetadataFieldMapper;
49-
import org.elasticsearch.index.seqno.SequenceNumbers;
5049
import org.elasticsearch.index.shard.IndexShard;
5150
import org.elasticsearch.index.shard.IndexShardTestCase;
5251
import org.elasticsearch.index.shard.ShardId;
@@ -438,8 +437,7 @@ public void testNoopUpdateRequest() throws Exception {
438437
.doc(Requests.INDEX_CONTENT_TYPE, "field", "value");
439438
BulkItemRequest primaryRequest = new BulkItemRequest(0, writeRequest);
440439

441-
DocWriteResponse noopUpdateResponse = new UpdateResponse(shardId, "_doc", "id", 0,
442-
DocWriteResponse.Result.NOOP);
440+
DocWriteResponse noopUpdateResponse = new UpdateResponse(shardId, "_doc", "id", 0, 2, 1, DocWriteResponse.Result.NOOP);
443441

444442
IndexShard shard = mock(IndexShard.class);
445443

@@ -471,7 +469,7 @@ public void testNoopUpdateRequest() throws Exception {
471469
equalTo(DocWriteResponse.Result.NOOP));
472470
assertThat(bulkShardRequest.items().length, equalTo(1));
473471
assertEquals(primaryRequest, bulkShardRequest.items()[0]); // check that bulk item was not mutated
474-
assertThat(primaryResponse.getResponse().getSeqNo(), equalTo(SequenceNumbers.UNASSIGNED_SEQ_NO));
472+
assertThat(primaryResponse.getResponse().getSeqNo(), equalTo(0L));
475473
}
476474

477475
public void testUpdateRequestWithFailure() throws Exception {

‎server/src/test/java/org/elasticsearch/action/update/UpdateResponseTests.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.elasticsearch.common.xcontent.XContentType;
3333
import org.elasticsearch.index.get.GetResult;
3434
import org.elasticsearch.index.get.GetResultTests;
35+
import org.elasticsearch.index.seqno.SequenceNumbers;
3536
import org.elasticsearch.index.shard.ShardId;
3637
import org.elasticsearch.test.ESTestCase;
3738
import org.elasticsearch.test.RandomObjects;
@@ -54,7 +55,7 @@ public class UpdateResponseTests extends ESTestCase {
5455

5556
public void testToXContent() throws IOException {
5657
{
57-
UpdateResponse updateResponse = new UpdateResponse(new ShardId("index", "index_uuid", 0), "type", "id", 0, NOT_FOUND);
58+
UpdateResponse updateResponse = new UpdateResponse(new ShardId("index", "index_uuid", 0), "type", "id", -2, 0, 0, NOT_FOUND);
5859
String output = Strings.toString(updateResponse);
5960
assertEquals("{\"_index\":\"index\",\"_type\":\"type\",\"_id\":\"id\",\"_version\":0,\"result\":\"not_found\"," +
6061
"\"_shards\":{\"total\":0,\"successful\":0,\"failed\":0}}", output);
@@ -162,21 +163,21 @@ public static Tuple<UpdateResponse, UpdateResponse> randomUpdateResponse(XConten
162163

163164
// We also want small number values (randomNonNegativeLong() tend to generate high numbers)
164165
// in order to catch some conversion error that happen between int/long after parsing.
165-
Long seqNo = randomFrom(randomNonNegativeLong(), (long) randomIntBetween(0, 10_000), null);
166-
long primaryTerm = seqNo == null ? 0 : randomIntBetween(1, 16);
166+
long seqNo = randomFrom(randomNonNegativeLong(), (long) randomIntBetween(0, 10_000), SequenceNumbers.UNASSIGNED_SEQ_NO);
167+
long primaryTerm = seqNo == SequenceNumbers.UNASSIGNED_SEQ_NO ? SequenceNumbers.UNASSIGNED_PRIMARY_TERM : randomIntBetween(1, 16);
167168

168169
ShardId actualShardId = new ShardId(index, indexUUid, shardId);
169170
ShardId expectedShardId = new ShardId(index, INDEX_UUID_NA_VALUE, -1);
170171

171172
UpdateResponse actual, expected;
172-
if (seqNo != null) {
173+
if (seqNo != SequenceNumbers.UNASSIGNED_SEQ_NO) {
173174
Tuple<ReplicationResponse.ShardInfo, ReplicationResponse.ShardInfo> shardInfos = RandomObjects.randomShardInfo(random());
174175

175176
actual = new UpdateResponse(shardInfos.v1(), actualShardId, type, id, seqNo, primaryTerm, version, result);
176177
expected = new UpdateResponse(shardInfos.v2(), expectedShardId, type, id, seqNo, primaryTerm, version, result);
177178
} else {
178-
actual = new UpdateResponse(actualShardId, type, id, version, result);
179-
expected = new UpdateResponse(expectedShardId, type, id, version, result);
179+
actual = new UpdateResponse(actualShardId, type, id, seqNo, primaryTerm, version, result);
180+
expected = new UpdateResponse(expectedShardId, type, id, seqNo, primaryTerm, version, result);
180181
}
181182

182183
if (actualGetResult.isExists()) {

0 commit comments

Comments
 (0)