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

storage: discard a unworthwhile merge TODO #28885

Merged
merged 1 commit into from
Aug 21, 2018

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Aug 21, 2018

This TODO suggested removing maybeWatchForMerge by teaching the LHS
replica to reach into the RHS replica after a merge committed to unblock
requests. It failed to consider that we'd need to do the same if the
merge aborted. We don't presently have an abort trigger, so this is
excessively difficult. Simply discard the TODO; the status quo is fine.

Release note: None

This TODO suggested removing maybeWatchForMerge by teaching the LHS
replica to reach into the RHS replica after a merge committed to unblock
requests. It failed to consider that we'd need to do the same if the
merge aborted. We don't presently have an abort trigger, so this is
excessively difficult. Simply discard the TODO; the status quo is fine.

Release note: None
@benesch benesch requested review from tbg and a team August 21, 2018 06:09
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg
Copy link
Member

tbg commented Aug 21, 2018 via email

@benesch
Copy link
Contributor Author

benesch commented Aug 21, 2018

bors r=tschottdorf

craig bot pushed a commit that referenced this pull request Aug 21, 2018
28885: storage: discard a unworthwhile merge TODO r=tschottdorf a=benesch

This TODO suggested removing maybeWatchForMerge by teaching the LHS
replica to reach into the RHS replica after a merge committed to unblock
requests. It failed to consider that we'd need to do the same if the
merge aborted. We don't presently have an abort trigger, so this is
excessively difficult. Simply discard the TODO; the status quo is fine.

Release note: None

Co-authored-by: Nikhil Benesch <[email protected]>
@craig
Copy link
Contributor

craig bot commented Aug 21, 2018

Build succeeded

@craig craig bot merged commit 583cc94 into cockroachdb:master Aug 21, 2018
@benesch benesch deleted the merge-todo branch August 21, 2018 21:08
craig bot pushed a commit that referenced this pull request Aug 27, 2018
29033: backport-2.1: storage: improve consistency checker diff, add debug decode-key r=a-robinson a=tschottdorf

Backport 2/2 commits from #29012.

/cc @cockroachdb/release

---

Avoid spilling non-printable characters and generally garbage into the
logs.

Release note: None


29082: backport-2.1: one week of merge PRs r=knz,tschottdorf a=benesch

Backport:
  * 1/1 commits from "sql: put rendered setting value in event log entry" (#29014)
  * 2/2 commits from " sql,settings: make SET CLUSTER SETTING wait for value to update, disallow in txn" (#29063)
  * 1/1 commits from "storage: discard a unworthwhile merge TODO" (#28885)
  * 3/3 commits from "storage: gate merges behind a cluster setting " (#28865)
  * 1/1 commits from "storage: update the tscache appropriately after a merge" (#28793)
  * 1/1 commits from "storage: avoid stopping twice in merge test" (#28902)
  * 1/1 commits from "roachpb,storage: rename GetSnapshotForMerge to Subsume" (#28910)
  * 1/1 commits from "build: fix generation rules for settings.html" (#28884)
  * 1/1 commits from "storage: deflake TestStoreRangeMergeTimestampCacheCausality" (#28928)
  * 1/1 commits from "storage: check for in-progress merge before installing new lease" (#28894)
  * 6/7 commits from " storage: enable merge queue by default" (#28961)

Please see individual PRs for details.

/cc @cockroachdb/release


Co-authored-by: Tobias Schottdorf <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Nikhil Benesch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants