fabric8-services / fabric8-auth

Identity and Access Management for fabric8 services
https://auth.openshift.io/api/status
Apache License 2.0
14 stars 26 forks source link

Fix deactivation worker locking mechanism #833

Closed alexeykazakov closed 5 years ago

alexeykazakov commented 5 years ago

This PR fixes a few issues with the locking mechanism:

  1. Don't use pglock.FailIfLocked() option in c.Acquire(name, pglock.FailIfLocked()) when trying to acquire a lock. If this option is used then pglock won't check if the lease has been expired. So even if hearbeat failed (because the owner is gone) this option will always return an error which will prevent to acquire the lock by another owner.
  2. Don't try to acquire a lock in every cycle. Do it only once when the worker is started. The acquiring routine will wait until the lock is available anyway (see the # 1 above).
  3. There is an upstream bug in pglock which was introduced in the latest release - https://github.com/cirello-io/pglock/pull/1 which breaks the hearbeat completely. While the bug is being fixed upstream I've switched to our fork with the fix in place.
  4. More logs
  5. More tests
codecov[bot] commented 5 years ago

Codecov Report

Merging #833 into master will decrease coverage by 0.23%. The diff coverage is 59.09%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #833      +/-   ##
========================================
- Coverage   78.24%    78%   -0.24%     
========================================
  Files          98     98              
  Lines        9206   9254      +48     
========================================
+ Hits         7203   7219      +16     
- Misses       1470   1497      +27     
- Partials      533    538       +5
Impacted Files Coverage Δ
...n/subscription/service/oso_subscription_service.go 79.13% <100%> (ø) :arrow_up:
...ication/account/worker/user_deactivation_worker.go 46.34% <100%> (ø) :arrow_up:
...nt/worker/user_deactivation_notification_worker.go 82.6% <100%> (ø) :arrow_up:
worker/worker.go 60.49% <26.31%> (-21.05%) :arrow_down:
worker/repository/worker_lock_repository.go 71.92% <68.29%> (-20.08%) :arrow_down:
log/log.go 74.1% <0%> (+1.43%) :arrow_up:

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 8469a52...13c2b67. Read the comment docs.