Skip to content

Commit

Permalink
IRONN-270 hotfix. (#4423)
Browse files Browse the repository at this point in the history
See also https://movember.atlassian.net/browse/IRONN-270
 
The `TimeoutLock` used to prevent concurrent EMPRO trigger states
overwrites was expiring prior to completion.

Elaborate chain of events needed to reproduce the problem, but the
existing 60 second timeout for any single user's updates to their
trigger_states->triggers data was inadequate when other jobs (such as
`update_patients_task`) came in before completion of trigger processing.

The end result was an area of code intended to be locked in a critical
section, would obtain a fresh lock even if one was in use, when it had
been longer than the expiration time.

This PR gives more than adequate space (2 hours rather than 60 seconds).

---------

Co-authored-by: Ivan Cvitkovic <ivanc@uw.edu>
Co-authored-by: Amy Chen <achen2401@gmail.com>
Co-authored-by: Amy Chen <clone@cesium.cirg.washington.edu>
  • Loading branch information
4 people authored Nov 13, 2024
1 parent 212e244 commit 89eb8ef
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 4 deletions.
20 changes: 18 additions & 2 deletions portal/static/js/src/admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,12 @@ let requestTimerId = 0;
errorElement.innerHTML = errorMessage;
}
},
clearError: function() {
var errorElement = document.getElementById("admin-table-error-message");
if (errorElement) {
errorElement.innerHTML = "";
}
},
injectDependencies: function () {
var self = this;
window.portalModules =
Expand Down Expand Up @@ -200,6 +206,8 @@ let requestTimerId = 0;
);
return;
}
//reset error
this.clearError();
this.patientDataAjaxRequest(params);
},
patientDataAjaxRequest: function (params) {
Expand All @@ -219,6 +227,12 @@ let requestTimerId = 0;
}
self.accessed = true;
params.success(results);
}).fail(function(xhr, status) {
console.log("Error ", xhr);
console.log("status", status);
self.setError("Error occurred loading data.");
params.success([]);
self.accessed = true;
});
},
handleCurrentUser: function () {
Expand Down Expand Up @@ -1229,11 +1243,12 @@ let requestTimerId = 0;
sync: true,
},
function (data) {
prefData = data || self.getDefaultTablePreference();
self.currentTablePreference = prefData;

if (!data || data.error) {
return false;
}
prefData = data || self.getDefaultTablePreference();
self.currentTablePreference = prefData;

if (setFilter) {
//set filter values
Expand Down Expand Up @@ -1303,6 +1318,7 @@ let requestTimerId = 0;
for (var item in prefData.filters) {
fname = "#adminTable .bootstrap-table-filter-control-" + item;
if ($(fname).length === 0) {
prefData.filters[item] = null;
continue;
}
//note this is based on the trigger event for filtering specify in the plugin
Expand Down
3 changes: 2 additions & 1 deletion portal/templates/admin/patients_by_org.html
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ <h4 class="tnth-headline">{{_("Patient List")}}</h4>
</thead>
</table>
</div>
<div id="admin-table-error-message" class="text-danger smaller-text"></div>
<br/>
<div id="admin-table-error-message" class="text-danger"></div>
{{ExportPopover()}}
</div>
{{ajaxDataScript(research_study_id=0)}}
Expand Down
10 changes: 9 additions & 1 deletion portal/trigger_states/empro_states.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from ..timeout_lock import TimeoutLock, LockTimeout

EMPRO_LOCK_KEY = "empro-trigger-state-lock-{user_id}"
EMPRO_LOCK_EXPIRATION = 2*60*60 # yes, 2 hours given interruptions by big jobs (see IRONN-270)
OPT_OUT_DELAY = 1800 # seconds to allow user to provide opt-out choices

class EMPRO_state(StateMachine):
Expand Down Expand Up @@ -296,6 +297,9 @@ def process_processed(ts):
# necessary to make deep copy in order to update DB JSON
triggers = copy.deepcopy(ts.triggers)
triggers['action_state'] = 'not applicable'
if 'actions' in triggers:
current_app.logger.error(
f"unexpected existing 'actions' in trigger_states.triggers({ts.id}): {triggers['actions']}")
triggers['actions'] = dict()
triggers['actions']['email'] = list()

Expand Down Expand Up @@ -413,6 +417,7 @@ def process_pending_actions(ts):
try:
with TimeoutLock(
key=EMPRO_LOCK_KEY.format(user_id=ts.user_id),
expires=EMPRO_LOCK_EXPIRATION,
timeout=NEVER_WAIT):
process_processed(ts)
db.session.commit()
Expand All @@ -425,6 +430,7 @@ def process_pending_actions(ts):
try:
with TimeoutLock(
key=EMPRO_LOCK_KEY.format(user_id=ts.user_id),
expires=EMPRO_LOCK_EXPIRATION,
timeout=NEVER_WAIT):
process_pending_actions(ts)
db.session.commit()
Expand Down Expand Up @@ -555,7 +561,9 @@ def extract_observations(questionnaire_response_id, override_state=False):

# given asynchronous possibility, require user's EMPRO lock
with TimeoutLock(
key=EMPRO_LOCK_KEY.format(user_id=qnr.subject_id), timeout=60):
key=EMPRO_LOCK_KEY.format(user_id=qnr.subject_id),
expires=EMPRO_LOCK_EXPIRATION,
timeout=EMPRO_LOCK_EXPIRATION+60):
ts = users_trigger_state(qnr.subject_id)
sm = EMPRO_state(ts)
if not override_state:
Expand Down

0 comments on commit 89eb8ef

Please sign in to comment.