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

roachpb: remove PushTxnRequest.InclusivePushTo #43227

Merged

Conversation

nvanbenschoten
Copy link
Member

This completes the migration started in #35297, which was partially finalized in #38446.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei)

@nvanbenschoten
Copy link
Member Author

bors r+

@craig
Copy link
Contributor

craig bot commented Dec 27, 2019

Merge conflict (retrying...)

1 similar comment
@craig
Copy link
Contributor

craig bot commented Dec 27, 2019

Merge conflict (retrying...)

This completes the migration started in cockroachdb#35297, which was partially
finalized in cockroachdb#38446.

Release note: None
@nvanbenschoten nvanbenschoten force-pushed the nvanbenschoten/remInclusivePushTo branch from 2579356 to 3bdbd6d Compare December 27, 2019 23:01
@craig
Copy link
Contributor

craig bot commented Dec 27, 2019

Canceled

@nvanbenschoten
Copy link
Member Author

bors r+

craig bot pushed a commit that referenced this pull request Dec 27, 2019
43227: roachpb: remove PushTxnRequest.InclusivePushTo r=nvanbenschoten a=nvanbenschoten

This completes the migration started in #35297, which was partially finalized in #38446.

Release note: None

43316: storagebase: take RangeDesc by reference in ContainsKey(Range) r=nvanbenschoten a=nvanbenschoten

All callers of this had a *RangeDescriptor, so there's no reason to pass in a large RangeDescriptor struct.

Release note: None

43563: storage/intentresolver: don't capture loop iteration vars in async task r=nvanbenschoten a=nvanbenschoten

It's unclear if we've ever seen issues from this, but I intend to backport the
fix to v19.2, v19.1, and v2.1. I believe the worst thing that could have
happened is that a batch that observed multiple intents or pushed multiple txns
would only end up cleaning up a single one of these. It would then run into some
of these intents again when it tried to re-evaluate, forcing it to push again.
This subverts the parallelism that we were trying to achieve here, but would
never cause a stall.

Release note (bug fix): Ensure that all intents or transactions that a
batch observes are asynchronously cleaned up.

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
@craig
Copy link
Contributor

craig bot commented Dec 27, 2019

Build succeeded

@craig craig bot merged commit 3bdbd6d into cockroachdb:master Dec 27, 2019
@nvanbenschoten nvanbenschoten deleted the nvanbenschoten/remInclusivePushTo branch December 28, 2019 04:16
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.

None yet

3 participants