flux-sched icon indicating copy to clipboard operation
flux-sched copied to clipboard

Rv1 `attributes` section will be deprecated

Open grondo opened this issue 1 year ago • 2 comments

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.

grondo avatar Nov 08 '23 15:11 grondo

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.

garlick avatar Nov 08 '23 17:11 garlick

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;

grondo avatar Nov 08 '23 20:11 grondo