Skip to content

Conversation

@spoutn1k
Copy link
Contributor

@spoutn1k spoutn1k commented Nov 18, 2025

Fixes EHT-1348.

The historical snapshots were made to assert the validity of the data while switching backends.

Now that we stabilized the library, they outlived their usefulness. We had to implement quite dirty hacks to compare them to new data, and going foward we will have to rename some metrics, invalidating these.

I remove all of this in this PR. Going forward we should validate the output from version to version and not relating to some obsolete version.

@spoutn1k spoutn1k requested review from breuhan and johnsonw November 18, 2025 10:37
@spoutn1k spoutn1k requested a review from jgrund as a code owner November 18, 2025 10:37
@notion-workspace
Copy link

@codecov
Copy link

codecov bot commented Nov 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.92%. Comparing base (07b820a) to head (17fe7f9).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #122      +/-   ##
==========================================
- Coverage   93.93%   91.92%   -2.01%     
==========================================
  Files          44       44              
  Lines        5573     5377     -196     
  Branches     5573     5377     -196     
==========================================
- Hits         5235     4943     -292     
- Misses        269      360      +91     
- Partials       69       74       +5     
Flag Coverage Δ
2_14_0_ddn133 34.74% <ø> (ø)
2_14_0_ddn145 35.72% <ø> (ø)
all-tests 91.92% <100.00%> (-2.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@spoutn1k spoutn1k force-pushed the spoutn1k/EHT-1348-history-in-the-making branch from 25fe838 to 6bede68 Compare November 18, 2025 10:40
@github-actions
Copy link

github-actions bot commented Nov 18, 2025

🐰 Bencher Report

Branchspoutn1k/EHT-1348-history-in-the-making
Testbedci-runner
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
nanoseconds (ns)
(Result Δ%)
Lower Boundary
nanoseconds (ns)
(Limit %)
Upper Boundary
nanoseconds (ns)
(Limit %)
parse_benchmarks/combine_performance📈 view plot
🚷 view threshold
129,540,000.00 ns
(-52.98%)Baseline: 275,483,750.00 ns
-707,499,705.20 ns
(-546.16%)
1,258,467,205.20 ns
(10.29%)
🐰 View full continuous benchmarking report in Bencher

@github-actions
Copy link

github-actions bot commented Nov 18, 2025

🐰 Bencher Report

Branchspoutn1k/EHT-1348-history-in-the-making
Testbedci-runner

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results
BenchmarkD1 Miss Ratemisses (%)D1mrmisses (reads) x 1e3D1mwmisses (writes) x 1e3DLmrmisses (reads)DLmwmisses (writes) x 1e3Drreads x 1e6Dwwrites x 1e6Estimated Cyclescycles x 1e6I1 Miss Ratemisses (%)I1mrmisses (reads) x 1e3ILmrmisses (reads)InstructionsBenchmark Result
instructions x 1e6
(Result Δ%)
Lower Boundary
instructions x 1e6
(Limit %)
Upper Boundary
instructions x 1e6
(Limit %)
L1 Hit Ratehits (%)L1 Hitshits x 1e6LL Hit Ratehits (%)LL Hitshits x 1e3LL Miss Ratemisses (%)LLd Miss Ratemisses (%)LLi Miss Ratemisses (%)RAM Hit Ratehits (%)RAM Hitshits x 1e3Total read+writereads/writes x 1e6
lustre_metrics::memory_benches::bench_encode_lustre_metrics with_setup:generate_records()📈 view plot
⚠️ NO THRESHOLD
0.93 %📈 view plot
⚠️ NO THRESHOLD
25.27 reads x 1e3📈 view plot
⚠️ NO THRESHOLD
9.22 writes x 1e3📈 view plot
⚠️ NO THRESHOLD
113.00 reads📈 view plot
⚠️ NO THRESHOLD
6.45 writes x 1e3📈 view plot
⚠️ NO THRESHOLD
2.47 x 1e6📈 view plot
⚠️ NO THRESHOLD
1.22 x 1e6📈 view plot
⚠️ NO THRESHOLD
14.80 x 1e6📈 view plot
⚠️ NO THRESHOLD
0.01 %📈 view plot
⚠️ NO THRESHOLD
1.03 reads x 1e3📈 view plot
⚠️ NO THRESHOLD
879.00 reads📈 view plot
🚷 view threshold
10.74 x 1e6
(-21.60%)Baseline: 13.70 x 1e6
2.60 x 1e6
(24.19%)
24.80 x 1e6
(43.31%)
📈 view plot
⚠️ NO THRESHOLD
99.75 %📈 view plot
⚠️ NO THRESHOLD
14.40 x 1e6📈 view plot
⚠️ NO THRESHOLD
0.19 %📈 view plot
⚠️ NO THRESHOLD
28.09 x 1e3📈 view plot
⚠️ NO THRESHOLD
0.05 %📈 view plot
⚠️ NO THRESHOLD
0.18 %📈 view plot
⚠️ NO THRESHOLD
0.01 %📈 view plot
⚠️ NO THRESHOLD
0.05 %📈 view plot
⚠️ NO THRESHOLD
7.44 x 1e3📈 view plot
⚠️ NO THRESHOLD
14.43 x 1e6
🐰 View full continuous benchmarking report in Bencher

@github-actions
Copy link

github-actions bot commented Nov 18, 2025

🐰 Bencher Report

Branchspoutn1k/EHT-1348-history-in-the-making
Testbedci-runner

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results
Benchmarkavg_runtime_rss_mibMeasure (MiB)avg_runtime_virtual_mibMeasure (MiB)end_rss_mibMeasure (MiB)end_virtual_mibMeasure (MiB)memory_growth_mibMeasure (MiB)peak_over_start_rss_ratioMeasure (units)peak_over_start_virtual_ratioMeasure (units)peak_rss_mibBenchmark Result
Measure (MiB)
(Result Δ%)
Lower Boundary
Measure (MiB)
(Limit %)
Upper Boundary
Measure (MiB)
(Limit %)
peak_virtual_mibMeasure (MiB)start_rss_mibMeasure (MiB)start_virtual_mibMeasure (MiB)virtual_growth_mibMeasure (MiB)
scrape_allocations📈 view plot
⚠️ NO THRESHOLD
43.58 MiB📈 view plot
⚠️ NO THRESHOLD
942.03 MiB📈 view plot
⚠️ NO THRESHOLD
43.66 MiB📈 view plot
⚠️ NO THRESHOLD
942.15 MiB📈 view plot
⚠️ NO THRESHOLD
0.34 MiB📈 view plot
⚠️ NO THRESHOLD
1.03 units📈 view plot
⚠️ NO THRESHOLD
1.02 units📈 view plot
🚷 view threshold
45.73 MiB
(-44.10%)Baseline: 81.81 MiB
-134.76 MiB
(-294.69%)
298.38 MiB
(15.33%)
📈 view plot
⚠️ NO THRESHOLD
944.83 MiB📈 view plot
⚠️ NO THRESHOLD
43.32 MiB📈 view plot
⚠️ NO THRESHOLD
936.20 MiB📈 view plot
⚠️ NO THRESHOLD
5.95 MiB
🐰 View full continuous benchmarking report in Bencher

jparris
jparris previously approved these changes Dec 1, 2025
@gaurangtapase
Copy link
Contributor

Do we have all snapshots updated with prometheus client lib?

@spoutn1k
Copy link
Contributor Author

spoutn1k commented Dec 2, 2025

@gaurangtapase all the snapshots are using the new prometheus-client crate. They were then processed to be compared to the old versions. Now what that means is that whenever we update, add, remove fields, then we need to update the processing to compare to those old versions.

@gaurangtapase
Copy link
Contributor

Changes look ok considering that we have seen consistency in the data with the introduction of the new library and we now can fully rely on that going forward. Just to make sure we are not missing anything, please check the code coverage drop.

@spoutn1k
Copy link
Contributor Author

spoutn1k commented Dec 3, 2025

Seems like the additions (eg stats) were not covered by commandeer, so I updated the 'snapshot' for them to be taken into account.

We have some pretty broken snapshots on main.

"stdout": "",
"stderr": "cat: /usr/local/bin/../../lustre-collector/src/fixtures/valid/lustre-2.14.0_ddn133/2.14.0_ddn133_quota.txt: No such file or directory\n",
"exit_code": 1
}
],
"lnetctl:stats show": [
{
"binary_name": "lnetctl",
"args": [
"stats",
"show"
],
"stdout": "",
"stderr": "cat: /usr/local/bin/../fixtures/lnetctl_stats.txt: No such file or directory\n",
"exit_code": 1
}
],
"lnetctl:net show -v 4": [
{
"binary_name": "lnetctl",
"args": [
"net",
"show",
"-v",
"4"
],
"stdout": "",
"stderr": "cat: /usr/local/bin/../fixtures/lnetctl_net_show.txt: No such file or directory\n",
"exit_code": 1
}

We need to fix those before merging anything.

@spoutn1k spoutn1k force-pushed the spoutn1k/EHT-1348-history-in-the-making branch from c5ed41f to a789529 Compare December 3, 2025 10:44
@spoutn1k spoutn1k force-pushed the spoutn1k/EHT-1348-history-in-the-making branch from a789529 to dbc3217 Compare December 3, 2025 10:48
@spoutn1k spoutn1k force-pushed the spoutn1k/EHT-1348-history-in-the-making branch from 0b9164c to 17fe7f9 Compare December 5, 2025 12:55
@spoutn1k spoutn1k requested a review from jparris December 5, 2025 13:05
Comment on lines -83 to -85
"stdout": "",
"stderr": "cat: /usr/local/bin/../fixtures/lnetctl_stats.txt: No such file or directory\n",
"exit_code": 1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed all of those

@spoutn1k
Copy link
Contributor Author

spoutn1k commented Dec 5, 2025

@breuhan @gaurangtapase @jparris please give it a glance, the coverage drops but seemingly only due to the sheer amount of lines removed as codecov cannot pinpoint a change.

@@ -215,4 +215,4 @@
}
]
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to add them back :)
All of them :)

@jparris
Copy link
Contributor

jparris commented Feb 2, 2026

@spoutn1k please rebase.

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.

6 participants