You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "ethantang93 (via GitHub)" <gi...@apache.org> on 2026/08/23 17:22:26 UTC
[PR] enable miri workflow for parquet crate [arrow-rs]
ethantang93 opened a new pull request, #10803:
URL: https://github.com/apache/arrow-rs/pull/10803
# Which issue does this PR close?
closes #614
# Rationale for this change
Follow-up to #10507, which added Miri coverage for the arrow crates but left parquet excluded because it was tracked separately in #614. This adds the parquet crate to the Miri workflow to detect undefined behavior in its unsafe code. the full parquet miri run took about 40mins on my M3 laptop
# What changes are included in this PR?
- Adds `-p parquet` to the Miri test suite in `miri.sh`
- Adds `parquet/**` to the workflow path filters in `miri.yaml`
- adds ignore to tests that take too long to run under Miri
- adds ignore to tests that hit operations Miri doesn't support: kqueue, fork, inline assembly, native Zstd C functions
# Are these changes tested?
i ran the following to test locally.
- `MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri nextest run -p parquet`
- `cargo test -p parquet`
- `cargo fmt --check -p parquet`
- `git diff --check`
# Are there any user-facing changes?
no, only tests and CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#issuecomment-5447057408
@Jefffrey is it okay to bump up the workers to 12? looks like with 12 workers it's back to around 20 mins
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3839125521
##########
parquet/tests/arrow_reader/bad_data.rs:
##########
@@ -107,9 +108,10 @@ fn test_arrow_gh_41317() {
#[test]
fn test_arrow_rs_gh_6229_dict_header() {
let err = read_file("ARROW-RS-GH-6229-DICTHEADER.parquet").unwrap_err();
- assert_eq!(
- err.to_string(),
- "External: Parquet argument error: Parquet error: Integer overflow: out of range integral type conversion attempted"
Review Comment:
this was causing the miri run to fail, the trailing text comes from std's TryFromIntError and varies by toolchain (stable vs. Miri's nightly).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3875427832
##########
parquet/tests/arrow_reader/statistics.rs:
##########
@@ -2896,6 +2940,7 @@ mod test {
}
}
+ #[cfg_attr(miri, ignore)] // tempfile::reopen triggers an unsupported Miri/rustix fstat path
Review Comment:
nice! thanks for filing those
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3875857837
##########
parquet/tests/arrow_reader/io/sync_reader.rs:
##########
@@ -320,6 +328,7 @@ fn test_read_multiple_row_filter() {
}
#[test]
+#[cfg_attr(miri, ignore)] // fork is not supported
Review Comment:
actually i re-ran it and it works now, so dropping these.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3869232868
##########
parquet/tests/arrow_reader/statistics.rs:
##########
@@ -2896,6 +2940,7 @@ mod test {
}
}
+ #[cfg_attr(miri, ignore)] // tempfile::reopen triggers an unsupported Miri/rustix fstat path
Review Comment:
- filed https://github.com/bytecodealliance/rustix/issues/1668
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3869808283
##########
parquet/tests/arrow_reader/statistics.rs:
##########
@@ -2896,6 +2940,7 @@ mod test {
}
}
+ #[cfg_attr(miri, ignore)] // tempfile::reopen triggers an unsupported Miri/rustix fstat path
Review Comment:
for reference it seems it wouldnt work on linux either due to
- https://github.com/rust-lang/miri/issues/5291
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3877413919
##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -870,7 +873,7 @@ mod tests {
(0.1, 1000000, 5772541),
(0.01, 1000000, 9681526),
(0.001, 1000000, 14607697),
- (1e-50, 1_000_000_000_000, 14226231280773240832),
+ (1e-9, 10_000_000, 1026297530),
Review Comment:
@jhorstmann helpfully pointed out theres a miri flag that can help with this
- https://github.com/apache/arrow-rs/pull/10884#discussion_r3870916804
worth a shot to see if that (or the rounding error one i used) might help in this case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#issuecomment-5457056033
@alamb here's the numbers from the last CI run, https://gist.github.com/ethantang93/f77d69c1571f2caea027263240e86e6b
I already removed some tests that takes >20 mins to run. the slowest rn is 280, 220 secs. let me know which ones you think we should remove. also happy to follow up with another pr for removing low value one and keep this one focused on parquet
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#issuecomment-5616451026
I was reviweing github runner usage recently, and I found the MIRI job is now consuming a very large amount of runner credits (basically 16% of all arrow project credits). I filed a ticket to track
- https://github.com/apache/arrow-rs/issues/11045
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3876504391
##########
parquet/tests/arrow_reader/statistics.rs:
##########
@@ -373,6 +373,7 @@ impl Test<'_> {
// Remaining cases
// f64::NAN
+#[cfg_attr(miri, ignore)] // kqueue unsupported by Miri
Review Comment:
turns out these fail on linux too. fstat64, same rust-lang/miri#5291 you filed. kqueue was just masking it on macos. reverted to unconditional ignore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#issuecomment-5447605767
> @Jefffrey is it okay to bump up the workers to 12? looks like with 12 workers it's back to around 20 mins
that sounds good, though cc @alamb since this does increase our actions total compute time quite a bit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#issuecomment-5463006885
thanks @ethantang93 & @alamb
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#issuecomment-5463002763
> @alamb here's the numbers from the last CI run, https://gist.github.com/ethantang93/f77d69c1571f2caea027263240e86e6b I already removed some tests that takes >20 mins to run. the slowest rn is 280, 220 secs. let me know which ones you think we should remove. also happy to follow up with another pr for removing low value one and keep this one focused on parquet
would you be able to raise a followup issue for this? ideally we'll cut any that takes longer than 1 minute (i chose this just based on what nextest reports as its default slow threshold)
we could explore having a lower threshold if we see enough benefits from that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey merged PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3875538215
##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -870,7 +873,7 @@ mod tests {
(0.1, 1000000, 5772541),
(0.01, 1000000, 9681526),
(0.001, 1000000, 14607697),
- (1e-50, 1_000_000_000_000, 14226231280773240832),
+ (1e-9, 10_000_000, 1026297530),
Review Comment:
good idea
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3839147126
##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -870,7 +873,7 @@ mod tests {
(0.1, 1000000, 5772541),
(0.01, 1000000, 9681526),
(0.001, 1000000, 14607697),
- (1e-50, 1_000_000_000_000, 14226231280773240832),
+ (1e-9, 10_000_000, 1026297530),
Review Comment:
had to change up this test case a little, since apparently power and ln does not guarantee bit for bit result, and also miri deliberately add rounding difference, this starts to fail, i changed it a smaller number such that it is stable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3869007704
##########
parquet/tests/arrow_reader/statistics.rs:
##########
@@ -373,6 +373,7 @@ impl Test<'_> {
// Remaining cases
// f64::NAN
+#[cfg_attr(miri, ignore)] // kqueue unsupported by Miri
Review Comment:
is this only a macos issue? for reference
- https://github.com/rust-lang/miri/issues/4007
perhaps we can gate it for macos, since our runners are ubuntu so we can try run these tests as part of CI
##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -870,7 +873,7 @@ mod tests {
(0.1, 1000000, 5772541),
(0.01, 1000000, 9681526),
(0.001, 1000000, 14607697),
- (1e-50, 1_000_000_000_000, 14226231280773240832),
+ (1e-9, 10_000_000, 1026297530),
Review Comment:
maybe we can just gate this specific test case from miri using `#[cfg()]` and leave it unchanged?
##########
parquet/tests/arrow_reader/io/sync_reader.rs:
##########
@@ -320,6 +328,7 @@ fn test_read_multiple_row_filter() {
}
#[test]
+#[cfg_attr(miri, ignore)] // fork is not supported
Review Comment:
similar question here, is this an issue of macos support or just fork support in general?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#issuecomment-5451633271
I think it sounds good to me
One thing I would like to request though is to audit all the MIRI tests (perhaps someone can post the timinings of the tests from nextest) and see if there are any particularly long ones we can remove (I bet there are some fuzz / etc type tests that MIRI doesn't add much value and that take a long time)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3883350592
##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -870,7 +873,7 @@ mod tests {
(0.1, 1000000, 5772541),
(0.01, 1000000, 9681526),
(0.001, 1000000, 14607697),
- (1e-50, 1_000_000_000_000, 14226231280773240832),
+ (1e-9, 10_000_000, 1026297530),
Review Comment:
cool, i tested adding that flag locally and it worked. i will get rid of the ignore here then, and then once #10884 is merged I'll rebase and it should pick up the changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable miri workflow for parquet crate [arrow-rs]
Posted by "ethantang93 (via GitHub)" <gi...@apache.org>.
ethantang93 commented on code in PR #10803:
URL: https://github.com/apache/arrow-rs/pull/10803#discussion_r3875370528
##########
parquet/tests/arrow_reader/statistics.rs:
##########
@@ -373,6 +373,7 @@ impl Test<'_> {
// Remaining cases
// f64::NAN
+#[cfg_attr(miri, ignore)] // kqueue unsupported by Miri
Review Comment:
yeah, that's sounds like a better plan
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org