CCMRG-1907 Refactor lock management in tasks to use LockManager #589

drazisil-codecov · 2025-12-01T14:42:20Z

Introduced LockManager for consistent lock handling across tasks.
Updated tasks (ManualTrigger, PreProcessUpload, UploadFinisher, etc.) to utilize LockManager for acquiring locks.
Added blocking timeout configuration to prevent indefinite waiting on locks.
Enhanced error handling with LockRetry for better retry logic on lock acquisition failures.
Removed redundant lock handling code .
Updated tests to reflect changes in lock management and ensure proper behavior under lock contention scenarios.
visibility_timeout is now adjustable based on if acks_late is true.
Migrated task logging for max retries exceeded to LockManager for central logging.

- Introduced LockManager for consistent lock handling across tasks. - Updated tasks (ManualTrigger, PreProcessUpload, UploadFinisher, etc.) to utilize LockManager for acquiring locks. - Added blocking timeout configuration to prevent indefinite waiting on locks. - Enhanced error handling with LockRetry for better retry logic on lock acquisition failures. - Removed redundant lock handling code and improved logging for lock operations. - Updated tests to reflect changes in lock management and ensure proper behavior under lock contention scenarios.

linear · 2025-12-01T14:42:23Z

CCMRG-1907 Migrate remaining tasks locks to use LockManager service

apps/worker/tasks/base.py

apps/worker/tasks/bundle_analysis_notify.py

apps/worker/tasks/bundle_analysis_processor.py

apps/worker/tasks/preprocess_upload.py

apps/worker/tasks/test_analytics_notifier.py

codecov-eu · 2025-12-01T14:51:48Z

Codecov Report

❌ Patch coverage is 93.58974% with 5 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
apps/worker/tasks/tests/utils.py	83.33%	2 Missing ⚠️
apps/worker/tasks/upload_finisher.py	87.50%	2 Missing ⚠️
apps/worker/tasks/preprocess_upload.py	90.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

codecov-notifications · 2025-12-01T14:51:54Z

Codecov Report

❌ Patch coverage is 93.58974% with 5 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
apps/worker/tasks/tests/utils.py	83.33%	2 Missing ⚠️
apps/worker/tasks/upload_finisher.py	87.50%	2 Missing ⚠️
apps/worker/tasks/preprocess_upload.py	90.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

sentry · 2025-12-01T14:52:00Z

Codecov Report

❌ Patch coverage is 93.58974% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.86%. Comparing base (b081221) to head (6ccd53d).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
apps/worker/tasks/tests/utils.py	83.33%	2 Missing ⚠️
apps/worker/tasks/upload_finisher.py	87.50%	2 Missing ⚠️
apps/worker/tasks/preprocess_upload.py	90.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #589      +/-   ##
==========================================
- Coverage   93.86%   93.86%   -0.01%     
==========================================
  Files        1284     1284              
  Lines       46492    46512      +20     
  Branches     1522     1522              
==========================================
+ Hits        43642    43660      +18     
- Misses       2540     2542       +2     
  Partials      310      310

Flag	Coverage Δ
apiunit	`96.55% <ø> (ø)`
sharedintegration	`38.79% <100.00%> (+0.03%)`	⬆️
sharedunit	`88.77% <100.00%> (+<0.01%)`	⬆️
workerintegration	`58.60% <50.70%> (-0.05%)`	⬇️
workerunit	`91.22% <92.95%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codspeed-hq · 2025-12-01T14:52:13Z

CodSpeed Performance Report

Merging #589 will not alter performance

_{Comparing CCMRG-1907-migrate-task-locks-to-lock-manager (6ccd53d) with main (39fb6a9)¹}

Summary

✅ 9 untouched

No successful run was found on main (b081221) during the generation of this report, so 39fb6a9 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

- Removed redundant comments regarding indefinite blocking timeout in multiple task files. - Updated documentation in the `ensure_hard_time_limit_task_is_numeric` helper to clarify its usage in tests. - Ensured consistency across tasks by standardizing the blocking timeout parameter handling.

- Eliminated unnecessary warning logs for lock acquisition failures in ManualTriggerTask, PreProcessUpload, UploadFinisherTask, and UploadTask. - Streamlined error handling by focusing on essential logging, improving code clarity and reducing log noise.

- Updated the `locked` method in `LockManager` to accept an optional `max_retries` parameter for better control over retry logic. - Adjusted task implementations (ManualTriggerTask, PreProcessUpload, UploadFinisherTask, UploadTask) to utilize the new `max_retries` parameter, improving error handling and logging consistency. - Removed redundant lock acquisition logging from tasks, streamlining the code and reducing log noise.

- Removed redundant comments in `LockManager` and `UploadTask` to enhance code clarity. - Updated `celery_config.py` to introduce configurable visibility timeouts for Redis broker based on task acknowledgment settings, improving task recovery and preventing duplicate executions. - Adjusted unit tests to reflect changes in visibility timeout configuration, ensuring accurate assertions.

calvin-codecov · 2025-12-01T23:12:29Z

It's failing on the 3 TestGetLockTimeout tests at the moment

apps/worker/tasks/upload_finisher.py