Add TASK_MANAGER_LOCK_TIMEOUT #15300
Conversation
TheRealHaoLiu
requested review from
kdelee,
fosterseth,
chrismeyersfsu and
AlanCoding
|
Shouldn't this also apply to other locks like the periodic scheduler? A locked instance could still block any scheduled jobs from starting. |
awx/main/utils/pglock.py
Outdated
| with django_pglocks_advisory_lock(*args, **kwargs) as internal_lock: | ||
| yield internal_lock | ||
| if lock_session_timeout_milliseconds > 0: | ||
| cur.execute(f"SET idle_in_transaction_session_timeout = {idle_in_transaction_session_timeout}") |
There was a problem hiding this comment.
in the case that we are closed, this will hit a InterfaceError: the cursor is closed error
in the case of task managers, we probably don't care because we are willing for the entire pid to go away, but if someone later on uses lock_session_timeout_milliseconds in another case there might be adverse affects
There was a problem hiding this comment.
cursor has a closed property that we can condition off of
There was a problem hiding this comment.
yah these 2 parts shouldnt have to shared a cursor... let me go ahead and use context manager for these 2 section of code
`TASK_MANAGER_LOCK_TIMEOUT` controls the `idle_in_transaction_session_timeout` and `idle_session_timeout` configuration for task manager connections and lock in database hope to prevent the situation that the task instance that holds the lock becomes unresponsive and preventing other instance to be able to run task manager
TheRealHaoLiu
force-pushed
the
task-manager-session-timeout
branch
from
b5bccc8
to
3a5c195
Compare
|
Can we file a followup issue in https://github.com/Xof/django-pglocks? This just seems really chatty and it's done on tasks running on 20 second intervals. An obvious action is to contribute this upstream to the library we're using and have it maintained there, but it might help to talk it out and ask if there should have been a better way to do this. Maybe some postgresql community could give some good feedback. Having this that works is a good starting place, but it would still be nice to iterate to make it better. |
* Add TASK_MANAGER_LOCK_TIMEOUT `TASK_MANAGER_LOCK_TIMEOUT` controls the `idle_in_transaction_session_timeout` and `idle_session_timeout` configuration for task manager connections and lock in database hope to prevent the situation that the task instance that holds the lock becomes unresponsive and preventing other instance to be able to run task manager * Add session timeout to periodic scheduler and all sub task manager locks
* Add TASK_MANAGER_LOCK_TIMEOUT `TASK_MANAGER_LOCK_TIMEOUT` controls the `idle_in_transaction_session_timeout` and `idle_session_timeout` configuration for task manager connections and lock in database hope to prevent the situation that the task instance that holds the lock becomes unresponsive and preventing other instance to be able to run task manager * Add session timeout to periodic scheduler and all sub task manager locks
SUMMARY
TASK_MANAGER_LOCK_TIMEOUTcontrols theidle_in_transaction_session_timeoutandidle_session_timeoutDB connection setting for task manager connections and lock in databasehope to prevent the situation that the task instance that holds the lock becomes unresponsive and preventing other instance to be able to run task manager
ISSUE TYPE
COMPONENT NAME
AWX VERSION
ADDITIONAL INFORMATION