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

bug(ci): multi-version tikv-jemalloc-sys break docker pipeline #12315

Closed
MrCroxx opened this issue Sep 14, 2023 · 18 comments
Closed

bug(ci): multi-version tikv-jemalloc-sys break docker pipeline #12315

MrCroxx opened this issue Sep 14, 2023 · 18 comments
Labels
type/bug Something isn't working
Milestone

Comments

@MrCroxx
Copy link
Contributor

MrCroxx commented Sep 14, 2023

Describe the bug

RisingWave got 2 version of tikv-jemalloc-sys built in docker pipeline. And it breaks CI.

cp ./target/release/build/tikv-jemalloc-sys-*/out/build/bin/jeprof /risingwave/bin/
image

Error message/log

<img width="1055" alt="image" src="https://github.com/risingwavelabs/risingwave/assets/22407295/21c68b59-7d1b-47a8-82a9-df8f30f9bb03">
@MrCroxx MrCroxx added the type/bug Something isn't working label Sep 14, 2023
@github-actions github-actions bot added this to the release-1.3 milestone Sep 14, 2023
@MrCroxx
Copy link
Contributor Author

MrCroxx commented Sep 14, 2023

Thanks to @xxchan for helping. The bug might be introduced by #12276 .

UPDATE: It's caused by #12307

@MrCroxx
Copy link
Contributor Author

MrCroxx commented Sep 14, 2023

cargo tree -d got:

[build-dependencies]
├── foyer-workspace-hack v0.1.0 (https://github.com/mrcroxx/foyer?rev=41b1d39#41b1d393) (*)
└── workspace-hack v1.1.0-alpha (/home/mrcroxx/github/risingwavelabs/risingwave/src/workspace-hack) (*)

tikv-jemalloc-sys v0.5.4+5.3.0-patched (https://github.com/risingwavelabs/jemallocator.git?rev=64a2d9#64a2d988)
[build-dependencies]
└── workspace-hack v1.1.0-alpha (/home/mrcroxx/github/risingwavelabs/risingwave/src/workspace-hack) (*)

tikv-jemalloc-sys v0.5.4+5.3.0-patched (https://github.com/risingwavelabs/jemallocator.git?rev=64a2d9#64a2d988)
├── tikv-jemalloc-ctl v0.5.4 (https://github.com/risingwavelabs/jemallocator.git?rev=64a2d9#64a2d988)
│   └── risingwave_compute v1.1.0-alpha (/home/mrcroxx/github/risingwavelabs/risingwave/src/compute) (*)
├── tikv-jemallocator v0.5.4 (https://github.com/risingwavelabs/jemallocator.git?rev=64a2d9#64a2d988)
│   ├── risingwave_cmd v1.1.0-alpha (/home/mrcroxx/github/risingwavelabs/risingwave/src/cmd) (*)
│   ├── risingwave_cmd_all v1.1.0-alpha (/home/mrcroxx/github/risingwavelabs/risingwave/src/cmd_all)
│   └── risingwave_simulation v0.1.0 (/home/mrcroxx/github/risingwavelabs/risingwave/src/tests/simulation)
│   [dev-dependencies]
│   └── risingwave_batch v1.1.0-alpha (/home/mrcroxx/github/risingwavelabs/risingwave/src/batch) (*)
└── workspace-hack v1.1.0-alpha (/home/mrcroxx/github/risingwavelabs/risingwave/src/workspace-hack) (*)

And compared with cargo tree -d, the two non-duplicated tikv-jemalloc-sys dependency comes from RisingWave's dependency and build-dependency.

@MrCroxx
Copy link
Contributor Author

MrCroxx commented Sep 14, 2023

#12307

@xxchan
Copy link
Member

xxchan commented Sep 15, 2023

The duplicated versions are with and without feature unprefixed_malloc_on_supported_platforms, which is target.'cfg(unix)'.dependencies.

I guess it's because the build-dependency (introduced by hakari) doesn't contain this feature.

We can see cargo does not unify features for dependencies and build-dependencies

[dependencies]
rand = { version = "0.8.5", default-features = false }

[build-dependencies]
rand = { version = "0.8.5", default-features = false, features = ["serde"] }


❯ cargo tree -p rand -e features -i              
rand v0.8.5
└── tmp v0.1.0 (/private/tmp/tmp)
    └── tmp feature "default" (command-line)

rand v0.8.5
└── rand feature "serde"
    [build-dependencies]
    └── tmp v0.1.0 (/private/tmp/tmp) (*)

@xxchan
Copy link
Member

xxchan commented Sep 15, 2023

If we add target in hakari config

# "x86_64-unknown-linux-gnu",

It will add new section

[target.x86_64-unknown-linux-gnu.dependencies]
#...
tikv-jemalloc-sys = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9", default-features = false, features = ["unprefixed_malloc_on_supported_platforms"] }
tikv-jemallocator = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms"] }
#...

[target.x86_64-unknown-linux-gnu.build-dependencies]
#...
tikv-jemalloc-sys = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9", default-features = false, features = ["unprefixed_malloc_on_supported_platforms"] }
tikv-jemallocator = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms"] }
#...

@xxchan
Copy link
Member

xxchan commented Sep 15, 2023

Alternatively, this also exclude it from hakari

diff --git a/.config/hakari.toml b/.config/hakari.toml
index e998f5fea..9cdfbb3b0 100644
--- a/.config/hakari.toml
+++ b/.config/hakari.toml
@@ -35,6 +35,4 @@ third-party = [
     { name = "criterion" },
     { name = "console" },
     { name = "similar" },
-    # FYI: https://github.com/risingwavelabs/risingwave/issues/12315
-    { name = "tikv-jemalloc-sys", git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9" },
 ]
diff --git a/src/compute/Cargo.toml b/src/compute/Cargo.toml
index 24d12151c..6e3f8674a 100644
--- a/src/compute/Cargo.toml
+++ b/src/compute/Cargo.toml
@@ -39,7 +39,6 @@ risingwave_storage = { workspace = true }
 risingwave_stream = { workspace = true }
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
-tikv-jemalloc-ctl = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9" }
 tokio = { version = "0.2", package = "madsim-tokio", features = [
     "rt",
     "rt-multi-thread",
@@ -54,6 +53,10 @@ tonic = { workspace = true }
 tower = { version = "0.4", features = ["util", "load-shed"] }
 tracing = "0.1"
 
+[target.'cfg(unix)'.dev-dependencies]
+tikv-jemalloc-ctl = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9" }
+
+
 [target.'cfg(not(madsim))'.dependencies]
 workspace-hack = { path = "../workspace-hack" }
 
diff --git a/src/tests/simulation/Cargo.toml b/src/tests/simulation/Cargo.toml
index 82992b8b0..01c20b172 100644
--- a/src/tests/simulation/Cargo.toml
+++ b/src/tests/simulation/Cargo.toml
@@ -46,11 +46,14 @@ serde_derive = "1.0.188"
 serde_json = "1.0.107"
 sqllogictest = "0.15.3"
 tempfile = "3"
-tikv-jemallocator = { workspace = true }
 tokio = { version = "0.2.23", package = "madsim-tokio" }
 tokio-postgres = "0.7"
 tracing = "0.1"
 tracing-subscriber = { version = "0.3", features = ["env-filter"] }
 
+[target.'cfg(unix)'.dev-dependencies]
+tikv-jemallocator = { workspace = true }
+
+
 [lints]
 workspace = true

@xxchan
Copy link
Member

xxchan commented Sep 15, 2023

By default, cargo hakari produces the minimal set of features that can be unified across all possible platforms. However, in practice, most developers on a codebase use one of a few platforms. cargo hakari can run specific queries for a few platforms, producing better results for them.

@MrCroxx
Copy link
Contributor Author

MrCroxx commented Sep 15, 2023

@xxchan Seems we can finally close this issue via #12333 ?

@MrCroxx
Copy link
Contributor Author

MrCroxx commented Sep 15, 2023

[target.x86_64-unknown-linux-gnu.dependencies]
axum = { version = "0.6" }
memchr = { version = "2" }
mime_guess = { version = "2" }
miniz_oxide = { version = "0.7", default-features = false, features = ["with-alloc"] }
once_cell = { version = "1", features = ["unstable"] }
openssl-sys = { version = "0.9", default-features = false, features = ["vendored"] }
opentelemetry = { version = "0.20", default-features = false, features = ["metrics", "rt-tokio", "trace"] }
opentelemetry_sdk = { version = "0.20", default-features = false, features = ["rt-tokio"] }
rdkafka-sys = { git = "https://github.com/MaterializeInc/rust-rdkafka", rev = "8ea07c4", default-features = false, features = ["cmake-build", "gssapi", "libz", "ssl-vendored", "zstd"] }
rustix = { version = "0.38", features = ["fs", "termios"] }
rustls = { version = "0.21", features = ["dangerous_configuration"] }
serde_json = { version = "1", default-features = false, features = ["raw_value"] }
tikv-jemalloc-sys = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms"] }
tikv-jemallocator = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms"] }
tower-http = { version = "0.4", features = ["add-extension", "cors", "fs"] }
zstd-sys = { version = "2", features = ["std"] }

[target.x86_64-unknown-linux-gnu.build-dependencies]
axum = { version = "0.6" }
memchr = { version = "2" }
mime_guess = { version = "2" }
miniz_oxide = { version = "0.7", default-features = false, features = ["with-alloc"] }
once_cell = { version = "1", features = ["unstable"] }
openssl-sys = { version = "0.9", default-features = false, features = ["vendored"] }
opentelemetry = { version = "0.20", default-features = false, features = ["metrics", "rt-tokio", "trace"] }
opentelemetry_sdk = { version = "0.20", default-features = false, features = ["rt-tokio"] }
rdkafka-sys = { git = "https://github.com/MaterializeInc/rust-rdkafka", rev = "8ea07c4", default-features = false, features = ["cmake-build", "gssapi", "libz", "ssl-vendored", "zstd"] }
rustix = { version = "0.38", features = ["fs", "termios"] }
rustls = { version = "0.21", features = ["dangerous_configuration"] }
serde_json = { version = "1", default-features = false, features = ["raw_value"] }
tikv-jemalloc-sys = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms"] }
tikv-jemallocator = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms"] }
tower-http = { version = "0.4", features = ["add-extension", "cors", "fs"] }
zstd-sys = { version = "2", features = ["std"] }

@xxchan tikv-jemalloc-sys duplicated again. 🥹

@MrCroxx
Copy link
Contributor Author

MrCroxx commented Sep 15, 2023

And this time both tikv-jemallocator and tikv-jemalloc-sys are listed. 😭

@xxchan
Copy link
Member

xxchan commented Sep 15, 2023

❯ cargo tree -d  | rg tikv   

There are no duplicates

@MrCroxx
Copy link
Contributor Author

MrCroxx commented Sep 15, 2023

❯ cargo tree -d  | rg tikv   

There are no duplicates

Wired. But it did build 2 duplicated targets. https://buildkite.com/risingwavelabs/docker/builds/14287

@xxchan
Copy link
Member

xxchan commented Sep 17, 2023

I found debug build doesn't have duplicates, but release build has. Too weird..

@xxchan
Copy link
Member

xxchan commented Sep 17, 2023

I found one copy is release build, while another is debug build 🤯

image

@xxchan
Copy link
Member

xxchan commented Sep 17, 2023

I think this is because (dependency's) build-dependency is built with different config.

@xxchan
Copy link
Member

xxchan commented Sep 17, 2023

https://doc.rust-lang.org/cargo/reference/profiles.html#build-dependencies

To compile quickly, all profiles, by default, do not optimize build dependencies

@xxchan
Copy link
Member

xxchan commented Sep 17, 2023

guppy-rs/guppy#83

xxchan added a commit that referenced this issue Sep 17, 2023
Before: `cargo build --release`:
Building [========>               ] ../1339
After:
Building [========>               ] ../1065

Background #12315 (comment)
@fuyufjh
Copy link
Member

fuyufjh commented Oct 5, 2023

This doesn't seem to be caused by [build-dependencies]

In my cases (#12625), tikv-jemalloc-sys was produced under [dependencies] but it still compiles both for release and debug, exactly as #12315 (comment) shows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants