From 54a02bac22e438b4073571e106aaef141556db03 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Thu, 9 Apr 2026 16:48:58 +0500 Subject: [PATCH 1/3] fix: added lock to prevent same task run multiple time at the same time --- .../ol_openedx_git_auto_export/constants.py | 14 +++ .../ol_openedx_git_auto_export/tasks.py | 20 +++- .../ol_openedx_git_auto_export/utils.py | 98 +++++++++++++++++++ 3 files changed, 130 insertions(+), 2 deletions(-) diff --git a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/constants.py b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/constants.py index 3d1b9882e..d1b33f6ab 100644 --- a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/constants.py +++ b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/constants.py @@ -5,3 +5,17 @@ COURSE_RERUN_STATE_SUCCEEDED = "succeeded" REPOSITORY_NAME_MAX_LENGTH = 100 # Max length from GitHub for repo name + +# Per-course git export distributed lock settings. +# The pending task's total wait budget (MAX_RETRIES * RETRY_DELAY) must exceed +# the lock TTL so a pending task cannot exhaust its retries while the running +# export is still in progress. +EXPORT_LOCK_TIMEOUT = 120 # seconds; safety TTL if a worker crashes holding the lock +EXPORT_LOCK_RETRY_DELAY = 30 # seconds between retries for the pending task +EXPORT_LOCK_MAX_RETRIES = 5 # max retries for the pending task +# At most one extra task queues behind the running task; all other duplicates drop. +EXPORT_PENDING_TIMEOUT = EXPORT_LOCK_TIMEOUT # match the lock TTL + +# Cache key templates for the distributed git-export lock. +EXPORT_LOCK_CACHE_KEY = "git_export_lock:{course_key}" +EXPORT_PENDING_CACHE_KEY = "git_export_pending:{course_key}" diff --git a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py index faaf5c889..7908fafd7 100644 --- a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py +++ b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py @@ -16,19 +16,29 @@ from ol_openedx_git_auto_export.models import CourseGitRepository from ol_openedx_git_auto_export.utils import ( + acquire_export_lock_or_schedule, + clear_stale_git_lock, export_course_to_git, github_repo_name_format, is_auto_repo_creation_enabled, + release_export_lock, ) LOGGER = get_task_logger(__name__) -@shared_task -def async_export_to_git(course_key_string, user=None): +@shared_task(bind=True) +def async_export_to_git(self, course_key_string, user=None): """ Exports a course to Git. + + Concurrency and deduplication are handled by + ``acquire_export_lock_or_schedule`` / ``release_export_lock`` in utils. + See those functions for the full locking protocol. """ # noqa: D401 + if not acquire_export_lock_or_schedule(self, course_key_string): + return # duplicate task — dropped by the lock helper + course_key = CourseKey.from_string(course_key_string) course_module = modulestore().get_course(course_key) @@ -39,6 +49,10 @@ def async_export_to_git(course_key_string, user=None): "Starting async course content export to git (course id: %s)", course_module.id, ) + # Remove any stale .git/index.lock left by a previously crashed worker. + # Dirty working-tree files from a prior crash are cleaned by the + # `git reset --hard origin/` + `git clean` inside export_to_git. + clear_stale_git_lock(course_repo.git_url) export_to_git(course_module.id, course_repo.git_url, user=user) else: LOGGER.info( @@ -63,6 +77,8 @@ def async_export_to_git(course_key_string, user=None): "Unknown error occured during async course content export to git (course id: %s)", # noqa: E501 course_module.id, ) + finally: + release_export_lock(course_key_string) @shared_task( diff --git a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/utils.py b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/utils.py index c422fda36..8da3f1105 100644 --- a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/utils.py +++ b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/utils.py @@ -5,15 +5,23 @@ import logging import os import re +from pathlib import Path from django.conf import settings from django.contrib.auth.models import User +from django.core.cache import cache from django.core.exceptions import ImproperlyConfigured from xmodule.modulestore.django import modulestore from ol_openedx_git_auto_export.constants import ( ENABLE_AUTO_GITHUB_REPO_CREATION, ENABLE_GIT_AUTO_EXPORT, + EXPORT_LOCK_CACHE_KEY, + EXPORT_LOCK_MAX_RETRIES, + EXPORT_LOCK_RETRY_DELAY, + EXPORT_LOCK_TIMEOUT, + EXPORT_PENDING_CACHE_KEY, + EXPORT_PENDING_TIMEOUT, REPOSITORY_NAME_MAX_LENGTH, ) @@ -103,6 +111,96 @@ def export_course_to_git(course_key): async_export_to_git.delay(str(course_key), user) +def clear_stale_git_lock(git_url): + """ + Remove a stale .git/index.lock file for the local clone of git_url, if present. + + This must only be called after acquiring the per-course distributed cache lock, + which guarantees no other process is running git operations on the same directory. + A stale lock file is left behind when a worker process is killed mid-operation. + """ + git_repo_export_dir = getattr( + settings, "GIT_REPO_EXPORT_DIR", "/openedx/export_course_repos" + ) + rdir = git_url.rsplit("/", 1)[-1].rsplit(".git", 1)[0] + index_lock = Path(git_repo_export_dir) / rdir / ".git" / "index.lock" + if index_lock.exists(): + log.warning( + "Removing stale .git/index.lock for repo %s at %s", git_url, index_lock + ) + index_lock.unlink() + + +def acquire_export_lock_or_schedule(task, course_key_string): + """ + Attempt to acquire the per-course git-export distributed lock. + + Uses two cache keys: + - ``EXPORT_LOCK_CACHE_KEY`` — held while the export is running. + - ``EXPORT_PENDING_CACHE_KEY`` — stores the task-ID of the single task + that is waiting to run after the lock-holder finishes. + + Returns True if the caller acquired the lock and should proceed with the + export. Returns False if the caller is a duplicate that was dropped. + Raises ``celery.exceptions.Retry`` if the caller is the designated pending + task and needs to retry later. + """ + lock_key = EXPORT_LOCK_CACHE_KEY.format(course_key=course_key_string) + pending_key = EXPORT_PENDING_CACHE_KEY.format(course_key=course_key_string) + task_id = task.request.id + + if not cache.add(lock_key, task_id, timeout=EXPORT_LOCK_TIMEOUT): + # Lock is held — check if we are already the designated pending task. + if cache.get(pending_key) == task_id: + log.info( + "Export lock still held for %s, pending task %s retrying in %ds" + " (attempt %d/%d)", + course_key_string, + task_id, + EXPORT_LOCK_RETRY_DELAY, + task.request.retries + 1, + EXPORT_LOCK_MAX_RETRIES, + ) + raise task.retry( + countdown=EXPORT_LOCK_RETRY_DELAY, max_retries=EXPORT_LOCK_MAX_RETRIES + ) + + # Try to become the single designated pending task (atomic). + if cache.add(pending_key, task_id, timeout=EXPORT_PENDING_TIMEOUT): + log.info( + "Export already in progress for %s; task %s queued as pending," + " retrying in %ds", + course_key_string, + task_id, + EXPORT_LOCK_RETRY_DELAY, + ) + raise task.retry( + countdown=EXPORT_LOCK_RETRY_DELAY, max_retries=EXPORT_LOCK_MAX_RETRIES + ) + + # Pending slot already taken — drop this duplicate. + log.info( + "Dropping duplicate export task %s for %s (lock held, pending slot taken)", + task_id, + course_key_string, + ) + return False + + # Lock acquired — clear the pending slot so a fresh task can claim it. + cache.delete(pending_key) + return True + + +def release_export_lock(course_key_string): + """ + Release the per-course git-export distributed lock. + + Must be called in a ``finally`` block after ``acquire_export_lock_or_schedule`` + returns True. + """ + cache.delete(EXPORT_LOCK_CACHE_KEY.format(course_key=course_key_string)) + + def is_auto_repo_creation_enabled(): """ Check if automatic GitHub repository creation is enabled. From 0f53727de06a367579fa2c7f364ea21aad3ad93a Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Wed, 15 Apr 2026 15:42:49 +0500 Subject: [PATCH 2/3] fix: add debounce strategy before calling the celery tasks to avoid queuing extra tasks --- .../ol_openedx_git_auto_export/constants.py | 21 ++-- .../ol_openedx_git_auto_export/tasks.py | 15 +-- .../ol_openedx_git_auto_export/utils.py | 100 ++++-------------- 3 files changed, 30 insertions(+), 106 deletions(-) diff --git a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/constants.py b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/constants.py index d1b33f6ab..16def68a1 100644 --- a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/constants.py +++ b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/constants.py @@ -6,16 +6,11 @@ COURSE_RERUN_STATE_SUCCEEDED = "succeeded" REPOSITORY_NAME_MAX_LENGTH = 100 # Max length from GitHub for repo name -# Per-course git export distributed lock settings. -# The pending task's total wait budget (MAX_RETRIES * RETRY_DELAY) must exceed -# the lock TTL so a pending task cannot exhaust its retries while the running -# export is still in progress. -EXPORT_LOCK_TIMEOUT = 120 # seconds; safety TTL if a worker crashes holding the lock -EXPORT_LOCK_RETRY_DELAY = 30 # seconds between retries for the pending task -EXPORT_LOCK_MAX_RETRIES = 5 # max retries for the pending task -# At most one extra task queues behind the running task; all other duplicates drop. -EXPORT_PENDING_TIMEOUT = EXPORT_LOCK_TIMEOUT # match the lock TTL - -# Cache key templates for the distributed git-export lock. -EXPORT_LOCK_CACHE_KEY = "git_export_lock:{course_key}" -EXPORT_PENDING_CACHE_KEY = "git_export_pending:{course_key}" +# Debounce settings for the signal handler. +# A single course save triggers 10-30 COURSE_PUBLISHED signals in one request. +# cache.add() on this key ensures only the first signal schedules a task; all +# subsequent signals within the window are silently dropped before hitting the broker. +# The task is scheduled with countdown=EXPORT_DEBOUNCE_DELAY so it runs after +# the burst window has closed and the course state is fully settled. +EXPORT_DEBOUNCE_DELAY = 5 # seconds — must exceed the publish burst window +EXPORT_DEBOUNCE_CACHE_KEY = "git_export_debounce:{course_key}" diff --git a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py index 7908fafd7..71420302f 100644 --- a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py +++ b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py @@ -16,29 +16,20 @@ from ol_openedx_git_auto_export.models import CourseGitRepository from ol_openedx_git_auto_export.utils import ( - acquire_export_lock_or_schedule, clear_stale_git_lock, export_course_to_git, github_repo_name_format, is_auto_repo_creation_enabled, - release_export_lock, ) LOGGER = get_task_logger(__name__) -@shared_task(bind=True) -def async_export_to_git(self, course_key_string, user=None): +@shared_task +def async_export_to_git(course_key_string, user=None): """ Exports a course to Git. - - Concurrency and deduplication are handled by - ``acquire_export_lock_or_schedule`` / ``release_export_lock`` in utils. - See those functions for the full locking protocol. """ # noqa: D401 - if not acquire_export_lock_or_schedule(self, course_key_string): - return # duplicate task — dropped by the lock helper - course_key = CourseKey.from_string(course_key_string) course_module = modulestore().get_course(course_key) @@ -77,8 +68,6 @@ def async_export_to_git(self, course_key_string, user=None): "Unknown error occured during async course content export to git (course id: %s)", # noqa: E501 course_module.id, ) - finally: - release_export_lock(course_key_string) @shared_task( diff --git a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/utils.py b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/utils.py index 8da3f1105..9303ce8b1 100644 --- a/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/utils.py +++ b/src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/utils.py @@ -16,12 +16,8 @@ from ol_openedx_git_auto_export.constants import ( ENABLE_AUTO_GITHUB_REPO_CREATION, ENABLE_GIT_AUTO_EXPORT, - EXPORT_LOCK_CACHE_KEY, - EXPORT_LOCK_MAX_RETRIES, - EXPORT_LOCK_RETRY_DELAY, - EXPORT_LOCK_TIMEOUT, - EXPORT_PENDING_CACHE_KEY, - EXPORT_PENDING_TIMEOUT, + EXPORT_DEBOUNCE_CACHE_KEY, + EXPORT_DEBOUNCE_DELAY, REPOSITORY_NAME_MAX_LENGTH, ) @@ -108,16 +104,30 @@ def export_course_to_git(course_key): ) user = get_publisher_username(course_module) - async_export_to_git.delay(str(course_key), user) + + debounce_key = EXPORT_DEBOUNCE_CACHE_KEY.format(course_key=str(course_key)) + if cache.add(debounce_key, "1", timeout=EXPORT_DEBOUNCE_DELAY): + log.info( + "Scheduling git export for course %s with %ds debounce delay", + course_key, + EXPORT_DEBOUNCE_DELAY, + ) + async_export_to_git.apply_async( + args=[str(course_key), user], + countdown=EXPORT_DEBOUNCE_DELAY, + ) + else: + log.info( + "Git export already scheduled for course %s, skipping duplicate signal", + course_key, + ) def clear_stale_git_lock(git_url): """ Remove a stale .git/index.lock file for the local clone of git_url, if present. - This must only be called after acquiring the per-course distributed cache lock, - which guarantees no other process is running git operations on the same directory. - A stale lock file is left behind when a worker process is killed mid-operation. + A stale lock file can be left behind when a worker process is killed mid-operation. """ git_repo_export_dir = getattr( settings, "GIT_REPO_EXPORT_DIR", "/openedx/export_course_repos" @@ -131,76 +141,6 @@ def clear_stale_git_lock(git_url): index_lock.unlink() -def acquire_export_lock_or_schedule(task, course_key_string): - """ - Attempt to acquire the per-course git-export distributed lock. - - Uses two cache keys: - - ``EXPORT_LOCK_CACHE_KEY`` — held while the export is running. - - ``EXPORT_PENDING_CACHE_KEY`` — stores the task-ID of the single task - that is waiting to run after the lock-holder finishes. - - Returns True if the caller acquired the lock and should proceed with the - export. Returns False if the caller is a duplicate that was dropped. - Raises ``celery.exceptions.Retry`` if the caller is the designated pending - task and needs to retry later. - """ - lock_key = EXPORT_LOCK_CACHE_KEY.format(course_key=course_key_string) - pending_key = EXPORT_PENDING_CACHE_KEY.format(course_key=course_key_string) - task_id = task.request.id - - if not cache.add(lock_key, task_id, timeout=EXPORT_LOCK_TIMEOUT): - # Lock is held — check if we are already the designated pending task. - if cache.get(pending_key) == task_id: - log.info( - "Export lock still held for %s, pending task %s retrying in %ds" - " (attempt %d/%d)", - course_key_string, - task_id, - EXPORT_LOCK_RETRY_DELAY, - task.request.retries + 1, - EXPORT_LOCK_MAX_RETRIES, - ) - raise task.retry( - countdown=EXPORT_LOCK_RETRY_DELAY, max_retries=EXPORT_LOCK_MAX_RETRIES - ) - - # Try to become the single designated pending task (atomic). - if cache.add(pending_key, task_id, timeout=EXPORT_PENDING_TIMEOUT): - log.info( - "Export already in progress for %s; task %s queued as pending," - " retrying in %ds", - course_key_string, - task_id, - EXPORT_LOCK_RETRY_DELAY, - ) - raise task.retry( - countdown=EXPORT_LOCK_RETRY_DELAY, max_retries=EXPORT_LOCK_MAX_RETRIES - ) - - # Pending slot already taken — drop this duplicate. - log.info( - "Dropping duplicate export task %s for %s (lock held, pending slot taken)", - task_id, - course_key_string, - ) - return False - - # Lock acquired — clear the pending slot so a fresh task can claim it. - cache.delete(pending_key) - return True - - -def release_export_lock(course_key_string): - """ - Release the per-course git-export distributed lock. - - Must be called in a ``finally`` block after ``acquire_export_lock_or_schedule`` - returns True. - """ - cache.delete(EXPORT_LOCK_CACHE_KEY.format(course_key=course_key_string)) - - def is_auto_repo_creation_enabled(): """ Check if automatic GitHub repository creation is enabled. From c920e41d0cf21b61b98daf320e3091f1703187f4 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Mon, 4 May 2026 15:18:48 +0500 Subject: [PATCH 3/3] chore: version bumped --- src/ol_openedx_git_auto_export/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol_openedx_git_auto_export/pyproject.toml b/src/ol_openedx_git_auto_export/pyproject.toml index ece56d2dc..8dc2ab1ed 100644 --- a/src/ol_openedx_git_auto_export/pyproject.toml +++ b/src/ol_openedx_git_auto_export/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "ol-openedx-git-auto-export" -version = "0.7.1" +version = "0.7.2" description = "A plugin that auto saves the course OLX to git when an author publishes it" authors = [ {name = "MIT Office of Digital Learning"}