diff --git a/src/bindings/python/fluxacct/accounting/__init__.py.in b/src/bindings/python/fluxacct/accounting/__init__.py.in index 9c5745fc2..56775c92f 100644 --- a/src/bindings/python/fluxacct/accounting/__init__.py.in +++ b/src/bindings/python/fluxacct/accounting/__init__.py.in @@ -1,6 +1,6 @@ DB_DIR = "@X_LOCALSTATEDIR@/lib/flux/" DB_PATH = "@X_LOCALSTATEDIR@/lib/flux/FluxAccounting.db" -DB_SCHEMA_VERSION = 24 +DB_SCHEMA_VERSION = 25 # flux-accounting DB table column names ASSOCIATION_TABLE = [ @@ -21,7 +21,7 @@ ASSOCIATION_TABLE = [ "projects", "default_project", ] -BANK_TABLE = ["bank_id", "bank", "active", "parent_bank", "shares", "job_usage"] +BANK_TABLE = ["bank_id", "bank", "active", "parent_bank", "shares", "job_usage", "max_preempt_after"] QUEUE_TABLE = [ "queue", "min_nodes_per_job", diff --git a/src/bindings/python/fluxacct/accounting/bank_subcommands.py b/src/bindings/python/fluxacct/accounting/bank_subcommands.py index 6781867b2..c4f41e324 100644 --- a/src/bindings/python/fluxacct/accounting/bank_subcommands.py +++ b/src/bindings/python/fluxacct/accounting/bank_subcommands.py @@ -16,6 +16,8 @@ from fluxacct.accounting import formatter as fmt from fluxacct.accounting import sql_util as sql +from flux.util import parse_fsd + ############################################################### # # # Helper Functions # @@ -218,7 +220,7 @@ def print_parsable_hierarchy(cur, bank, hierarchy_str, indent=""): ############################################################### -def add_bank(conn, bank, shares, parent_bank=""): +def add_bank(conn, bank, shares, parent_bank="", max_preempt_after=None): cur = conn.cursor() if parent_bank == "": @@ -248,19 +250,24 @@ def add_bank(conn, bank, shares, parent_bank=""): reactivate_bank(conn, cur, bank, parent_bank) return 0 + if max_preempt_after: + # parse Flux Standard Duration (see RFC 23) and add it to the INSERT statement + duration = parse_fsd(max_preempt_after) + insert_stmt = ( + "INSERT INTO bank_table " + "(bank, parent_bank, shares, max_preempt_after) " + "VALUES (?, ?, ?, ?)" + ) + stmt_parameters = (bank, parent_bank, shares, duration) + else: + insert_stmt = ( + "INSERT INTO bank_table (bank, parent_bank, shares) VALUES (?, ?, ?)" + ) + stmt_parameters = (bank, parent_bank, shares) + # insert the bank values into the database try: - conn.execute( - """ - INSERT INTO bank_table ( - bank, - parent_bank, - shares - ) - VALUES (?, ?, ?) - """, - (bank, parent_bank, shares), - ) + conn.execute(insert_stmt, stmt_parameters) # commit changes conn.commit() @@ -384,12 +391,14 @@ def edit_bank( bank=None, shares=None, parent_bank=None, + max_preempt_after=None, ): cur = conn.cursor() params = locals() editable_fields = [ "shares", "parent_bank", + "max_preempt_after", ] for field in editable_fields: if params[field] is not None: @@ -405,6 +414,14 @@ def edit_bank( if field == "shares": if int(shares) <= 0: raise ValueError("new shares amount must be >= 0") + if field == "max_preempt_after": + if max_preempt_after == str(-1): + duration = None + else: + # convert FSD to seconds + duration = parse_fsd(max_preempt_after) + params[field] = duration + print(f"params[field]: {params[field]}") update_stmt = "UPDATE bank_table SET " + field diff --git a/src/bindings/python/fluxacct/accounting/create_db.py b/src/bindings/python/fluxacct/accounting/create_db.py index 77a8d4820..28187cbf1 100755 --- a/src/bindings/python/fluxacct/accounting/create_db.py +++ b/src/bindings/python/fluxacct/accounting/create_db.py @@ -119,12 +119,13 @@ def create_db( conn.execute( """ CREATE TABLE IF NOT EXISTS bank_table ( - bank_id integer PRIMARY KEY AUTOINCREMENT, - bank text NOT NULL, - active int(11) DEFAULT 1 NOT NULL, - parent_bank text DEFAULT '', - shares int NOT NULL, - job_usage real DEFAULT 0.0 NOT NULL + bank_id integer PRIMARY KEY AUTOINCREMENT, + bank text NOT NULL, + active int(11) DEFAULT 1 NOT NULL, + parent_bank text DEFAULT '', + shares int NOT NULL, + job_usage real DEFAULT 0.0 NOT NULL, + max_preempt_after real );""" ) logging.info("Created bank_table successfully") diff --git a/src/cmd/flux-account-service.py b/src/cmd/flux-account-service.py index 3c9abae50..3a25992c3 100755 --- a/src/cmd/flux-account-service.py +++ b/src/cmd/flux-account-service.py @@ -271,6 +271,7 @@ def add_bank(self, handle, watcher, msg, arg): msg.payload["bank"], msg.payload["shares"], msg.payload["parent_bank"], + msg.payload["max_preempt_after"], ) payload = {"add_bank": val} @@ -312,6 +313,7 @@ def edit_bank(self, handle, watcher, msg, arg): msg.payload["bank"], msg.payload["shares"], msg.payload["parent_bank"], + msg.payload["max_preempt_after"], ) payload = {"edit_bank": val} diff --git a/src/cmd/flux-account.py b/src/cmd/flux-account.py index 2127b3d6c..6152f5ff8 100755 --- a/src/cmd/flux-account.py +++ b/src/cmd/flux-account.py @@ -282,6 +282,15 @@ def add_add_bank_arg(subparsers): subparser_add_bank.add_argument( "shares", help="number of shares to allocate to bank", metavar="SHARES" ) + subparser_add_bank.add_argument( + "--max-preempt-after", + help=( + "max number of time until a job running under this bank can become " + "preemptible; duration must be in Flux Standard Duration" + ), + type=str, + metavar="MAX_PREEMPT_AFTER", + ) def add_view_bank_arg(subparsers): @@ -356,6 +365,15 @@ def add_edit_bank_arg(subparsers): help="parent bank", metavar="PARENT BANK", ) + subparser_edit_bank.add_argument( + "--max-preempt-after", + help=( + "max number of time until a job running under this bank can become " + "preemptible; duration must be in Flux Standard Duration" + ), + type=str, + metavar="MAX_PREEMPT_AFTER", + ) def add_list_banks_arg(subparsers): diff --git a/t/Makefile.am b/t/Makefile.am index 2a50f3163..6e9cdbf67 100644 --- a/t/Makefile.am +++ b/t/Makefile.am @@ -43,6 +43,7 @@ TESTSCRIPTS = \ t1041-view-jobs-by-project.t \ t1042-issue508.t \ t1043-view-jobs-by-bank.t \ + t1044-issue553.t \ t5000-valgrind.t \ python/t1000-example.py \ python/t1001_db.py \ diff --git a/t/expected/flux_account/A_bank.expected b/t/expected/flux_account/A_bank.expected index f35c6ffec..2da31cdfe 100644 --- a/t/expected/flux_account/A_bank.expected +++ b/t/expected/flux_account/A_bank.expected @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -2 A 1 root 1 0.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +2 A 1 root 1 0.0 None Users Under Bank A: diff --git a/t/expected/flux_account/D_bank.expected b/t/expected/flux_account/D_bank.expected index f83560751..6dc435ca2 100644 --- a/t/expected/flux_account/D_bank.expected +++ b/t/expected/flux_account/D_bank.expected @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -5 D 1 root 1 0.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +5 D 1 root 1 0.0 None Bank Username RawShares RawUsage Fairshare D 1 0.0 diff --git a/t/expected/flux_account/E_bank.expected b/t/expected/flux_account/E_bank.expected index e7f29cc9a..c1478cf98 100644 --- a/t/expected/flux_account/E_bank.expected +++ b/t/expected/flux_account/E_bank.expected @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -6 E 1 D 1 0.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +6 E 1 D 1 0.0 None Bank Username RawShares RawUsage Fairshare E 1 0.0 diff --git a/t/expected/flux_account/F_bank_tree.expected b/t/expected/flux_account/F_bank_tree.expected index b6cd50528..ba67e1fa5 100644 --- a/t/expected/flux_account/F_bank_tree.expected +++ b/t/expected/flux_account/F_bank_tree.expected @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -7 F 1 D 1 0.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +7 F 1 D 1 0.0 None Bank Username RawShares RawUsage Fairshare F 1 0.0 diff --git a/t/expected/flux_account/F_bank_users.expected b/t/expected/flux_account/F_bank_users.expected index 73ea41096..01163a8ec 100644 --- a/t/expected/flux_account/F_bank_users.expected +++ b/t/expected/flux_account/F_bank_users.expected @@ -1,4 +1,4 @@ -bank_id bank active parent_bank shares job_usage -7 F 1 D 1 0.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +7 F 1 D 1 0.0 None no users under F diff --git a/t/expected/flux_account/deleted_bank.expected b/t/expected/flux_account/deleted_bank.expected index 41f193259..6176bb1c7 100644 --- a/t/expected/flux_account/deleted_bank.expected +++ b/t/expected/flux_account/deleted_bank.expected @@ -1,3 +1,3 @@ -bank_id bank active parent_bank shares -4 C 0 root 50 +bank_id bank active parent_bank shares job_usage max_preempt_after +4 C 0 root 50 0.0 None diff --git a/t/expected/flux_account/full_hierarchy.expected b/t/expected/flux_account/full_hierarchy.expected index 2c1e5d74e..aae7e002a 100644 --- a/t/expected/flux_account/full_hierarchy.expected +++ b/t/expected/flux_account/full_hierarchy.expected @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -1 root 1 1 0.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +1 root 1 1 0.0 None Bank Username RawShares RawUsage Fairshare root 1 0.0 diff --git a/t/expected/flux_account/root_bank.expected b/t/expected/flux_account/root_bank.expected index cb42503f2..5f063dc0f 100644 --- a/t/expected/flux_account/root_bank.expected +++ b/t/expected/flux_account/root_bank.expected @@ -1,3 +1,3 @@ -bank_id bank active parent_bank shares job_usage -1 root 1 1 0.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +1 root 1 1 0.0 None diff --git a/t/expected/pop_db/db_hierarchy_base.expected b/t/expected/pop_db/db_hierarchy_base.expected index c3cfb42d5..66d125d72 100644 --- a/t/expected/pop_db/db_hierarchy_base.expected +++ b/t/expected/pop_db/db_hierarchy_base.expected @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -1 root 1 1 0.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +1 root 1 1 0.0 None Bank Username RawShares RawUsage Fairshare root 1 0.0 diff --git a/t/expected/pop_db/db_hierarchy_new_users.expected b/t/expected/pop_db/db_hierarchy_new_users.expected index 3a4c0e68a..5a25c2dfb 100644 --- a/t/expected/pop_db/db_hierarchy_new_users.expected +++ b/t/expected/pop_db/db_hierarchy_new_users.expected @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -1 root 1 1 0.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +1 root 1 1 0.0 None Bank Username RawShares RawUsage Fairshare root 1 0.0 diff --git a/t/expected/print_hierarchy/small_no_tie.txt b/t/expected/print_hierarchy/small_no_tie.txt index f7c6165b0..e152cdb76 100644 --- a/t/expected/print_hierarchy/small_no_tie.txt +++ b/t/expected/print_hierarchy/small_no_tie.txt @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -1 root 1 1000 133.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +1 root 1 1000 133.0 None Bank Username RawShares RawUsage Fairshare root 1000 133.0 diff --git a/t/expected/print_hierarchy/small_tie.txt b/t/expected/print_hierarchy/small_tie.txt index 382c3a213..3c6aa3302 100644 --- a/t/expected/print_hierarchy/small_tie.txt +++ b/t/expected/print_hierarchy/small_tie.txt @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -1 root 1 1000 133.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +1 root 1 1000 133.0 None Bank Username RawShares RawUsage Fairshare root 1000 133.0 diff --git a/t/expected/print_hierarchy/small_tie_all.txt b/t/expected/print_hierarchy/small_tie_all.txt index 5d40fd21b..de0c4799a 100644 --- a/t/expected/print_hierarchy/small_tie_all.txt +++ b/t/expected/print_hierarchy/small_tie_all.txt @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -1 root 1 1000 1332.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +1 root 1 1000 1332.0 None Bank Username RawShares RawUsage Fairshare root 1000 1332.0 diff --git a/t/expected/update_fshare/post_fshare_update.expected b/t/expected/update_fshare/post_fshare_update.expected index 51479e108..84f9d47c7 100644 --- a/t/expected/update_fshare/post_fshare_update.expected +++ b/t/expected/update_fshare/post_fshare_update.expected @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -1 root 1 1000 180.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +1 root 1 1000 180.0 None Bank Username RawShares RawUsage Fairshare root 1000 180.0 diff --git a/t/expected/update_fshare/pre_fshare_update.expected b/t/expected/update_fshare/pre_fshare_update.expected index f7c6165b0..e152cdb76 100644 --- a/t/expected/update_fshare/pre_fshare_update.expected +++ b/t/expected/update_fshare/pre_fshare_update.expected @@ -1,5 +1,5 @@ -bank_id bank active parent_bank shares job_usage -1 root 1 1000 133.0 +bank_id bank active parent_bank shares job_usage max_preempt_after +1 root 1 1000 133.0 None Bank Username RawShares RawUsage Fairshare root 1000 133.0 diff --git a/t/t1044-issue553.t b/t/t1044-issue553.t new file mode 100755 index 000000000..c226f7d2f --- /dev/null +++ b/t/t1044-issue553.t @@ -0,0 +1,73 @@ +#!/bin/bash + +test_description='test configuring max-preempt-after values for banks' + +. `dirname $0`/sharness.sh +DB_PATH=$(pwd)/FluxAccountingTest.db + +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 ${DB_PATH} create-db +' + +test_expect_success 'start flux-accounting service' ' + flux account-service -p ${DB_PATH} -t +' + +test_expect_success 'add root bank to the DB' ' + flux account add-bank root 1 +' + +test_expect_success 'add a sub bank with max-preempt in seconds' ' + flux account add-bank --parent-bank=root --max-preempt-after=60s A 1 && + flux account view-bank A > A.test && + grep "60.0" A.test +' + +test_expect_success 'add a sub bank with max-preempt in hours' ' + flux account add-bank --parent-bank=root --max-preempt-after=1h B 1 && + flux account view-bank B > B.test && + grep "3600.0" B.test +' + +test_expect_success 'add a sub bank with max-preempt in days' ' + flux account add-bank --parent-bank=root --max-preempt-after=1d C 1 && + flux account view-bank C > C.test && + grep "86400.0" C.test +' + +test_expect_success 'add a sub bank with no max-preempt specified' ' + flux account add-bank --parent-bank=root D 1 && + flux account view-bank D > D.test && + grep "None" D.test +' + +test_expect_success 'trying to add a sub bank with a bad FSD will raise an error' ' + test_must_fail flux account add-bank \ + --parent-bank=root \ + --max-preempt-after=foo E 1 > error.out 2>&1 && + grep "error in add_bank: invalid Flux standard duration" error.out +' + +test_expect_success 'edit max-preempt-after for a bank' ' + flux account edit-bank A --max-preempt-after=200s && + flux account view-bank A > A_edited.test && + grep "200.0" A_edited.test +' + +test_expect_success 'clear max-preempt-after for a bank' ' + flux account edit-bank A --max-preempt-after=-1 && + flux account view-bank A > A_cleared.test && + grep "None" A_cleared.test +' + +test_expect_success 'shut down flux-accounting service' ' + flux python -c "import flux; flux.Flux().rpc(\"accounting.shutdown_service\").get()" +' + +test_done