Merge 6ca6866 into 77733b2

src/plugins/mf_priority.cpp

@@ -605,10 +605,9 @@ static int priority_cb (flux_plugin_t *p,
                 return -1;
 
             // if we get here, the bank was unknown when this job was first
-            // accepted, and therefore the active and run job counts for this
+            // accepted, and therefore the active job counts for this
             // job need to be incremented here
             bank_it->second.cur_active_jobs++;
-            bank_it->second.cur_run_jobs++;
 
             // update current job with user/bank information
             if (flux_jobtap_job_aux_set (p,
@@ -634,8 +633,6 @@ static int priority_cb (flux_plugin_t *p,
         return -1;
     }
 
-    b->cur_run_jobs++;
-
     return 0;
 }
 
@@ -917,6 +914,34 @@ static int depend_cb (flux_plugin_t *p,
 }
 
 
+static int run_cb (flux_plugin_t *p,
+                   const char *topic,
+                   flux_plugin_arg_t *args,
+                   void *data)
+{
+    int userid;
+    struct bank_info *b;
+
+    b = static_cast<bank_info *>
+        (flux_jobtap_job_aux_get (p,
+                                  FLUX_JOBTAP_CURRENT_JOB,
+                                  "mf_priority:bank_info"));
+
+    if (b == NULL) {
+        flux_jobtap_raise_exception (p, FLUX_JOBTAP_CURRENT_JOB, "mf_priority",
+                                     0, "job.state.run: bank info is " \
+                                     "missing");
+
+        return -1;
+    }
+
+    // increment the user's current running jobs count
+    b->cur_run_jobs++;
+
+    return 0;
+}
+
+
 static int inactive_cb (flux_plugin_t *p,
                         const char *topic,
                         flux_plugin_arg_t *args,
@@ -952,8 +977,14 @@ static int inactive_cb (flux_plugin_t *p,
         return -1;
     }
 
-    b->cur_run_jobs--;
     b->cur_active_jobs--;
+    // nothing more to do if this job was never running
+    if (!flux_jobtap_job_event_posted (p, FLUX_JOBTAP_CURRENT_JOB, "alloc"))
+        return 0;
+
+    // this job was running, so decrement the current running jobs count
+    // and look to see if any held jobs can be released
+    b->cur_run_jobs--;
 
     // if the user/bank combo has any currently held jobs and the user is now
     // under their max jobs limit, remove the dependency from first held job
@@ -980,6 +1011,7 @@ static const struct flux_plugin_handler tab[] = {
     { "job.priority.get", priority_cb, NULL },
     { "job.state.inactive", inactive_cb, NULL },
     { "job.state.depend", depend_cb, NULL },
+    { "job.state.run", run_cb, NULL},
     { "plugin.query", query_cb, NULL},
     { 0 },
 };

t/Makefile.am

@@ -20,7 +20,8 @@ TESTSCRIPTS = \
 	t1016-export-db.t \
 	t1017-update-db.t \
 	t1018-mf-priority-disable-entry.t \
-	t1019-mf-priority-info-fetch.t
+	t1019-mf-priority-info-fetch.t \
+	t1020-mf-priority-issue262.t
 
 dist_check_SCRIPTS = \
 	$(TESTSCRIPTS) \

t/t1020-mf-priority-issue262.t

@@ -0,0 +1,110 @@
+#!/bin/bash
+
+test_description='Test comparing job counts when submitting jobs that take up all resources'
+
+. `dirname $0`/sharness.sh
+MULTI_FACTOR_PRIORITY=${FLUX_BUILD_DIR}/src/plugins/.libs/mf_priority.so
+SUBMIT_AS=${SHARNESS_TEST_SRCDIR}/scripts/submit_as.py
+DB_PATH=$(pwd)/FluxAccountingTest.db
+EXPECTED_FILES=${SHARNESS_TEST_SRCDIR}/expected/plugin_state
+
+export TEST_UNDER_FLUX_NO_JOB_EXEC=y
+export TEST_UNDER_FLUX_SCHED_SIMPLE_MODE="limited=1"
+test_under_flux 1 job
+
+flux setattr log-stderr-level 1
+
+test_expect_success 'create flux-accounting DB' '
+    flux account -p $(pwd)/FluxAccountingTest.db create-db
+'
+
+test_expect_success 'start flux-accounting service' '
+    flux account-service -p ${DB_PATH} -t
+'
+
+test_expect_success 'load multi-factor priority plugin' '
+    flux jobtap load -r .priority-default ${MULTI_FACTOR_PRIORITY}
+'
+
+test_expect_success 'check that mf_priority plugin is loaded' '
+    flux jobtap list | grep mf_priority
+'
+
+test_expect_success 'add some banks to the DB' '
+    flux account add-bank root 1 &&
+    flux account add-bank --parent-bank=root account1 1 &&
+    flux account add-bank --parent-bank=root account2 1 &&
+    flux account add-bank --parent-bank=root account3 1
+'
+
+test_expect_success 'add a user with two different banks to the DB' '
+    flux account add-user --username=user1001 --userid=1001 --bank=account1 --max-running-jobs=5 --max-active-jobs=10 &&
+    flux account add-user --username=user1001 --userid=1001 --bank=account2
+'
+
+test_expect_success 'send flux-accounting DB information to the plugin' '
+    flux account-priority-update -p $(pwd)/FluxAccountingTest.db
+'
+
+test_expect_success 'submit a sleep 180 job and ensure it is running' '
+    jobid1=$(flux python ${SUBMIT_AS} 1001 sleep 180) &&
+    test $(flux jobs -no {state} ${jobid1}) = RUN
+'
+
+test_expect_success 'stop scheduler from allocating resources to jobs' '
+    flux queue stop
+'
+
+test_expect_success 'submit 2 more sleep 180 jobs; ensure both are in SCHED state' '
+    jobid2=$(flux python ${SUBMIT_AS} 1001 sleep 180) &&
+    jobid3=$(flux python ${SUBMIT_AS} 1001 sleep 180) &&
+    test $(flux jobs -no {state} ${jobid2}) = SCHED &&
+    test $(flux jobs -no {state} ${jobid3}) = SCHED
+'
+
+test_expect_success 'ensure current running and active jobs are correct: 1 running, 3 active' '
+    flux jobtap query mf_priority.so > query_1.json &&
+    test_debug "jq -S . <query_1.json" &&
+    jq -e ".mf_priority_map[0].banks[0].cur_run_jobs == 1" <query_1.json &&
+    jq -e ".mf_priority_map[0].banks[0].cur_active_jobs == 3" <query_1.json
+'
+
+test_expect_success 'update the plugin and ensure current running and active jobs are correct' '
+    flux account-priority-update -p $(pwd)/FluxAccountingTest.db &&
+    flux jobtap query mf_priority.so > query_2.json &&
+    test_debug "jq -S . <query_2.json" &&
+    jq -e ".mf_priority_map[0].banks[0].cur_run_jobs == 1" <query_2.json &&
+    jq -e ".mf_priority_map[0].banks[0].cur_active_jobs == 3" <query_2.json
+'
+
+test_expect_success 'change the priority of one of the jobs' '
+    flux job urgency $jobid2 31 &&
+    flux account-priority-update -p $(pwd)/FluxAccountingTest.db &&
+    flux job eventlog $jobid2 | grep ^priority | tail -n 1 | priority=4294967295
+'
+
+test_expect_success 'ensure job counts are still the same: 1 running, 3 active' '
+    flux jobtap query mf_priority.so > query_3.json &&
+    test_debug "jq -S . <query_3.json" &&
+    jq -e ".mf_priority_map[0].banks[0].cur_run_jobs == 1" <query_3.json &&
+    jq -e ".mf_priority_map[0].banks[0].cur_active_jobs == 3" <query_3.json
+'
+
+test_expect_success 'cancel one of the scheduled jobs, check job counts are correct: 1 running, 2 active' '
+    flux job cancel $jobid2 &&
+    flux jobtap query mf_priority.so > query_4.json &&
+    test_debug "jq -S . <query_4.json" &&
+    jq -e ".mf_priority_map[0].banks[0].cur_run_jobs == 1" <query_4.json &&
+    jq -e ".mf_priority_map[0].banks[0].cur_active_jobs == 2" <query_4.json
+'
+
+test_expect_success 'cancel sleep 180 job(s), check job counts: 0 running, 0 active' '
+    flux job cancel $jobid1 &&
+    flux job cancel $jobid3 &&
+    flux jobtap query mf_priority.so > query_5.json &&
+    test_debug "jq -S . <query_5.json" &&
+    jq -e ".mf_priority_map[0].banks[0].cur_run_jobs == 0" <query_5.json &&
+    jq -e ".mf_priority_map[0].banks[0].cur_active_jobs == 0" <query_5.json
+'
+
+test_done