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

vendor: bump Pebble to a24bc6c83054489b1bb0d9f53bcdd4ad8ec75977 #53208

Merged
merged 1 commit into from
Aug 22, 2020

Conversation

sumeerbhola
Copy link
Collaborator

a24bc6c sstable: optimize seeks to use next
76afcca db: optimize levelIter for repeated seeks with narrow and monotonic bounds

Release note: None

@sumeerbhola sumeerbhola requested a review from itsbilal August 21, 2020 16:36
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

LGTM, though @knz should speak up regarding the upgrade of cockroachdb/errors. Or you can figure out how to revert that portion of this bump.

@knz
Copy link
Contributor

knz commented Aug 21, 2020

yes you should revert that -- the crdb code is not ready for it, you'll get plenty of CI failures.
I'm doing the errors bump in #53199.

a24bc6c sstable: optimize seeks to use next
76afcca db: optimize levelIter for repeated seeks with narrow and monotonic bounds

Release note: None
Copy link
Collaborator Author

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

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

Done

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

Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

I think @knz is already bumping cockroachdb/redact in #53199. Not sure if that also is fraught to upgrade here, but if we want to be really safe I'd leave it out of this PR. I'm curious why these bits are getting picked up. I thought there was a way to bump only a single dependency.

@sumeerbhola
Copy link
Collaborator Author

I think @knz is already bumping cockroachdb/redact in #53199. Not sure if that also is fraught to upgrade here, but if we want to be really safe I'd leave it out of this PR. I'm curious why these bits are getting picked up. I thought there was a way to bump only a single dependency.

They are getting picked up simply by running go get -u github.com/cockroachdb/pebble@a24bc6c83054489b1bb0d9f53bcdd4ad8ec75977

Looks like @knz is picking up the same versions of redact, datadriven and pretty. Given the CI is green here and not in #53199 and it is late there, can I go ahead and merge this?

Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

LGTM

@sumeerbhola
Copy link
Collaborator Author

TFTR!

@sumeerbhola
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 22, 2020

This PR was included in a batch that was canceled, it will be automatically retried

@craig
Copy link
Contributor

craig bot commented Aug 22, 2020

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 22, 2020

Build succeeded:

@craig craig bot merged commit 5d1428a into cockroachdb:master Aug 22, 2020
@knz
Copy link
Contributor

knz commented Aug 22, 2020

Looks like @knz is picking up the same versions of redact, datadriven and pretty. Given the CI is green here and not in #53199 and it is late there, can I go ahead and merge this?

The concern I had was for cockroachdb/errors specifically. The redact and the other packages were 100% fine to upgrade. Thanks for merging this!

@sumeerbhola sumeerbhola deleted the pebble_bump branch November 11, 2020 00:01
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.

4 participants