Fix parsing recovery status for multiple targets #124
Conversation
Signed-off-by: Gaurang Tapase <gtapase@ddn.com>
|
Bencher Report| Branch | gtapase/fix-recovery-multiple-targets |
| Testbed | ci-runner |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result nanoseconds (ns) (Result Δ%) | Lower Boundary nanoseconds (ns) (Limit %) | Upper Boundary nanoseconds (ns) (Limit %) |
|---|---|---|---|---|
| parse_benchmarks/combine_performance | 📈 view plot 🚷 view threshold | 128,950,000.00 ns(-54.38%)Baseline: 282,656,000.00 ns | -736,237,513.01 ns (-570.95%) | 1,301,549,513.01 ns (9.91%) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
+ Coverage 94.10% 94.40% +0.29%
==========================================
Files 44 44
Lines 5669 5681 +12
Branches 5669 5681 +12
==========================================
+ Hits 5335 5363 +28
+ Misses 265 245 -20
- Partials 69 73 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
| Branch | gtapase/fix-recovery-multiple-targets |
| Testbed | ci-runner |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
- Total read+write (reads/writes)
- DLmr (misses (reads))
- L1 Hits (hits)
- Estimated Cycles (cycles)
- D1mr (misses (reads))
- D1mw (misses (writes))
- DLmw (misses (writes))
- Dr (reads)
- D1 Miss Rate (misses (%))
- Dw (writes)
- I1mr (misses (reads))
- ILmr (misses (reads))
- L1 Hit Rate (hits (%))
- LL Miss Rate (misses (%))
- LL Hit Rate (hits (%))
- LLd Miss Rate (misses (%))
- LLi Miss Rate (misses (%))
- RAM Hit Rate (hits (%))
- RAM Hits (hits)
- LL Hits (hits)
- I1 Miss Rate (misses (%))
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-thresholdsflag.
Click to view all benchmark results
| Benchmark | D1 Miss Rate | misses (%) | D1mr | misses (reads) x 1e3 | D1mw | misses (writes) x 1e3 | DLmr | misses (reads) | DLmw | misses (writes) x 1e3 | Dr | reads x 1e6 | Dw | writes x 1e6 | Estimated Cycles | cycles x 1e6 | I1 Miss Rate | misses (%) | I1mr | misses (reads) x 1e3 | ILmr | misses (reads) | Instructions | Benchmark Result instructions x 1e6 (Result Δ%) | Lower Boundary instructions x 1e6 (Limit %) | Upper Boundary instructions x 1e6 (Limit %) | L1 Hit Rate | hits (%) | L1 Hits | hits x 1e6 | LL Hit Rate | hits (%) | LL Hits | hits x 1e3 | LL Miss Rate | misses (%) | LLd Miss Rate | misses (%) | LLi Miss Rate | misses (%) | RAM Hit Rate | hits (%) | RAM Hits | hits x 1e3 | Total read+write | reads/writes x 1e6 |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| lustre_metrics::memory_benches::bench_encode_lustre_metrics with_setup:generate_records() | 📈 view plot | 0.92 % | 📈 view plot | 25.04 reads x 1e3 | 📈 view plot | 9.09 writes x 1e3 | 📈 view plot | 115.00 reads | 📈 view plot | 6.45 writes x 1e3 | 📈 view plot | 2.47 x 1e6 | 📈 view plot | 1.23 x 1e6 | 📈 view plot | 14.81 x 1e6 | 📈 view plot | 0.01 % | 📈 view plot | 1.04 reads x 1e3 | 📈 view plot | 894.00 reads | 📈 view plot 🚷 view threshold | 10.75 x 1e6(-23.14%)Baseline: 13.99 x 1e6 | 2.43 x 1e6 (22.65%) | 25.54 x 1e6 (42.10%) | 📈 view plot | 99.76 % | 📈 view plot | 14.41 x 1e6 | 📈 view plot | 0.19 % | 📈 view plot | 27.72 x 1e3 | 📈 view plot | 0.05 % | 📈 view plot | 0.18 % | 📈 view plot | 0.01 % | 📈 view plot | 0.05 % | 📈 view plot | 7.45 x 1e3 | 📈 view plot | 14.44 x 1e6 |
breuhan
left a comment
breuhan
left a comment
There was a problem hiding this comment.
Could you add a demo or brief description?
There was a problem hiding this comment.
Pull request overview
This pull request fixes the recovery status parser to correctly handle multiple targets (OSTs and MDTs) in a single output. Previously, the parser only handled a single target; now it uses the many1 combinator to parse one or more targets sequentially.
Key Changes
- Modified the recovery status parser to use
many1combinator for parsing multiple targets - Updated the
check_outputfunction signature to acceptparamsas a parameter instead of computing it internally - Added a new integration test
test_parse_recovery_status_outputto verify multiple target parsing
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lustre-collector/src/recovery_status_parser.rs | Wrapped the parser in many1 to handle multiple targets and added flatten operation to combine results |
| lustre-collector/src/lib.rs | Refactored check_output to accept params as argument and added test for multiple target recovery status parsing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Done |
Signed-off-by: Gaurang Tapase <gtapase@ddn.com>
johnsonw
left a comment
johnsonw
left a comment
There was a problem hiding this comment.
I'm happy with this approach. Using the top level parser to wrap the component parsers feels like the correct approach here.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…it tests Signed-off-by: Gaurang Tapase <gtapase@ddn.com>
This pull request fixes the the parsing output of recovery status of multiple targets by using top level parser for all of the function calls.