modules: Move feasibility/satisfiability checking into a new module #1285

jacobtkeio · 2024-08-30T21:32:41Z

Problem: Feasibility checking takes up a significant amount of sched-fluxion-resource's time that could be better spent on scheduling.
Solution: Move feasibility checking into a new module, sched-fluxion-feasibility, that can run on multiple ranks.

Problem: feasibility.check RPCs take up too much of sched-fluxion-resource's single-threaded time. Add sched-fluxion-feasibility module that can run on multiple ranks to handle feasibility.check RPCs.

Problem: All calls to the feasibility.check RPC are in s-f-resource tests, not s-f-feasibility tests. Create a feasibility module test file, t4014, and update other feasibility.check tests to query the feasibility module.

Problem: t4014 does not test the behavior of resource -> feasibility notification after the feasibility module restarts. This is a special case because resource needs to send first-time information after the restart, including all resources, lost resources, and resource graph expiration. Create t4015 to test feasibility notification.

Problem: PR flux-framework#1352 changed populate_resource_db to support resource shrinking, but it left in an old version of the function. Remove it.

codecov · 2025-07-17T21:17:47Z

Codecov Report

Attention: Patch coverage is 75.67128% with 299 lines in your changes missing coverage. Please review.

Project coverage is 76.5%. Comparing base (fc7621f) to head (cece306).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
resource/modules/resource.cpp	74.4%	219 Missing ⚠️
resource/modules/feasibility.cpp	74.6%	56 Missing ⚠️
resource/modules/resource_match.cpp	83.3%	24 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #1285     +/-   ##
========================================
+ Coverage    76.4%   76.5%   +0.1%     
========================================
  Files         112     115      +3     
  Lines       16700   16929    +229     
========================================
+ Hits        12765   12962    +197     
- Misses       3935    3967     +32

Files with missing lines	Coverage Δ
qmanager/modules/qmanager.cpp	`74.3% <ø> (+3.9%)`	⬆️
resource/modules/resource_match.hpp	`100.0% <100.0%> (ø)`
resource/modules/resource_match.cpp	`67.9% <83.3%> (-3.1%)`	⬇️
resource/modules/feasibility.cpp	`74.6% <74.6%> (ø)`
resource/modules/resource.cpp	`74.4% <74.4%> (ø)`

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

milroy · 2025-07-22T06:45:37Z

resource/modules/feasibility.cpp

+/*****************************************************************************\
+ * Copyright 2014 Lawrence Livermore National Security, LLC
+ * (c.f. AUTHORS, NOTICE.LLNS, LICENSE)


Update the copyright year:

Suggested change

/*****************************************************************************\

* Copyright 2014 Lawrence Livermore National Security, LLC

* (c.f. AUTHORS, NOTICE.LLNS, LICENSE)

/*****************************************************************************\

* Copyright 2025 Lawrence Livermore National Security, LLC

* (c.f. AUTHORS, NOTICE.LLNS, LICENSE)

milroy · 2025-07-22T06:47:51Z

resource/modules/resource.cpp

@@ -0,0 +1,1548 @@
+/*****************************************************************************\
+ * Copyright 2014 Lawrence Livermore National Security, LLC


Suggested change

* Copyright 2014 Lawrence Livermore National Security, LLC

* Copyright 2025 Lawrence Livermore National Security, LLC

milroy

This is really good work. I have a few nits, and a minor point for discussion regarding the RPC name.

Apart from that, I'd like to discuss changing the satisfiability pruning filter to be ALL:core,ALL:node and perhaps add ALL:gpu as well. Adding a node type to the pruning filter by default should make the is_satisfiable function in the traverser faster and more powerful.

milroy · 2025-07-22T06:54:33Z

src/cmd/flux-ion-resource.py

@@ -113,7 +113,7 @@ def rpc_namespace_info(self, rank, type_name, identity):

    def rpc_satisfiability(self, jobspec):
        payload = {"jobspec": jobspec}
-        return self.handle.rpc("sched-fluxion-resource.satisfiability", payload).get()
+        return self.handle.rpc("feasibility.check", payload).get()


Let's revisit this RPC name. Adhering to the existing pattern yields sched-fluxion-feasibility.satisfiability which is clunky.

milroy · 2025-07-22T06:55:50Z

resource/modules/resource_match.hpp

@@ -0,0 +1,192 @@
+/*****************************************************************************\
+ * Copyright 2014 Lawrence Livermore National Security, LLC


Suggested change

* Copyright 2014 Lawrence Livermore National Security, LLC

* Copyright 2025 Lawrence Livermore National Security, LLC

milroy · 2025-07-22T07:17:28Z

t/data/resource/jobspecs/shrink/shrink2.yaml

+version: 9999
+resources:
+   - type: node
+     count: 2
+     with:
+         - type: slot
+           count: 1
+           label: default
+           with:
+             - type: core
+               count: 1
+attributes:
+  system:
+    duration: 300
+tasks:
+  - command: [ "hostlist" ]
+    slot: default
+    count:
+      per_slot: 1
+


Following the current directory structure convention, I think it's better to move shrink2-4 into t/data/resource/jobspecs/satisfiability.

milroy · 2025-07-22T07:21:21Z

t/t4005-match-unsat.t

+prune-filters=ALL:core subsystems=containment policy=high &&
+    load_feasibility load-file=${grug} load-format=grug \


I'm wondering if we should set prune-filters to ALL:core,ALL:node in the tests and in the module.

milroy · 2025-07-22T07:23:05Z

t/CMakeLists.txt

@@ -86,6 +86,7 @@ set(ALL_TESTS
  t4012-set-status.t


The commit message body should wrap at 72 characters.

jacobtkeio force-pushed the master branch from c3d821e to 5c573ad Compare September 29, 2024 00:41

jacobtkeio force-pushed the master branch from 5c573ad to 17a1d65 Compare October 28, 2024 01:24

jacobtkeio force-pushed the master branch from 17a1d65 to d83fe61 Compare November 24, 2024 00:57

jacobtkeio force-pushed the master branch 3 times, most recently from c31f3e7 to c6f6c95 Compare December 2, 2024 20:57

jacobtkeio force-pushed the master branch 5 times, most recently from 93b13a6 to 3d94893 Compare December 20, 2024 21:27

jacobtkeio force-pushed the master branch 7 times, most recently from 9e2814e to bdb3fca Compare January 1, 2025 02:26

jacobtkeio force-pushed the master branch 5 times, most recently from 6308980 to b8f1289 Compare January 8, 2025 08:18

jacobtkeio force-pushed the master branch 7 times, most recently from 9b84f5d to eae90b4 Compare January 19, 2025 09:41

jacobtkeio force-pushed the master branch from 46f7563 to 1bfdaa1 Compare March 1, 2025 06:35

jacobtkeio marked this pull request as ready for review March 1, 2025 08:23

jacobtkeio force-pushed the master branch 3 times, most recently from 6e18b88 to cd8ba09 Compare March 6, 2025 20:10

jacobtkeio force-pushed the master branch from dd5b39f to 01400c2 Compare April 3, 2025 07:31

jacobtkeio force-pushed the master branch 5 times, most recently from 65ca1fc to 4eaef1c Compare May 1, 2025 21:20

jacobtkeio force-pushed the master branch from 4eaef1c to 53e671c Compare May 15, 2025 21:09

jacobtkeio force-pushed the master branch 7 times, most recently from fd25295 to fd31259 Compare May 28, 2025 19:58

jacobtkeio force-pushed the master branch from fd31259 to 8f4a664 Compare June 15, 2025 07:56

Jacob Tkeio and others added 4 commits June 30, 2025 10:58

Split feasibility checking from resource module

77474e2

Problem: feasibility.check RPCs take up too much of sched-fluxion-resource's single-threaded time. Add sched-fluxion-feasibility module that can run on multiple ranks to handle feasibility.check RPCs.

Add feasibility module test

cc3b3db

Problem: All calls to the feasibility.check RPC are in s-f-resource tests, not s-f-feasibility tests. Create a feasibility module test file, t4014, and update other feasibility.check tests to query the feasibility module.

Remove vestigial populate_resource_db in resource_match

cece306

Problem: PR flux-framework#1352 changed populate_resource_db to support resource shrinking, but it left in an old version of the function. Remove it.

jacobtkeio force-pushed the master branch from 8f4a664 to cece306 Compare June 30, 2025 17:59

milroy reviewed Jul 22, 2025

View reviewed changes

milroy requested a review from trws July 22, 2025 07:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

modules: Move feasibility/satisfiability checking into a new module #1285

modules: Move feasibility/satisfiability checking into a new module #1285

Uh oh!

jacobtkeio commented Aug 30, 2024 •

edited

Loading

Uh oh!

codecov bot commented Jul 17, 2025

Uh oh!

milroy Jul 22, 2025

Uh oh!

milroy Jul 22, 2025

Uh oh!

milroy left a comment

Uh oh!

milroy Jul 22, 2025

Uh oh!

milroy Jul 22, 2025

Uh oh!

milroy Jul 22, 2025

Uh oh!

milroy Jul 22, 2025

Uh oh!

milroy Jul 22, 2025

Uh oh!

Uh oh!

		@@ -0,0 +1,1548 @@
		/*****************************************************************************\
		* Copyright 2014 Lawrence Livermore National Security, LLC

	* Copyright 2014 Lawrence Livermore National Security, LLC
	* Copyright 2025 Lawrence Livermore National Security, LLC

		@@ -0,0 +1,192 @@
		/*****************************************************************************\
		* Copyright 2014 Lawrence Livermore National Security, LLC

		prune-filters=ALL:core subsystems=containment policy=high &&
		load_feasibility load-file=${grug} load-format=grug \

modules: Move feasibility/satisfiability checking into a new module #1285

Are you sure you want to change the base?

modules: Move feasibility/satisfiability checking into a new module #1285

Uh oh!

Conversation

jacobtkeio commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 17, 2025

Codecov Report

Uh oh!

milroy Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

milroy Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

milroy left a comment

Choose a reason for hiding this comment

Uh oh!

milroy Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

milroy Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

milroy Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

milroy Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

milroy Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jacobtkeio commented Aug 30, 2024 •

edited

Loading