Return sentinel values from GET calls when sequence numbers are disabled#143575
Conversation
The _seq_no and _primary_term fields will return -1 for all GET and MGET calls, making it clearer that these fields are not available for searching or versioning when sequence numbers have been disabled at the index level.
|
Pinging @elastic/es-distributed (Team:Distributed) |
fcofdez
left a comment
There was a problem hiding this comment.
Looks good! I left a couple of suggestions around testing
| .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) | ||
| ) | ||
| ); | ||
| client().prepareIndex(index).setId("1").setSource("field", "value").get(); |
There was a problem hiding this comment.
can we randomly refresh/flush so we exercise all the get paths?
There was a problem hiding this comment.
Yes, ideally we test after refresh, after flush, after nothing (still in translog) etc
| ); | ||
| client().prepareIndex(index).setId("1").setSource("field", "value").get(); | ||
|
|
||
| GetResponse getResponse = client().prepareGet(index, "1").get(); |
There was a problem hiding this comment.
maybe we can randomly set realtime too?
|
|
||
| GetResponse getResponse = client().prepareGet(index, "1").get(); | ||
| assertTrue(getResponse.isExists()); | ||
| assertThat(getResponse.getSeqNo(), equalTo(SequenceNumbers.UNASSIGNED_SEQ_NO)); |
There was a problem hiding this comment.
I think that we discussed returning a different sentinel value for this case, but I see that it can add some complexity, so I'm ok with this.
| .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) | ||
| ) | ||
| ); | ||
| client().prepareIndex(index).setId("1").setSource("field", "value").get(); |
There was a problem hiding this comment.
Yes, ideally we test after refresh, after flush, after nothing (still in translog) etc
| assertThat(getResponse.getPrimaryTerm(), equalTo(SequenceNumbers.UNASSIGNED_PRIMARY_TERM)); | ||
|
|
||
| MultiGetResponse multiGetResponse = client().prepareMultiGet().add(index, "1").get(); | ||
| assertThat(multiGetResponse.getResponses().length, equalTo(1)); |
There was a problem hiding this comment.
nit: maybe worth testing on more than1 document
| Settings settings = Settings.builder() | ||
| .put(defaultSettings.getSettings()) | ||
| .put(IndexSettings.DISABLE_SEQUENCE_NUMBERS.getKey(), true) | ||
| .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) |
There was a problem hiding this comment.
If you can you add SEQ_NO_INDEX_OPTIONS_SETTING to docs values only already that's great.
There was a problem hiding this comment.
Sorry, the bot got there first!
…led (elastic#143575) The _seq_no and _primary_term fields will return -2 for all GET and MGET calls, making it clearer that these fields are not available for searching or versioning when sequence numbers have been disabled at the index level. Related to elastic#136305
…led (elastic#143575) The _seq_no and _primary_term fields will return -2 for all GET and MGET calls, making it clearer that these fields are not available for searching or versioning when sequence numbers have been disabled at the index level. Related to elastic#136305
The _seq_no and _primary_term fields will return -2 for all GET and MGET
calls, making it clearer that these fields are not available for searching or
versioning when sequence numbers have been disabled at the index level.
Related to #136305