flux-framework / flux-sched

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

traverser: fix default job duration when no duration specified #1104

Closed grondo closed 6 months ago

grondo commented 7 months ago

When a resource graph has a fixed duration (i.e. a non-zero expiration), Fluxion currently assigns the graph duration to job requests that do not specify a duration. This results in job expirations that exceed the instance lifetime.

This PR fixes that by adjusting unspecified jobspec durations to the remaining graph duration, instead of the entire graph duration. This results in the expiration of these jobs matching that of the instance.

The existing test that checked this behavior was also updated and fixed.

grondo commented 7 months ago

Thanks @milroy! I've made the suggested changes and pushed the result.

grondo commented 6 months ago

I also noticed that the added test here does not work because flux alloc and flux batch in the flux-sched testsuite do not load Fluxion module by default. (Other tests that use flux batch load fluxion modules manually).

So, I'll be force-pushing this branch with a completely different fix and the testsuite improved.

codecov[bot] commented 6 months ago

Codecov Report

Merging #1104 (641a82a) into master (8da547c) will increase coverage by 0.0%. The diff coverage is 100.0%.

:exclamation: Current head 641a82a differs from pull request most recent head 218f7ec. Consider uploading reports for the commit 218f7ec to get more accurate results

@@          Coverage Diff           @@
##           master   #1104   +/-   ##
======================================
  Coverage    71.8%   71.8%           
======================================
  Files          88      88           
  Lines       11524   11525    +1     
======================================
+ Hits         8279    8280    +1     
  Misses       3245    3245           
Files Coverage Δ
resource/traversers/dfu_impl.hpp 94.7% <100.0%> (+0.1%) :arrow_up:
grondo commented 6 months ago

Thanks! I'll set MWP.