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 excessive increments in soft delete policy #38813

Merged
merged 7 commits into from
Feb 13, 2019

Conversation

jasontedor
Copy link
Member

In this case, we were incrementing the policy too much. This means on every iteration we actually keep increasing the minimum retained sequence number, even with leases in place. It was a bug from when the soft deletes policy had retention leases incorporated into it. This commit fixes this bug by ensuring we only increment in the proper places, and adds careful tests for the various situations.

Relates #37167

In this case, we were incrementing the policy too much. This means on
every iteration we actually keep increasing the minimum retained
sequence number, even with leases in place. It was a bug from when the
soft deletes policy had retention leases incorporated into it. This
commit fixes this bug by ensuring we only increment in the proper
places, and adds careful tests for the various situations.
@jasontedor jasontedor added >non-issue v7.0.0 :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v6.7.0 v8.0.0 v7.2.0 labels Feb 12, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@jasontedor jasontedor changed the title Fix off-by-one error in soft delete policy Fix excessive increments in soft delete policy Feb 12, 2019
Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

Great find. LGTM.

@jasontedor
Copy link
Member Author

@elasticmachine run elasticsearch-ci/1

* master:
  Remove _type term filters from cluster alert watches (elastic#38819)
  Adjust log and unmute testFailOverOnFollower (elastic#38762)
  Fix line separators in JSON logging tests (elastic#38771)
  Fix synchronization in LocalCheckpointTracker#contains (elastic#38755)
  muted test
  Remove TLSv1.2 pinning in ssl reload tests (elastic#38651)
  Don't fail init script if `/proc/.../max_map_count` absent (elastic#35933)
  Format Watcher.status.lastChecked and lastMetCondition (elastic#38626)
  SQL: Implement `::` cast operator (elastic#38774)
  Ignore failing test
* master:
  Improve CcrRepositoryIT mappings tests (elastic#38817)
  Unmute testClusterJoinDespiteOfPublishingIssues and testElectMasterWithLatestVersion (elastic#38555)
  ML allow aliased .ml-anomalies* index on PUT Job (elastic#38821)
  Filter out upgraded version index settings when starting index following (elastic#38838)
  Handle the fact that `ShardStats` instance may have no commit or seqno stats (elastic#38782)
  Edits to text of Profile API documentation (elastic#38742)
  muted test
@jasontedor
Copy link
Member Author

The latest failure looks an infrastructure issue. Let us try this again.

@elasticmachine run elasticsearch-ci/default-distro

@jasontedor jasontedor merged commit 9631c1a into elastic:master Feb 13, 2019
jasontedor added a commit that referenced this pull request Feb 13, 2019
In this case, we were incrementing the policy too much. This means on
every iteration we actually keep increasing the minimum retained
sequence number, even with leases in place. It was a bug from when the
soft deletes policy had retention leases incorporated into it. This
commit fixes this bug by ensuring we only increment in the proper
places, and adds careful tests for the various situations.
jasontedor added a commit that referenced this pull request Feb 13, 2019
In this case, we were incrementing the policy too much. This means on
every iteration we actually keep increasing the minimum retained
sequence number, even with leases in place. It was a bug from when the
soft deletes policy had retention leases incorporated into it. This
commit fixes this bug by ensuring we only increment in the proper
places, and adds careful tests for the various situations.
jasontedor added a commit that referenced this pull request Feb 13, 2019
In this case, we were incrementing the policy too much. This means on
every iteration we actually keep increasing the minimum retained
sequence number, even with leases in place. It was a bug from when the
soft deletes policy had retention leases incorporated into it. This
commit fixes this bug by ensuring we only increment in the proper
places, and adds careful tests for the various situations.
@jasontedor jasontedor deleted the soft-delete-policy-off-by-one branch February 13, 2019 19:17
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 13, 2019
* master:
  Fix excessive increments in soft delete policy (elastic#38813)
  Perform precise check for types warnings in cluster restart tests. (elastic#37944)
  [ML] Extract base class for integ tests with native processes (elastic#38850)
  Add get file chunk timeouts with listener timeouts (elastic#38758)
  Fix PreConfiguredTokenFilters getSynonymFilter() implementations (elastic#38839)
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.7.0 v7.0.0-rc1 v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants