Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix from_range in search_after in changes snapshot #33335

Merged
merged 2 commits into from
Sep 3, 2018

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Sep 2, 2018

I introduced this bug while addressing code review in #33222 (comment).

We can have multiple documents in Lucene with the same seq_no for parent-child documents (or without rollback). In this case, the usage "lastSeenSeqNo + 1" is an off-by-one error as it may miss some documents. This error merely affects the skippedOperations contract.

I labeled "non-issue" for this non-released bug.

Closes #33318

@dnhatn dnhatn added >non-issue v7.0.0 :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v6.5.0 labels Sep 2, 2018
@dnhatn dnhatn requested review from jpountz and s1monw September 2, 2018 00:57
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@s1monw s1monw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dnhatn
Copy link
Member Author

dnhatn commented Sep 3, 2018

Thanks @s1monw and @jpountz.

@dnhatn dnhatn merged commit 24d60c7 into elastic:master Sep 3, 2018
@dnhatn dnhatn deleted the fix-from-range branch September 3, 2018 15:58
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 4, 2018
…e-default-distribution

* elastic/master: (213 commits)
  ML: Fix build after HLRC change
  Fix inner hits retrieval when stored fields are disabled (_none_) (elastic#33018)
  SQL: Show/desc commands now support table ids (elastic#33363)
  Mute testValidateFollowingIndexSettings
  HLRC: Add delete by query API (elastic#32782)
  [ML] The sort field on get records should default to the record_score (elastic#33358)
  [ML] Minor improvements to categorization Grok pattern creation (elastic#33353)
  [DOCS] fix a couple of typos (elastic#33356)
  Disable assemble task instead of removing it (elastic#33348)
  Simplify the return type of FieldMapper#parse. (elastic#32654)
  [ML] Delete forecast API (elastic#31134) (elastic#33218)
  Introduce private settings (elastic#33327)
  [Docs] Add search timeout caveats (elastic#33354)
  TESTS: Fix Race Condition in Temp Path Creation (elastic#33352)
  Fix from_range in search_after in changes snapshot (elastic#33335)
  TESTS+DISTR.: Fix testIndexCheckOnStartup Flake (elastic#33349)
  Null completion field should not throw IAE (elastic#33268)
  Adds code to help with IndicesRequestCacheIT failures (elastic#33313)
  Prevent NPE parsing the stop datafeed request. (elastic#33347)
  HLRC: Add ML get overall buckets API (elastic#33297)
  ...
dnhatn added a commit that referenced this pull request Sep 5, 2018
We can have multiple documents in Lucene with the same seq_no for
parent-child documents (or without rollback). In this case, the usage
"lastSeenSeqNo + 1" is an off-by-one error as it may miss some
documents. This error merely affects the `skippedOperations` contract.

See: #33222 (comment)

Closes #33318
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >non-issue v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants