flux-framework / flux-sched

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

Rv1 `attributes` section will be deprecated #1108

Closed grondo closed 1 week ago

grondo commented 5 months ago

flux-framework/rfc#402 proposes to remove the optional attributes section from Rv1. Fluxion currently uses this section to store the queue in attributes.system.scheduler.queue, though it isn't clear why this is needed since this duplicates the queue set in current jobspec.

Additionally, the attributes section does not appear to be set on our production systems with configured queues. Perhaps this is a holdover from before flux-core supported queues and Fluxion had its own queue implementation?

In any event, if stashing the queue in R is still necessary, there is already the opaque scheduling key provided for the scheduler's use which is perhaps better used for this purpose.

garlick commented 5 months ago

I can't think why this would be required.

In the hello handshake where only R is provided, the resources need to be re-allocated immediately so no queueing should be needed. Also, the resources are already selected at that point so the queue constraint would be ignored.

grondo commented 5 months ago

I agree, but I attempted to remove the code that adds the queue "meta" attribute to jobs and this caused t1009-recovery-multiqueue.t to fail. I didn't look into why (all other tests appeared to pass)

diff --git a/resource/traversers/dfu_impl_update.cpp b/resource/traversers/dfu_impl_update.cpp
index 2abb2a22..3c05e325 100644
--- a/resource/traversers/dfu_impl_update.cpp
+++ b/resource/traversers/dfu_impl_update.cpp
@@ -568,12 +568,6 @@ int dfu_impl_t::update (vtx_t root, std::shared_ptr<match_writers_t> &writers,
              m_err_msg += __FUNCTION__;
              m_err_msg += ": emit_tm returned -1.\n";
          }
-         if (jobmeta.is_queue_set ()) {
-             if (writers->emit_attrs ("queue", jobmeta.get_queue ()) == -1) {
-                 m_err_msg += __FUNCTION__;
-                 m_err_msg += ": emit_attrs returned -1.\n";
-             }
-         }
      }

     return (rc > 0)? 0 : -1;
@@ -632,12 +626,6 @@ int dfu_impl_t::update (vtx_t root, std::shared_ptr<match_writers_t> &writers,
              m_err_msg += __FUNCTION__;
              m_err_msg += ": emit_tm returned -1.\n";
          }
-         if (jobmeta.is_queue_set ()) {
-             if (writers->emit_attrs ("queue", jobmeta.get_queue ()) == -1) {
-                 m_err_msg += __FUNCTION__;
-                 m_err_msg += ": emit_attrs returned -1.\n";
-             }
-         }
     }

     return (rc > 0)? 0: -1;