flux-framework / flux-sched

Fluxion Graph-based Scheduler
GNU Lesser General Public License v3.0
89 stars 41 forks source link

Don't prune traversal by leaf vertex subplans #1314

Closed milroy closed 2 weeks ago

milroy commented 2 weeks ago

Issue #1260 and logs from production clusters have reported match errors such as by_subplan: planner_multi_avail_during returned -1. for JGFs with node vertices with an edge to the cluster vertex. The error occurs because PR #1248 prevents creation of subplan filters on leaf vertices, causing subsequent checks against the planner to fail.

This PR adds a check to by_subplan to bypass pruning based on null subplans.

Currently, the test case succeeds without the fix because the error isn't logged in flux dmesg for some reason.

milroy commented 2 weeks ago

After many attempts I was unsuccessful at getting the error to propagate to flux dmesg. I updated the testsuite test to use resource-query instead. Let me know if I should keep poking at the resource module-based test.

I think the PR is ready for review.

milroy commented 2 weeks ago

I updated the testsuite test to use resource-query instead.

(The updated test based on resource-query fails without the change in this PR and succeeds with it.)

milroy commented 2 weeks ago

Thanks for the reviews. Setting MWP.

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.2%. Comparing base (1c1f8d6) to head (ee57490). Report is 4 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1314 +/- ## ======================================== - Coverage 75.2% 75.2% -0.1% ======================================== Files 111 111 Lines 15983 15986 +3 ======================================== - Hits 12032 12028 -4 - Misses 3951 3958 +7 ``` | [Files with missing lines](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1314?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework) | Coverage Δ | | |---|---|---| | [resource/traversers/dfu\_impl.cpp](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1314?src=pr&el=tree&filepath=resource%2Ftraversers%2Fdfu_impl.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-cmVzb3VyY2UvdHJhdmVyc2Vycy9kZnVfaW1wbC5jcHA=) | `83.6% <100.0%> (-0.4%)` | :arrow_down: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/flux-framework/flux-sched/pull/1314/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework)