From eb1eb1d700f7f2f9da4d52b41b623f2260324b7e Mon Sep 17 00:00:00 2001 From: Jorrit Schaap <schaap@astron.nl> Date: Wed, 18 Jul 2018 07:45:39 +0000 Subject: [PATCH] SW-426: fixes for the new tests in t_radb.py. Main changes: a claim cannot have zero duration anymore. a task which goes to conflict releases its claimed claims --- .../ResourceAssignmentDatabase/radb.py | 36 +++++-- .../radb/sql/add_functions_and_triggers.sql | 98 ++++++++----------- 2 files changed, 73 insertions(+), 61 deletions(-) diff --git a/SAS/ResourceAssignment/ResourceAssignmentDatabase/radb.py b/SAS/ResourceAssignment/ResourceAssignmentDatabase/radb.py index a160d325768..3ce28f5dd09 100644 --- a/SAS/ResourceAssignment/ResourceAssignmentDatabase/radb.py +++ b/SAS/ResourceAssignment/ResourceAssignmentDatabase/radb.py @@ -39,6 +39,9 @@ _FETCH_NONE=0 _FETCH_ONE=1 _FETCH_ALL=2 +class RADBError(Exception): + pass + class RADatabase: def __init__(self, dbcreds=None, log_queries=False): self.dbcreds = dbcreds @@ -98,6 +101,9 @@ class RADatabase: logger.error("Rolling back query=\'%s\' due to error: \'%s\'" % (self._queryAsSingleLine(query, qargs), e)) self.rollback() return [] + # TODO: instead of doing a "silent" rollback and continue, we should raise an RADBError. + # We cannot oversee the impact of such a change at this moment, so let's investigate that later. + # raise RADBError(e.message) self._log_database_notifications() @@ -412,7 +418,7 @@ class RADatabase: VALUES (%s, %s, %s, %s, %s) RETURNING id;''' - id = self._executeQuery(query, (mom_id, otdb_id, task_status, task_type, specification_id), fetch=_FETCH_ONE)['id'] + id = self._executeQuery(query, (mom_id, otdb_id, task_status, task_type, specification_id), fetch=_FETCH_ONE).get('id') if commit: self.commit() return id @@ -808,7 +814,15 @@ class RADatabase: claim_status_id = claim_status query = '''SELECT * from resource_allocation.get_current_resource_usage(%s, %s)''' - return self._executeQuery(query, (resource_id, claim_status_id), fetch=_FETCH_ONE) + result = self._executeQuery(query, (resource_id, claim_status_id), fetch=_FETCH_ONE) + + if result is None or result.get('resource_id') is None: + result = { 'resource_id': resource_id, + 'status_id': claim_status_id, + 'as_of_timestamp': datetime.utcnow(), + 'usage': 0 } + + return result def get_resource_usage_at_or_before(self, resource_id, timestamp, claim_status='claimed', exactly_at=False, only_before=False): if isinstance(claim_status, basestring): @@ -817,7 +831,14 @@ class RADatabase: claim_status_id = claim_status query = '''SELECT * from resource_allocation.get_resource_usage_at_or_before(%s, %s, %s, %s, %s, %s)''' - return self._executeQuery(query, (resource_id, claim_status_id, timestamp, exactly_at, only_before, False), fetch=_FETCH_ONE) + result = self._executeQuery(query, (resource_id, claim_status_id, timestamp, exactly_at, only_before, False), fetch=_FETCH_ONE) + + if result is None or result.get('resource_id') is None: + result = { 'resource_id': resource_id, + 'status_id': claim_status_id, + 'as_of_timestamp': timestamp, + 'usage': 0 } + return result def updateResourceAvailability(self, resource_id, active=None, available_capacity=None, total_capacity=None, commit=True): if active is not None: @@ -1260,6 +1281,7 @@ class RADatabase: result = self.insertResourceClaims(task_id, [claim], username, user_id, commit) if result: return result[0] + return None def insertResourceClaims(self, task_id, claims, username, user_id, commit=True): '''bulk insert of a list of resource claims for a task(_id). All claims are inserted with status tentative. @@ -1280,12 +1302,12 @@ class RADatabase: ''' logger.info('insertResourceClaims for task_id=%d with %d claim(s)' % (task_id, len(claims))) - status_strings = set([c['status'] for c in claims if isinstance(c['status'], basestring)]) + status_strings = set([c.get('status', 'tentative') for c in claims if isinstance(c.get('status', 'tentative'), basestring)]) if status_strings: status_string2id = {s:self.getResourceClaimStatusId(s) for s in status_strings} for c in claims: - if isinstance(c['status'], basestring): - c['status_id'] = status_string2id[c['status']] + if isinstance(c.get('status', 'tentative'), basestring): + c['status_id'] = status_string2id[c.get('status', 'tentative')] elif isinstance(c['status'], int): c['status_id'] = c['status'] @@ -1590,6 +1612,8 @@ class RADatabase: if commit: self.commit() return {'inserted': True, 'specification_id': specId, 'task_id': taskId} + else: + self.rollback() except Exception as e: logger.error(e) self.rollback() diff --git a/SAS/ResourceAssignment/ResourceAssignmentDatabase/radb/sql/add_functions_and_triggers.sql b/SAS/ResourceAssignment/ResourceAssignmentDatabase/radb/sql/add_functions_and_triggers.sql index 9fa8caae0f5..b22fbd78f54 100644 --- a/SAS/ResourceAssignment/ResourceAssignmentDatabase/radb/sql/add_functions_and_triggers.sql +++ b/SAS/ResourceAssignment/ResourceAssignmentDatabase/radb/sql/add_functions_and_triggers.sql @@ -12,10 +12,13 @@ DECLARE claim_tentative_status_id int := 0; --beware: hard coded instead of lookup for performance claim_claimed_status_id int := 1; --beware: hard coded instead of lookup for performance task_approved_status_id int := 300; --beware: hard coded instead of lookup for performance + task_conflict_status_id int := 335; --beware: hard coded instead of lookup for performance BEGIN IF NEW.status_id <> OLD.status_id THEN - IF NEW.status_id = task_approved_status_id THEN - UPDATE resource_allocation.resource_claim rc SET status_id=claim_tentative_status_id WHERE rc.task_id=NEW.id AND rc.status_id <> claim_tentative_status_id; + IF NEW.status_id = task_approved_status_id OR NEW.status_id = task_conflict_status_id THEN + UPDATE resource_allocation.resource_claim + SET status_id=claim_tentative_status_id + WHERE (task_id=NEW.id AND status_id = claim_claimed_status_id); ELSIF NEW.status_id = ANY(ARRAY[400, 500, 600, 900, 1000, 1100]) THEN --prevent task status to be upgraded to scheduled (or beyond) when not all its claims are claimed IF EXISTS (SELECT id FROM resource_allocation.resource_claim WHERE task_id = NEW.id AND status_id <> claim_claimed_status_id) THEN @@ -232,37 +235,13 @@ CREATE TRIGGER T_specification_insertupdate_check_startendtimes --------------------------------------------------------------------------------------------------------------------- -CREATE OR REPLACE FUNCTION resource_allocation.on_claim_insertupdate_check_startendtimes() - RETURNS trigger AS -$BODY$ -BEGIN - IF NEW.starttime > NEW.endtime THEN - RAISE EXCEPTION 'claim starttime > endtime: %', NEW; - END IF; -RETURN NEW; -END; -$BODY$ - LANGUAGE plpgsql VOLATILE - COST 100; -ALTER FUNCTION resource_allocation.on_claim_insertupdate_check_startendtimes() - OWNER TO resourceassignment; - -DROP TRIGGER IF EXISTS T_claim_insertupdate_check_startendtimes ON resource_allocation.resource_claim; -CREATE TRIGGER T_claim_insertupdate_check_startendtimes - BEFORE INSERT OR UPDATE - ON resource_allocation.resource_claim - FOR EACH ROW - EXECUTE PROCEDURE resource_allocation.on_claim_insertupdate_check_startendtimes(); - ---------------------------------------------------------------------------------------------------------------------- - CREATE OR REPLACE FUNCTION resource_allocation.process_new_claim_into_resource_usages(new_claim resource_allocation.resource_claim) RETURNS void AS $$ DECLARE - usage_at_or_before_start RECORD; - usage_at_or_before_end RECORD; - intermediate_usage RECORD; + usage_at_or_before_start resource_allocation.resource_usage; + usage_at_or_before_end resource_allocation.resource_usage; + intermediate_usage resource_allocation.resource_usage; BEGIN -- find resource_usage at claim starttime SELECT * FROM resource_allocation.get_resource_usage_at_or_before(new_claim.resource_id, new_claim.status_id, new_claim.starttime, false, false, false) into usage_at_or_before_start; @@ -292,6 +271,7 @@ BEGIN INSERT INTO resource_allocation.resource_usage (resource_id, status_id, as_of_timestamp, usage) VALUES (new_claim.resource_id, new_claim.status_id, new_claim.endtime, usage_at_or_before_end.usage); END IF; + --TODO: 20180709; why no else with an upate? ELSE -- no previous usage known, so insert 0 as the last usage INSERT INTO resource_allocation.resource_usage (resource_id, status_id, as_of_timestamp, usage) @@ -602,6 +582,7 @@ BEGIN IF usage_at_end.usage = 0 THEN --usage_at_end was 'caused' by this deleted claim only, so delete it + --TODO:20180704 do not delete if another claim with this status and timestamp also causes this 0 DELETE FROM resource_allocation.resource_usage ru WHERE ru.id = usage_at_end.id; END IF; @@ -658,15 +639,7 @@ BEGIN -- try again, but now without the option to rebuild_usage_when_not_found (to prevent endless recursion) SELECT * FROM resource_allocation.get_resource_usage_at_or_before(_resource_id, _claim_status_id, _timestamp, exactly_at, only_before, false) INTO result; - RAISE NOTICE 'get_resource_usage_at_or_before(_resource_id=%, status_id=%, timestamp=%, exactly_at=%, only_before=%, rebuild_usage_when_not_found=%): after rebuild, result=%.', _resource_id, _claim_status_id, _timestamp, exactly_at, only_before, rebuild_usage_when_not_found, result; - END IF; - - IF result IS NULL THEN - -- if result is still null (after possible rebuild etc), then return a 'default' usage of 0 - result.resource_id = _resource_id; - result.status_id = _claim_status_id; - result.as_of_timestamp = _timestamp; - result.usage = 0; + RAISE NOTICE 'get_resource_usage_at_or_before(_resource_id=%, status_id=%, timestamp=%, exactly_at=%, only_before=%, rebuild_usage_when_not_found=%): after rebuild, result=%.', _resource_id, _claim_status_id, _timestamp, exactly_at, only_before, false, result; END IF; RETURN result; @@ -711,8 +684,8 @@ CREATE OR REPLACE FUNCTION resource_allocation.get_max_resource_usage_between(_r RETURNS resource_allocation.resource_usage AS $$ DECLARE - max_resource_usage_in_time_window record; - max_resource_at_or_before_starttime record; + max_resource_usage_in_time_window resource_allocation.resource_usage; + max_resource_at_or_before_starttime resource_allocation.resource_usage; BEGIN SELECT * FROM resource_allocation.get_resource_usage_at_or_before(_resource_id, _claim_status_id, _lower, false, false, false) into max_resource_at_or_before_starttime; @@ -725,10 +698,14 @@ BEGIN LIMIT 1 INTO max_resource_usage_in_time_window; IF max_resource_usage_in_time_window IS NOT NULL THEN - IF max_resource_usage_in_time_window.usage > max_resource_at_or_before_starttime.usage THEN - RETURN max_resource_usage_in_time_window; + IF max_resource_at_or_before_starttime IS NULL THEN + RETURN max_resource_usage_in_time_window; ELSE - RETURN max_resource_at_or_before_starttime; + IF max_resource_usage_in_time_window.usage > max_resource_at_or_before_starttime.usage THEN + RETURN max_resource_usage_in_time_window; + ELSE + RETURN max_resource_at_or_before_starttime; + END IF; END IF; ELSE -- could also be NULL but that is checked for elsewhere @@ -783,7 +760,7 @@ BEGIN END IF; END; $$ LANGUAGE plpgsql; -ALTER FUNCTION resource_allocation.get_resource_claimable_capacity_between(_resource_id int, _lower timestamp, _upper timestamp) +ALTER FUNCTION resource_allocation.get_resource_claimable_capacity_between(_resource_id int, _lower timestamp, _upper timestamp) OWNER TO resourceassignment; COMMENT ON FUNCTION resource_allocation.get_resource_claimable_capacity_between(_resource_id int, _lower timestamp, _upper timestamp) IS 'get the maximum resource usage for the given _resource_id for claims with given _claim_status_id in the period between the given _lower and _upper timestamps'; @@ -867,6 +844,14 @@ DECLARE BEGIN --order of following steps is important, do not reorder the steps + IF TG_OP = 'INSERT' OR TG_OP = 'UPDATE' THEN + IF NEW.starttime >= NEW.endtime THEN + -- Conceptually, you can't claim and release a resource at the same timestamp. + -- Nor can you claim a resource for a negative timespan. + RAISE EXCEPTION 'claim starttime >= endtime: %', NEW; + END IF; + END IF; + -- bounce any inserted claim which is not tentative IF TG_OP = 'INSERT' THEN IF NEW.status_id <> claim_tentative_status_id THEN @@ -931,6 +916,11 @@ BEGIN END IF; END IF; + IF TG_OP = 'INSERT' OR TG_OP = 'UPDATE' THEN + --update the resource usages affected by this claim + PERFORM resource_allocation.process_new_claim_into_resource_usages(NEW); + END IF; + IF TG_OP = 'DELETE' THEN RETURN OLD; END IF; @@ -966,14 +956,6 @@ DECLARE affected_claim resource_allocation.resource_claim; claim_has_conflicts boolean; BEGIN - --do not process_old_claim_outof_resource_usages(OLD) - --because that has been done already in before_claim_insertupdatedelete - - IF TG_OP = 'INSERT' OR TG_OP = 'UPDATE' THEN - --update the resource usages affected by this claim - PERFORM resource_allocation.process_new_claim_into_resource_usages(NEW); - END IF; - -- in the before trigger function, everything on the claim has been checked and adapted. -- now (in the after trigger, when all claims were inserted/updated in the database), let's check if the task should also be updated (to conflict status for example) -- only if claim status was changed or inserted... @@ -982,8 +964,14 @@ BEGIN --if claim status went to conflict, then set the task status to conflict as well UPDATE resource_allocation.task SET status_id=task_conflict_status_id WHERE id=NEW.task_id AND status_id <> task_conflict_status_id; ELSIF NEW.status_id = claim_tentative_status_id THEN - IF NOT EXISTS (SELECT id FROM resource_allocation.resource_claim WHERE task_id = NEW.task_id AND status_id = claim_conflict_status_id) THEN - UPDATE resource_allocation.task SET status_id=task_approved_status_id WHERE id=NEW.task_id AND status_id <> task_approved_status_id; + IF NOT EXISTS (SELECT id FROM resource_allocation.resource_claim + WHERE task_id = NEW.task_id + AND status_id = claim_conflict_status_id) THEN + IF NOT EXISTS (SELECT id FROM resource_allocation.task + WHERE id = NEW.task_id + AND status_id = task_approved_status_id) THEN + UPDATE resource_allocation.task SET status_id=task_approved_status_id WHERE id=NEW.task_id AND status_id <> task_approved_status_id; + END IF; END IF; END IF; END IF; @@ -1030,7 +1018,7 @@ BEGIN END IF; END LOOP; END IF; - + IF TG_OP = 'DELETE' THEN RETURN OLD; END IF; -- GitLab