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

[7.x] Fix wrong error upper bound when performing incremental reductions (#43874) #76475

Merged
merged 2 commits into from
Aug 16, 2021

Conversation

imotov
Copy link
Contributor

@imotov imotov commented Aug 12, 2021

When performing incremental reductions, 0 value of docCountError may mean that
the error was not previously calculated, or that the error was indeed previously
calculated and its value was 0. We end up rejecting true values set to 0 this
way. This may lead to wrong upper bound of error in result. To fix it, this PR
makes docCountError nullable. null values mean that error was not calculated
yet.

Fixes #40005, #75667

Hohol and others added 2 commits August 12, 2021 12:27
…lastic#43874)

When performing incremental reductions, 0 value of docCountError may mean that
the error was not previously calculated, or that the error was indeed previously
calculated and its value was 0. We end up rejecting true values set to 0 this
way. This may lead to wrong upper bound of error in result. To fix it, this PR
makes docCountError nullable. null values mean that error was not calculated
yet.

Fixes elastic#40005

Co-authored-by: Igor Motov <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Fix docCountError calculation in case of multiple reduces. It fixes 2 mistakes
in elastic#43874. The first error was introduced in the original PR, where unknown doc
count errors were initialized equal to 0, the second was introduced during in
order to fix the first one by ignoring these 0s, which essentially disabled the
original fix.

Fixes elastic#75667
imotov added a commit to imotov/elasticsearch that referenced this pull request Aug 15, 2021
Disabling bwc tests to merge elastic#76475
imotov added a commit that referenced this pull request Aug 16, 2021
Disabling bwc tests to merge #76475
@imotov imotov merged commit d13ec5e into elastic:7.x Aug 16, 2021
imotov added a commit to imotov/elasticsearch that referenced this pull request Aug 16, 2021
Reenabling bwc tests after merge of elastic#76475
imotov added a commit that referenced this pull request Aug 16, 2021
Reenabling bwc tests after merge of #76475
wjp719 added a commit to wjp719/elasticsearch that referenced this pull request Aug 17, 2021
* master: (868 commits)
  Query API key - Rest spec and yaml tests (elastic#76238)
  Delay shard reassignment from nodes which are known to be restarting (elastic#75606)
  Reenable bwc tests for elastic#76475 (elastic#76576)
  Set version to 7.15 in BWC code (elastic#76577)
  Don't remove warning headers on all failure (elastic#76434)
  Disable bwc tests for elastic#76475 (elastic#76541)
  Re-enable bwc tests (elastic#76567)
  Keep track of data recovered from snapshots in RecoveryState (elastic#76499)
  [Transform] Align transform checkpoint range with date_histogram interval for better performance (elastic#74004)
  EQL: Remove "wildcard" function (elastic#76099)
  Fix 'accept' and 'content_type' fields for search_mvt API
  Add persistent licensed feature tracking (elastic#76476)
  Add system data streams to feature state snapshots (elastic#75902)
  fix the error message for instance methods that don't exist (elastic#76512)
  ILM: Add validation of the number_of_shards parameter in Shrink Action of ILM (elastic#74219)
  remove dashboard only reserved role (elastic#76507)
  Fix Stack Overflow in UnassignedInfo in Corner Case (elastic#76480)
  Add (Extended)KeyUsage KeyUsage, CipherSuite & Protocol to SSL diagnostics (elastic#65634)
  Add recovery from snapshot to tests (elastic#76535)
  Reenable BwC Tests after elastic#76532 (elastic#76534)
  ...
@imotov imotov deleted the nullable-doc-count-error-7.x branch October 2, 2021 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants