From 3e9892f084a5c28e5e36a0b86d19f0551260538a Mon Sep 17 00:00:00 2001 From: Christopher Moussa Date: Thu, 8 Jul 2021 15:07:23 -0700 Subject: [PATCH 1/5] create_db: add "default_bank" field --- src/bindings/python/fluxacct/accounting/create_db.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bindings/python/fluxacct/accounting/create_db.py b/src/bindings/python/fluxacct/accounting/create_db.py index c3bcdc6c..97aa6e00 100755 --- a/src/bindings/python/fluxacct/accounting/create_db.py +++ b/src/bindings/python/fluxacct/accounting/create_db.py @@ -94,6 +94,7 @@ def create_db( userid int(11) DEFAULT 65534 NOT NULL, admin_level smallint(6) DEFAULT 1 NOT NULL, bank tinytext NOT NULL, + default_bank tinytext NOT NULL, shares int(11) DEFAULT 1 NOT NULL, job_usage real DEFAULT 0.0 NOT NULL, fairshare real DEFAULT 0.5 NOT NULL, From 842f5321e594f571266e003dc4bb58db32cd4460 Mon Sep 17 00:00:00 2001 From: Christopher Moussa Date: Thu, 8 Jul 2021 15:07:48 -0700 Subject: [PATCH 2/5] add_user: add default bank support --- .../fluxacct/accounting/user_subcommands.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/bindings/python/fluxacct/accounting/user_subcommands.py b/src/bindings/python/fluxacct/accounting/user_subcommands.py index 47d7535b..f2289f0b 100755 --- a/src/bindings/python/fluxacct/accounting/user_subcommands.py +++ b/src/bindings/python/fluxacct/accounting/user_subcommands.py @@ -58,6 +58,18 @@ def add_user( except KeyError as key_error: print(key_error) + # check for a default bank of the user being added; if the user is new, set + # the first bank they were added to as their default bank + cur = conn.cursor() + select_stmt = "SELECT default_bank FROM association_table WHERE username=?" + cur.execute(select_stmt, (username,)) + row = cur.fetchone() + + if row is None: + default_bank = bank + else: + default_bank = row[0] + try: # insert the user values into association_table conn.execute( @@ -70,9 +82,10 @@ def add_user( userid, admin_level, bank, + default_bank, shares ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) """, ( int(time.time()), @@ -82,6 +95,7 @@ def add_user( uid, admin_level, bank, + default_bank, shares, ), ) @@ -129,6 +143,7 @@ def edit_user(conn, username, field, new_value): "username", "admin_level", "bank", + "default_bank", "shares", "max_jobs", "max_wall_pj", From ffe8c1b2a7d6e12bd2e7aafffe4d80b1d8c4f103 Mon Sep 17 00:00:00 2001 From: Christopher Moussa Date: Thu, 8 Jul 2021 15:08:05 -0700 Subject: [PATCH 3/5] test: add tests for default bank support --- .../accounting/test/test_create_db.py | 4 +- .../accounting/test/test_user_subcommands.py | 40 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/bindings/python/fluxacct/accounting/test/test_create_db.py b/src/bindings/python/fluxacct/accounting/test/test_create_db.py index 11dbee37..600c6af1 100755 --- a/src/bindings/python/fluxacct/accounting/test/test_create_db.py +++ b/src/bindings/python/fluxacct/accounting/test/test_create_db.py @@ -56,9 +56,9 @@ def test_02_create_association(self): """ INSERT INTO association_table (creation_time, mod_time, deleted, username, userid, admin_level, - bank, shares) + bank, default_bank, shares) VALUES - (0, 0, 0, "test user", 1234, 1, "test account", 0) + (0, 0, 0, "test user", 1234, 1, "test account", "test_account", 0) """ ) cursor = conn.cursor() diff --git a/src/bindings/python/fluxacct/accounting/test/test_user_subcommands.py b/src/bindings/python/fluxacct/accounting/test/test_user_subcommands.py index f34f6b9e..d8205198 100755 --- a/src/bindings/python/fluxacct/accounting/test/test_user_subcommands.py +++ b/src/bindings/python/fluxacct/accounting/test/test_user_subcommands.py @@ -127,6 +127,46 @@ def test_06_delete_user(self): self.assertEqual(len(num_rows_after_delete), 0) + # check for a new user's default bank + def test_07_check_default_bank_new_user(self): + u.add_user( + acct_conn, + username="test_user1", + uid="5000", + bank="test_bank", + ) + cursor = acct_conn.cursor() + cursor.execute( + "SELECT default_bank FROM association_table WHERE username='test_user1'" + ) + + self.assertEqual(cursor.fetchone()[0], "test_bank") + + # check for an existing user's default bank + def test_08_check_default_bank_existing_user(self): + u.add_user( + acct_conn, + username="test_user1", + uid="5000", + bank="other_test_bank", + ) + cursor = acct_conn.cursor() + cursor.execute( + "SELECT default_bank FROM association_table WHERE username='test_user1'" + ) + + self.assertEqual(cursor.fetchone()[0], "test_bank") + + # check that we can successfully edit the default bank for a user + def test_09_edit_default_bank(self): + u.edit_user(acct_conn, "test_user1", "default_bank", "other_test_bank") + cursor = acct_conn.cursor() + cursor.execute( + "SELECT default_bank FROM association_table WHERE username='test_user1'" + ) + + self.assertEqual(cursor.fetchone()[0], "other_test_bank") + # remove database and log file @classmethod def tearDownClass(self): From a8c82ac8f4da97b681b6c223088f081b0c458855 Mon Sep 17 00:00:00 2001 From: Christopher Moussa Date: Mon, 12 Jul 2021 11:56:47 -0700 Subject: [PATCH 4/5] plugin: change default bank behavior Now that the flux-accounting DB keeps track of a user's default bank via a column in the association_table, change the behavior of unpacking this data to look at the default column. If the bank that the user belongs to matches the default column, add the user's fairshare data to the "default" key in the map data structure. Otherwise, create a new entry with the new bank and fairshare value. Add a "default_bank" key-value pair to the payload data that gets sent to the plugin by the bulk_update Python script. --- src/plugins/bulk_update.py | 11 +++++++++-- src/plugins/mf_priority.cpp | 9 ++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/plugins/bulk_update.py b/src/plugins/bulk_update.py index 1ebe86b3..4728a6e5 100755 --- a/src/plugins/bulk_update.py +++ b/src/plugins/bulk_update.py @@ -47,9 +47,16 @@ def bulk_update(path): cur = conn.cursor() # fetch all rows from association_table (will print out tuples) - for row in cur.execute("SELECT userid, bank, fairshare FROM association_table"): + for row in cur.execute( + "SELECT userid, bank, default_bank, fairshare FROM association_table" + ): # create a JSON payload with the results of the query - data = {"userid": str(row[0]), "bank": str(row[1]), "fairshare": str(row[2])} + data = { + "userid": str(row[0]), + "bank": str(row[1]), + "default_bank": str(row[2]), + "fairshare": str(row[3]), + } flux.Flux().rpc("job-manager.mf_priority.rec_update", data).get() diff --git a/src/plugins/mf_priority.cpp b/src/plugins/mf_priority.cpp index 4ecc5525..cb91203a 100644 --- a/src/plugins/mf_priority.cpp +++ b/src/plugins/mf_priority.cpp @@ -96,11 +96,12 @@ static void rec_update_cb (flux_t *h, const flux_msg_t *msg, void *arg) { - char *uid, *fshare, *bank; + char *uid, *fshare, *bank, *default_bank; - if (flux_request_unpack (msg, NULL, "{s:s, s:s, s:s}", + if (flux_request_unpack (msg, NULL, "{s:s, s:s, s:s, s:s}", "userid", &uid, "bank", &bank, + "default_bank", &default_bank, "fairshare", &fshare) < 0) { flux_log_error (h, "failed to unpack custom_priority.trigger msg"); goto error; @@ -109,9 +110,7 @@ static void rec_update_cb (flux_t *h, if (flux_respond (h, msg, NULL) < 0) flux_log_error (h, "flux_respond"); - // if the user being added to the does not yet have any entries in the map, - // treat their first bank as the "default" bank - if (users.count (std::atoi (uid)) == 0) + if (strcmp (bank, default_bank) == 0) users[std::atoi (uid)]["default"] = std::stod (fshare); users[std::atoi (uid)][bank] = std::stod (fshare); From 848b4c60a382b2bc3ae7cec9d9ef117ca44a1258 Mon Sep 17 00:00:00 2001 From: Christopher Moussa Date: Mon, 12 Jul 2021 11:59:30 -0700 Subject: [PATCH 5/5] t: add "default_bank" field in payload data --- t/t1001-mf-priority-basic.t | 4 ++-- t/t1002-mf-priority-small-no-tie.t | 14 +++++++------- t/t1003-mf-priority-small-tie.t | 16 ++++++++-------- t/t1004-mf-priority-small-tie-all.t | 18 +++++++++--------- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/t/t1001-mf-priority-basic.t b/t/t1001-mf-priority-basic.t index 939432da..4daac391 100755 --- a/t/t1001-mf-priority-basic.t +++ b/t/t1001-mf-priority-basic.t @@ -45,9 +45,9 @@ test_expect_success 'create fake_payload.py' ' username = getpass.getuser() userid = pwd.getpwnam(username).pw_uid # create a JSON payload - data = {"userid": str(userid), "bank": "account3", "fairshare": "0.45321"} + data = {"userid": str(userid), "bank": "account3", "default_bank": "account3", "fairshare": "0.45321"} flux.Flux().rpc("job-manager.mf_priority.rec_update", data).get() - data = {"userid": str(userid), "bank": "account2", "fairshare": "0.11345"} + data = {"userid": str(userid), "bank": "account2", "default_bank": "account3", "fairshare": "0.11345"} flux.Flux().rpc("job-manager.mf_priority.rec_update", data).get() EOF ' diff --git a/t/t1002-mf-priority-small-no-tie.t b/t/t1002-mf-priority-small-no-tie.t index 9945c364..22470ceb 100755 --- a/t/t1002-mf-priority-small-no-tie.t +++ b/t/t1002-mf-priority-small-no-tie.t @@ -25,13 +25,13 @@ test_expect_success 'create a group of users with unique fairshare values' ' cat <<-EOF >fake_small_no_tie.json { "users" : [ - {"userid": "5011", "bank": "account1", "fairshare": "0.285714"}, - {"userid": "5012", "bank": "account1", "fairshare": "0.142857"}, - {"userid": "5013", "bank": "account1", "fairshare": "0.428571"}, - {"userid": "5021", "bank": "account2", "fairshare": "0.714286"}, - {"userid": "5022", "bank": "account2", "fairshare": "0.571429"}, - {"userid": "5031", "bank": "account3", "fairshare": "1.0"}, - {"userid": "5032", "bank": "account3", "fairshare": "0.857143"} + {"userid": "5011", "bank": "account1", "default_bank": "account1", "fairshare": "0.285714"}, + {"userid": "5012", "bank": "account1", "default_bank": "account1", "fairshare": "0.142857"}, + {"userid": "5013", "bank": "account1", "default_bank": "account1", "fairshare": "0.428571"}, + {"userid": "5021", "bank": "account2", "default_bank": "account2", "fairshare": "0.714286"}, + {"userid": "5022", "bank": "account2", "default_bank": "account2", "fairshare": "0.571429"}, + {"userid": "5031", "bank": "account3", "default_bank": "account3", "fairshare": "1.0"}, + {"userid": "5032", "bank": "account3", "default_bank": "account3", "fairshare": "0.857143"} ] } EOF diff --git a/t/t1003-mf-priority-small-tie.t b/t/t1003-mf-priority-small-tie.t index a38d6948..4ef3eb0e 100755 --- a/t/t1003-mf-priority-small-tie.t +++ b/t/t1003-mf-priority-small-tie.t @@ -25,14 +25,14 @@ test_expect_success 'create a group of users with some ties in fairshare values' cat <<-EOF >fake_small_tie.json { "users" : [ - {"userid": "5011", "bank": "account1", "fairshare": "0.5"}, - {"userid": "5012", "bank": "account1", "fairshare": "0.5"}, - {"userid": "5013", "bank": "account1", "fairshare": "0.75"}, - {"userid": "5021", "bank": "account2", "fairshare": "0.5"}, - {"userid": "5022", "bank": "account2", "fairshare": "0.5"}, - {"userid": "5023", "bank": "account2", "fairshare": "0.75"}, - {"userid": "5031", "bank": "account3", "fairshare": "1.0"}, - {"userid": "5032", "bank": "account3", "fairshare": "0.875"} + {"userid": "5011", "bank": "account1", "default_bank": "account1", "fairshare": "0.5"}, + {"userid": "5012", "bank": "account1", "default_bank": "account1", "fairshare": "0.5"}, + {"userid": "5013", "bank": "account1", "default_bank": "account1", "fairshare": "0.75"}, + {"userid": "5021", "bank": "account2", "default_bank": "account2", "fairshare": "0.5"}, + {"userid": "5022", "bank": "account2", "default_bank": "account2", "fairshare": "0.5"}, + {"userid": "5023", "bank": "account2", "default_bank": "account2", "fairshare": "0.75"}, + {"userid": "5031", "bank": "account3", "default_bank": "account3", "fairshare": "1.0"}, + {"userid": "5032", "bank": "account3", "default_bank": "account3", "fairshare": "0.875"} ] } EOF diff --git a/t/t1004-mf-priority-small-tie-all.t b/t/t1004-mf-priority-small-tie-all.t index df58e9e5..32cc1b90 100755 --- a/t/t1004-mf-priority-small-tie-all.t +++ b/t/t1004-mf-priority-small-tie-all.t @@ -25,15 +25,15 @@ test_expect_success 'create a group of users with many ties in fairshare values' cat <<-EOF >fake_small_tie_all.json { "users" : [ - {"userid": "5011", "bank": "account1", "fairshare": "0.666667"}, - {"userid": "5012", "bank": "account1", "fairshare": "0.666667"}, - {"userid": "5013", "bank": "account1", "fairshare": "1"}, - {"userid": "5021", "bank": "account2", "fairshare": "0.666667"}, - {"userid": "5022", "bank": "account2", "fairshare": "0.666667"}, - {"userid": "5023", "bank": "account2", "fairshare": "1"}, - {"userid": "5031", "bank": "account3", "fairshare": "0.666667"}, - {"userid": "5032", "bank": "account3", "fairshare": "0.666667"}, - {"userid": "5033", "bank": "account3", "fairshare": "1"} + {"userid": "5011", "bank": "account1", "default_bank": "account1", "fairshare": "0.666667"}, + {"userid": "5012", "bank": "account1", "default_bank": "account1", "fairshare": "0.666667"}, + {"userid": "5013", "bank": "account1", "default_bank": "account1", "fairshare": "1"}, + {"userid": "5021", "bank": "account2", "default_bank": "account2", "fairshare": "0.666667"}, + {"userid": "5022", "bank": "account2", "default_bank": "account2", "fairshare": "0.666667"}, + {"userid": "5023", "bank": "account2", "default_bank": "account2", "fairshare": "1"}, + {"userid": "5031", "bank": "account3", "default_bank": "account3", "fairshare": "0.666667"}, + {"userid": "5032", "bank": "account3", "default_bank": "account3", "fairshare": "0.666667"}, + {"userid": "5033", "bank": "account3", "default_bank": "account3", "fairshare": "1"} ] } EOF