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

[2.0 backport] rocksdb: Update ref to include SST unique ID fix #41541

Closed

Conversation

itsbilal
Copy link
Contributor

This change updates the RocksDB ref on 2.0 to include this fix:
cockroachdb/rocksdb#65

Release note: None

This change updates the RocksDB ref on 2.0 to include this fix:
cockroachdb/rocksdb#65

Release note: None
@itsbilal itsbilal requested review from ajkr, petermattis, sumeerbhola and a team October 11, 2019 21:06
@itsbilal itsbilal self-assigned this Oct 11, 2019
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@itsbilal itsbilal changed the title rocksdb: Update ref to include SST unique ID fix [2.0 backport] rocksdb: Update ref to include SST unique ID fix Oct 11, 2019
@itsbilal
Copy link
Contributor Author

@garvitjuniwal You should be able to merge this into your branch

@itsbilal
Copy link
Contributor Author

Can't merge due to failing stale Docker acceptance tests on this branch. So I'll close this PR as-is and leave the branch in place; anyone interested can just merge this branch / update the ref as they find fit.

@neeral
Copy link
Contributor

neeral commented Oct 31, 2019

@itsbilal i'm running into the same problem with acceptance tests failing. What's your assessment of the failures? Does it indicate that this change to rocksdb could have introduced a bug? The acceptance tests are passing without this change.

These four tests are failing.

--- FAIL: TestDockerCLI (452.72s)
    --- FAIL: TestDockerCLI/test_missing_log_output.tcl (38.98s)
    --- FAIL: TestDockerCLI/test_secure.tcl (38.38s)
    --- FAIL: TestDockerCLI/test_url_db_override.tcl (2.41s)
--- FAIL: TestGossipRestartFirstNodeNeedsIncoming (4.24s)
    --- FAIL: TestGossipRestartFirstNodeNeedsIncoming/runMode=local (4.24s)

with unable to parse server version.

@itsbilal
Copy link
Contributor Author

@neeral Thanks for the ping! It's actually this issue: #33446 and is related to how we generate a version string from git describe. You should be able to add a version tag (eg. v2.0.x-rubrik) to the latest commit on your branch and that should fix it. Seems like the code fix for not having acceptance tests rely on it was backported to 2.1 but not 2.0.

@neeral
Copy link
Contributor

neeral commented Oct 31, 2019

Thanks, I've cherry-picked #33446 and it is working now :)

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