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

[dependency] update rocksdb dependency to 0.17.0 #9046

Merged
merged 1 commit into from
Aug 30, 2021

Conversation

yuxiamit
Copy link
Contributor

Current dependency (rocksdb = 0.15.0) causes compiling issues on apple silicon chips: (facebook/rocksdb#7710). Updating the depending version solves this problem.

I haven't checked if this will break anything

Motivation

(Write your motivation for proposed changes here.)

Have you read the Contributing Guidelines on pull requests?

(Write your answer here.)

Test Plan

(Share your test plan here. If you changed code, please provide us with clear instructions for verifying that your changes work.)

Related PRs

(If this PR adds or changes functionality, please take some time to update or suggest changes to the docs at https://developers.diem.com, and link to your PR here.)

If targeting a release branch, please fill the below out as well

  • Justification and breaking nature (who does it affect? validators, full nodes, tooling, operators, AOS, etc.)
  • Comprehensive test results that demonstrate the fix working and not breaking existing workflows.
  • Why we must have it for V1 launch.
  • What workarounds and alternative we have if we do not push the PR.

@yuxiamit yuxiamit requested review from zekun000 and bmwill August 29, 2021 03:57
@yuxiamit yuxiamit force-pushed the rocksdb-dependency-update branch from baa462f to 9fa0728 Compare August 29, 2021 15:28
@zekun000 zekun000 requested a review from msmouse August 30, 2021 04:37
msmouse
msmouse previously approved these changes Aug 30, 2021
Copy link

@msmouse msmouse left a comment

Choose a reason for hiding this comment

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

cargo build on my Intel mac went through, so as long as CI passes we are good.

@msmouse
Copy link

msmouse commented Aug 30, 2021

/land

bors-libra pushed a commit that referenced this pull request Aug 30, 2021
@github-actions
Copy link

Cluster Test Result

Test runner setup time spent 254 secs
Compatibility test results for land_b6c6c83f ==> land_b77131c9 (PR)
1. All instances running land_b6c6c83f, generating some traffic on network
2. First full node land_b6c6c83f ==> land_b77131c9, to validate new full node to old validator node traffic
3. First Validator node land_b6c6c83f ==> land_b77131c9, to validate storage compatibility
4. First batch validators (14) land_b6c6c83f ==> land_b77131c9, to test consensus and traffic between old full nodes and new validator node
5. First batch full nodes (14) land_b6c6c83f ==> land_b77131c9
6. Second batch validators (15) land_b6c6c83f ==> land_b77131c9, to upgrade rest of the validators
7. Second batch of full nodes (15) land_b6c6c83f ==> land_b77131c9, to finish the network upgrade, time spent 649 secs
all up : 1274 TPS, 3554 ms latency, 4050 ms p99 latency, no expired txns, time spent 249 secs
Logs: http://kibana.ct-2-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-08-30T17:29:23Z',to:'2021-08-30T17:51:30Z'))
Dashboard: http://grafana.ct-2-k8s-testnet.aws.hlw3truzy4ls.com/d/performance/performance?from=1630344563000&to=1630345890000
Validator 1 logs: http://kibana.ct-2-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-08-30T17:29:23Z',to:'2021-08-30T17:51:30Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

Repro cmd:

./scripts/cti --tag land_b6c6c83f --cluster-test-tag land_b77131c9 -E BATCH_SIZE=15 -E UPDATE_TO_TAG=land_b77131c9 --report report.json --suite land_blocking_compat

🎉 Land-blocking cluster test passed! 👌

@bors-libra
Copy link
Contributor

💔 Test Failed - ci-test

@@ -14,6 +14,7 @@ edition = "2018"
Inflector = { version = "0.11.4", features = ["default", "heavyweight", "lazy_static", "regex"] }
anyhow = { version = "1.0.40", features = ["backtrace", "default", "std"] }
backtrace = { version = "0.3.56", features = ["addr2line", "default", "gimli-symbolize", "miniz_oxide", "object", "serde", "std"] }
bitvec = { version = "0.19.5", features = ["alloc", "atomic", "default", "std"] }
Copy link

Choose a reason for hiding this comment

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

Are these irrelevant?

Copy link
Contributor Author

@yuxiamit yuxiamit Aug 30, 2021

Choose a reason for hiding this comment

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

cargo build on my Intel mac went through, so as long as CI passes we are good.

Yes, it also works on my previous intel mac. The issue with rocksDB is that it assumed linux when arm64 is present.

Are these irrelevant?

This is generated by cargo x generate-workspace-hack after changing the version of rocksdb in storage/schemadb/Cargo.toml.

@zekun000
Copy link
Contributor

from @rexhoffman , you need to do a cargo build to update the Cargo.lock to pass the CI

@libra-action
Copy link

release binaries dependency change summary:

target packages:
  M rocksdb 0.17.0 (direct third-party, crates.io)
    * version upgraded from 0.15.0
    * (unchanged features: lz4)
  M librocksdb-sys 6.20.3 (transitive third-party, crates.io)
    * version upgraded from 6.11.4
    * (unchanged features: default, lz4, static)

host packages:
  A bitvec 0.19.5 (transitive third-party, crates.io)
    * features: alloc, std
  A funty 1.1.0 (transitive third-party, crates.io)
    * features: [none]
  A radium 0.5.3 (transitive third-party, crates.io)
    * features: [none]
  A tap 1.0.1 (transitive third-party, crates.io)
    * features: [none]
  A wyz 0.2.0 (transitive third-party, crates.io)
    * features: alloc
  M bindgen 0.59.1 (transitive third-party, crates.io)
    * version upgraded from 0.54.0
    * removed features: clap, default, env_logger, log, logging, which, which-rustfmt
    * (unchanged features: runtime)
  M cexpr 0.5.0 (transitive third-party, crates.io)
    * version upgraded from 0.4.0
    * (unchanged features: [none])
  M clang-sys 1.2.1 (transitive third-party, crates.io)
    * version upgraded from 0.29.3
    * added features: clang_3_5, clang_3_6, clang_3_7, clang_3_8, clang_3_9, clang_4_0, clang_5_0
    * removed features: gte_clang_3_6, gte_clang_3_7, gte_clang_3_8, gte_clang_3_9, gte_clang_4_0, gte_clang_5_0, gte_clang_6_0
    * (unchanged features: clang_6_0, libloading, runtime)
  M libloading 0.7.0 (transitive third-party, crates.io)
    * version upgraded from 0.5.2
    * (unchanged features: [none])
  M memchr 2.4.0 (transitive third-party, crates.io)
    * removed features: default
    * (unchanged features: std, use_std)
  M nom 6.1.2 (transitive third-party, crates.io)
    * version upgraded from 5.1.2
    * added features: bitvec, funty
    * (unchanged features: alloc, std)
  M regex 1.4.3 (transitive third-party, crates.io)
    * removed features: aho-corasick, default, memchr, perf, perf-cache, perf-dfa, perf-inline, perf-literal, thread_local
    * (unchanged features: std, unicode, unicode-age, unicode-bool, unicode-case, unicode-gencat, unicode-perl, unicode-script, unicode-segment)
  M regex-syntax 0.6.23 (transitive third-party, crates.io)
    * removed features: default
    * (unchanged features: unicode, unicode-age, unicode-bool, unicode-case, unicode-gencat, unicode-perl, unicode-script, unicode-segment)
  M shlex 1.1.0 (transitive third-party, crates.io)
    * version upgraded from 0.1.1
    * added features: default, std
    * (unchanged features: [none])
  R aho-corasick 0.7.15 (transitive third-party, crates.io)
    * (old features: default, std)
  R ansi_term 0.11.0 (transitive third-party, crates.io)
    * (old features: [none])
  R atty 0.2.14 (transitive third-party, crates.io)
    * (old features: [none])
  R cfg-if 0.1.10 (transitive third-party, crates.io) (other versions: 1.0.0)
    * (old features: [none])
  R clap 2.33.3 (transitive third-party, crates.io)
    * (old features: ansi_term, atty, color, default, strsim, suggestions, vec_map)
  R env_logger 0.7.1 (transitive third-party, crates.io)
    * (old features: atty, default, humantime, regex, termcolor)
  R humantime 1.3.0 (transitive third-party, crates.io)
    * (old features: [none])
  R log 0.4.14 (transitive third-party, crates.io)
    * (old features: std)
  R quick-error 1.2.3 (transitive third-party, crates.io)
    * (old features: [none])
  R strsim 0.8.0 (transitive third-party, crates.io)
    * (old features: [none])
  R termcolor 1.1.2 (transitive third-party, crates.io)
    * (old features: [none])
  R textwrap 0.11.0 (transitive third-party, crates.io)
    * (old features: [none])
  R thread_local 1.1.3 (transitive third-party, crates.io)
    * (old features: [none])
  R unicode-width 0.1.8 (transitive third-party, crates.io)
    * (old features: default)
  R vec_map 0.8.2 (transitive third-party, crates.io)
    * (old features: [none])
  R which 3.1.1 (transitive third-party, crates.io)
    * (old features: [none])

zekun000
zekun000 previously approved these changes Aug 30, 2021
@yuxiamit
Copy link
Contributor Author

/land

@github-actions
Copy link

Cluster Test Result

Test runner setup time spent 259 secs
Compatibility test results for land_7ce122e6 ==> land_c4d6aee9 (PR)
1. All instances running land_7ce122e6, generating some traffic on network
2. First full node land_7ce122e6 ==> land_c4d6aee9, to validate new full node to old validator node traffic
3. First Validator node land_7ce122e6 ==> land_c4d6aee9, to validate storage compatibility
4. First batch validators (14) land_7ce122e6 ==> land_c4d6aee9, to test consensus and traffic between old full nodes and new validator node
5. First batch full nodes (14) land_7ce122e6 ==> land_c4d6aee9
6. Second batch validators (15) land_7ce122e6 ==> land_c4d6aee9, to upgrade rest of the validators
7. Second batch of full nodes (15) land_7ce122e6 ==> land_c4d6aee9, to finish the network upgrade, time spent 691 secs
all up : 1256 TPS, 3607 ms latency, 4100 ms p99 latency, no expired txns, time spent 249 secs
Logs: http://kibana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-08-30T23:03:06Z',to:'2021-08-30T23:25:53Z'))
Dashboard: http://grafana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/d/performance/performance?from=1630364586000&to=1630365953000
Validator 1 logs: http://kibana.ct-0-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-08-30T23:03:06Z',to:'2021-08-30T23:25:53Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

Repro cmd:

./scripts/cti --tag land_7ce122e6 --cluster-test-tag land_c4d6aee9 -E BATCH_SIZE=15 -E UPDATE_TO_TAG=land_c4d6aee9 --report report.json --suite land_blocking_compat

🎉 Land-blocking cluster test passed! 👌

@bors-libra bors-libra merged commit c4d6aee into diem:main Aug 30, 2021
@bors-libra bors-libra temporarily deployed to Sccache August 30, 2021 23:34 Inactive
@bors-libra bors-libra temporarily deployed to Sccache August 30, 2021 23:35 Inactive
@bors-libra bors-libra temporarily deployed to Docker August 30, 2021 23:35 Inactive
@libra-action
Copy link

This PR contains dependency updates. However, Depdive update review report exceeds max. characters allowed in a GitHub comment. Please, run depdive locally or check actions.

@jsladerman jsladerman mentioned this pull request Jun 10, 2022
@jsladerman jsladerman mentioned this pull request Jun 22, 2022
rutkaracn pushed a commit to rutkaracn/work that referenced this pull request Dec 27, 2022
bors-diem pushed a commit that referenced this pull request Dec 28, 2022
bors-diem pushed a commit that referenced this pull request Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants