Expose sequence number and primary terms in search responses #37639

bleskes · 2019-01-20T14:41:40Z

Users may require the sequence number and primary terms to perform optimistic concurrency control operations. Currently, you can get the sequence number via the docvalues_fields API but the primary term is not accessible because it is maintained by the SeqNoFieldMapper and the infrastructure can't find it.

This PR adds a dedicated sub fetch phase to return both numbers that is connected to a new seq_no_primary_term parameter.

it's irrelevant

elasticmachine · 2019-01-20T14:41:42Z

Pinging @elastic/es-search

jimczi

The change looks good, I left some comments regarding xcontent parsing and tests.

jimczi · 2019-01-21T08:06:50Z

server/src/main/java/org/elasticsearch/search/SearchHit.java

@@ -587,6 +618,8 @@ public static SearchHit createFromMap(Map<String, Object> values) {
        }
        searchHit.score(get(Fields._SCORE, values, DEFAULT_SCORE));
        searchHit.version(get(Fields._VERSION, values, -1L));
+        searchHit.setSeqNo(get(Fields._SEQ_NO, values, SequenceNumbers.UNASSIGNED_SEQ_NO));


You also need to declare the field in SearchHit#declareInnerHitsParseFields ?

jimczi · 2019-01-21T08:06:55Z

server/src/main/java/org/elasticsearch/search/SearchHit.java

@@ -587,6 +618,8 @@ public static SearchHit createFromMap(Map<String, Object> values) {
        }
        searchHit.score(get(Fields._SCORE, values, DEFAULT_SCORE));
        searchHit.version(get(Fields._VERSION, values, -1L));
+        searchHit.setSeqNo(get(Fields._SEQ_NO, values, SequenceNumbers.UNASSIGNED_SEQ_NO));
+        searchHit.setPrimaryTerm(get(Fields._PRIMARY_TERM, values, SequenceNumbers.UNASSIGNED_PRIMARY_TERM));


Same here ?

done. What tests am I missing that were supposed to catch it?

SearchHitTests and SearchHitsTests, they are supposed to test the (de)serialization of random search hits using xcontent and streamable but you need to fill these fields in the createTestItem function to ensure that they are tested.

jimczi · 2019-01-21T08:11:15Z

server/src/main/java/org/elasticsearch/search/SearchHit.java

+    public void setPrimaryTerm(long primaryTerm) {
+        this.primaryTerm = primaryTerm;
+    }
+


Can you add these fields in SearchHitTests#createTestItem to test all types of (de)serialization ?

jimczi

LGTM

jpountz · 2019-01-21T17:03:51Z

server/src/main/java/org/elasticsearch/search/SearchHit.java

+    /** returns the primary term of the last modification to the document */
+    public long getPrimaryTerm() {
+        return this.primaryTerm;
+    }


maybe document default return values, if seq_no_primary_term is not set?

bleskes · 2019-01-22T10:32:18Z

@elasticmachine please run elasticsearch-ci/2

bleskes · 2019-01-22T11:29:33Z

@elasticmachine please run elasticsearch-ci/2

bleskes · 2019-01-22T13:39:34Z

@elasticmachine please run elasticsearch-ci/2

…ead-de-duplication * elastic/master: (24 commits) [TEST] Mute MlMappingsUpgradeIT testMappingsUpgrade Streamline skip_unavailable handling (elastic#37672) Only bootstrap and elect node in current voting configuration (elastic#37712) Ensure either success or failure path for SearchOperationListener is called (elastic#37467) Target only specific index in update settings test Add a note how to benchmark Elasticsearch Don't use Groovy's `withDefault` (elastic#37726) Adapt SyncedFlushService (elastic#37691) Mute FilterAggregatorTests#testRandom Switch mapping/aggregations over to java time (elastic#36363) [ML] Update ML results mappings on process start (elastic#37706) Modify removal_of_types.asciidoc (elastic#37648) Fix edge case in PutMappingRequestTests (elastic#37665) Use new bulk API endpoint in the docs (elastic#37698) Expose sequence number and primary terms in search responses (elastic#37639) Remove LicenseServiceClusterNotRecoveredTests (elastic#37528) Migrate SpecificMasterNodesIT to Zen2 (elastic#37532) Fix MetaStateFormat tests Use plain text instead of latexmath Fix a typo in a warning message in TestFixturesPlugin (elastic#37631) ...

Users may require the sequence number and primary terms to perform optimistic concurrency control operations. Currently, you can get the sequence number via the `docvalues_fields` API but the primary term is not accessible because it is maintained by the `SeqNoFieldMapper` and the infrastructure can't find it. This commit adds a dedicated sub fetch phase to return both numbers that is connected to a new `seq_no_primary_term` parameter.

* elastic/master: Optimize warning header de-duplication (elastic#37725) Bubble exceptions up in ClusterApplierService (elastic#37729) SQL: Improve handling of invalid args for PERCENTILE/PERCENTILE_RANK (elastic#37803) Remove unused ThreadBarrier class (elastic#37666) Add built-in user and role for code plugin (elastic#37030) Consolidate testclusters tests into a single project (elastic#37362) Fix docs for MappingUpdatedAction SQL: Introduce SQL DATE data type (elastic#37693) disabling bwc test while backporting elastic#37639 Mute ClusterDisruptionIT testAckedIndexing Set acking timeout to 0 on dynamic mapping update (elastic#31140) Remove index audit output type (elastic#37707) Mute FollowerFailOverIT testReadRequestsReturnsLatestMappingVersion [ML] Increase close job timeout and lower the max number (elastic#37770) Remove Custom Listeners from SnapshotsService (elastic#37629) Use m_m_nodes from Zen1 master for Zen2 bootstrap (elastic#37701) Fix index filtering in follow info api. (elastic#37752) Use project dependency instead of substitutions for distributions (elastic#37730) Update authenticate to allow unknown fields (elastic#37713) Deprecate HLRC EmptyResponse used by security (elastic#37540)

… 6.6 docs This turned out to not be true and is tackled by #37639 . That PR will be part of 6.7

… 6.6 docs (#37876) This turned out to not be true and is tackled by #37639 . That PR will be part of 6.7

…y control (#37857) The delete and update by query APIs both offer protection against overriding concurrent user changes to the documents they touch. They currently are using internal versioning. This PR changes that to rely on sequences numbers and primary terms. Relates #37639 Relates #36148 Relates #10708

…y control (elastic#37857) The delete and update by query APIs both offer protection against overriding concurrent user changes to the documents they touch. They currently are using internal versioning. This PR changes that to rely on sequences numbers and primary terms. Relates elastic#37639 Relates elastic#36148 Relates elastic#10708

…37872 (#38155) * Move update and delete by query to use seq# for optimistic concurrency control (#37857) The delete and update by query APIs both offer protection against overriding concurrent user changes to the documents they touch. They currently are using internal versioning. This PR changes that to rely on sequences numbers and primary terms. Relates #37639 Relates #36148 Relates #10708 * Add Seq# based optimistic concurrency control to UpdateRequest (#37872) The update request has a lesser known support for a one off update of a known document version. This PR adds an a seq# based alternative to power these operations. Relates #36148 Relates #10708 * Move watcher to use seq# and primary term for concurrency control (#37977) * Adapt minimum versions for seq# power operations After backporting #37977, #37857 and #37872

bleskes added 16 commits January 17, 2019 10:22

add fetch phase

4ab4133

fix InnerHitBuilder.equals and TopHitsAggregationBuilder parsing

75df265

fix to xcontent

da27bf7

fix seq no loading

51f1117

doh

a4208dd

and rest tests and fix

9611838

fix indenting

d161f08

fix rest tests

87f4ef8

nested fields

5d20b18

more rest fixes

38c1710

fixed collapsing

f1716af

top hits move away from nested

6d7df1e

remove seq_no_primary_term from testInitialSearchParamsMisc

e56da1e

it's irrelevant

remote cluster fix

6ba7569

remove seq_no_and_term

f6d3c08

add seq no + primary term to examples

3d745ca

bleskes added >enhancement :Search/Search Search-related issues that do not fall into other categories v7.0.0 v6.7.0 labels Jan 20, 2019

bleskes requested a review from jimczi January 20, 2019 14:41

bleskes added 3 commits January 20, 2019 17:54

fix parent_child

772d8fc

fix testMonitoringBulk

7b38fe5

remove left over seq_no_and_primary_term

3400b49

jimczi requested changes Jan 21, 2019

View reviewed changes

bleskes added 2 commits January 21, 2019 10:25

Merge remote-tracking branch 'upstream/master' into cas_seq_no_in_search

0fc235e

feedback

cecf185

bleskes requested a review from jimczi January 21, 2019 09:34

jimczi approved these changes Jan 21, 2019

View reviewed changes

bleskes added 2 commits January 21, 2019 15:26

feedback

a7a71d6

add documentation

c3ec471

bleskes force-pushed the cas_seq_no_in_search branch from a330e32 to c3ec471 Compare January 21, 2019 16:18

jpountz approved these changes Jan 21, 2019

View reviewed changes

bleskes added 3 commits January 21, 2019 18:13

java doc defaults

6c88411

fix 300_sequence_numbers.yml

40b5656

Merge remote-tracking branch 'upstream/master' into cas_seq_no_in_search

e56eee6

bleskes merged commit 52ba407 into elastic:master Jan 23, 2019

bleskes deleted the cas_seq_no_in_search branch January 23, 2019 08:04

bleskes added a commit that referenced this pull request Jan 24, 2019

disabling bwc test while backporting #37639

b6317ed

bleskes added a commit that referenced this pull request Jan 24, 2019

enable bwc tests and bump versions after backporting #37639

af2f4c8

bleskes mentioned this pull request Jan 25, 2019

Move update and delete by query to use seq# for optimistic concurrency control #37857

Merged

bleskes added a commit that referenced this pull request Jan 25, 2019

Remove statement about search and optimistic concurrency control from…

5dcae0e

… 6.6 docs This turned out to not be true and is tackled by #37639 . That PR will be part of 6.7

bleskes mentioned this pull request Jan 25, 2019

Remove statement about search and optimistic concurrency control from 6.6 docs #37876

Merged

bleskes added a commit that referenced this pull request Jan 25, 2019

Remove statement about search and optimistic concurrency control from…

f96bc41

… 6.6 docs (#37876) This turned out to not be true and is tackled by #37639 . That PR will be part of 6.7

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose sequence number and primary terms in search responses #37639

Expose sequence number and primary terms in search responses #37639

bleskes commented Jan 20, 2019 •

edited

Loading

elasticmachine commented Jan 20, 2019

jimczi left a comment

jimczi Jan 21, 2019

jimczi Jan 21, 2019

bleskes Jan 21, 2019

jimczi Jan 21, 2019

jimczi Jan 21, 2019

bleskes Jan 21, 2019

jimczi left a comment

jpountz Jan 21, 2019

bleskes Jan 21, 2019

bleskes commented Jan 22, 2019

bleskes commented Jan 22, 2019

bleskes commented Jan 22, 2019

Expose sequence number and primary terms in search responses #37639

Expose sequence number and primary terms in search responses #37639

Conversation

bleskes commented Jan 20, 2019 • edited Loading

elasticmachine commented Jan 20, 2019

jimczi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimczi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bleskes commented Jan 22, 2019

bleskes commented Jan 22, 2019

bleskes commented Jan 22, 2019

bleskes commented Jan 20, 2019 •

edited

Loading