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
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant