Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

fix: Improve advisory locks (follow-up to #482) #483

Merged
merged 7 commits into from
Oct 5, 2021

Conversation

achimnol
Copy link
Member

@achimnol achimnol commented Oct 4, 2021

This is a follow-up of #482.

  • Handle "lock not available" error upon shutdown.
  • Use non-blocking pg_try_advisory_lock() instead of blocking pg_advisory_lock() to avoid conflicts with statement/lock timeout configurations.
    • This incurs a slightly more CPU usage due to polling, but it will keep the DB client connection to be alive.
  • refactor: Single-source database connection routines as models.utils.connect_database() async context manager.

@achimnol achimnol added this to the 21.03 milestone Oct 4, 2021
@achimnol achimnol added the bug label Oct 4, 2021
@codecov
Copy link

codecov bot commented Oct 4, 2021

Codecov Report

Merging #483 (324314e) into main (d38942c) will decrease coverage by 0.00%.
The diff coverage is 81.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #483      +/-   ##
==========================================
- Coverage   48.78%   48.77%   -0.01%     
==========================================
  Files          54       54              
  Lines        8882     8896      +14     
==========================================
+ Hits         4333     4339       +6     
- Misses       4549     4557       +8     
Impacted Files Coverage Δ
src/ai/backend/manager/server.py 60.82% <50.00%> (-0.66%) ⬇️
src/ai/backend/manager/models/utils.py 76.27% <85.29%> (-1.89%) ⬇️
src/ai/backend/manager/plugin/__init__.py 84.88% <0.00%> (-3.49%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d38942c...324314e. Read the comment docs.

- This will avoid being affected by arbitrary statement and lock timeout configs
  as well as TCP keepalive configs, with a small sacrifice on CPU cycles
  and network traffic at the database connection for each timer.

* refactor: Single-source the database creation routine
* test: Use newly created database engines in test_distributed.TimerNode
  to avoid event loop mismatch
@achimnol achimnol force-pushed the fix/follow-up-of-pr482 branch from 8c05d80 to 64b74b6 Compare October 4, 2021 16:45
@achimnol achimnol changed the title fix: Ignore DBAPIError for locking error upon service shutdown fix: Improve advisory locks (follow-up to #482) Oct 5, 2021
@achimnol achimnol merged commit 7d26f10 into main Oct 5, 2021
@achimnol achimnol deleted the fix/follow-up-of-pr482 branch October 5, 2021 03:54
achimnol added a commit that referenced this pull request Oct 5, 2021
* Use pg_try_advisory_lock() combined with client-side sleep
  - This will avoid being affected by arbitrary statement and lock timeout configs
    as well as TCP keepalive configs, with a small sacrifice on CPU cycles
    and network traffic at the database connection for each timer.
* refactor: Single-source the database creation routine
  as models.utils.connect_database()

Backported-From: main (21.09)
Backported-To: 21.03
achimnol added a commit that referenced this pull request Dec 9, 2021
* But preserve other refactoring parts of #483.
achimnol added a commit that referenced this pull request Dec 14, 2021
* But preserve other refactoring parts of #483.
achimnol added a commit that referenced this pull request Dec 14, 2021
* But preserve other refactoring parts of #483.

Backported-From: main (22.03)
Backported-To: 21.09
achimnol added a commit that referenced this pull request Dec 14, 2021
* But preserve other refactoring parts of #483.

Backported-From: main (22.03)
Backported-To: 21.03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant