Skip to content

Commit 9f1a2f9

Browse files
leoaulasneo98Abdul Rehman
authored andcommitted
feat: implement XqueueViewSet with full xqueue-watcher compatibility
- Add XqueueViewSet with complete xqueue-watcher service compatibility - Implement get_submission service for retrieving pending submissions - Add put_result service with row-level locking (select_for_update(nowait=True)) to prevent race conditions - Ensure concurrent xqueue-watcher instances process each submission exactly once, even under high load - Implement standardized response format for backward compatibility - Add session management and authentication handling for XWatcher clients - Add comprehensive test coverage for core interactions feat: Improve transaction handling to prevent concurrent processing and add timeout mechanism (Get Submission) - Implemented locking to ensure submissions are processed by a single xqueue watcher. - Added timeout mechanism for submissions stuck in 'pulled' state. - Updated tests to cover new error scenarios and timeout handling. feat: Optimize submission retrieval and processing flow - Renamed variables from 'submission_record' to 'external_grader' throughout the   codebase for better consistency with model naming - Added new 'retry' status to integrate with submission processing retry services - Removed unused 'is_processable' method that wasn't providing any value - Enhanced test coverage - Add new status external grader detail migration feat: Add event emission for external grader scores This commit adds the EXTERNAL_GRADER_SCORE_SUBMITTED event emission to the put_result endpoint in the XQueueViewSet. When a grader submits a result and the score is successfully saved, the system now emits an event with all the necessary information for the LMS to render the graded XBlock. Key changes: - Add queue_key field to ExternalGraderDetail model - Include queue_key in create_external_grader_detail method - Emit EXTERNAL_GRADER_SCORE_SUBMITTED event after successful score update - Implement robust false to propagate error and put submission in pending queue again - Add migration for the new queue_key field - Add openedx-events dependency fix: persist queue_key and expose it in ExternalGraderDetail admin This commit fixes grading flow issues caused by the missing `queue_key` in ExternalGraderDetail records and improves overall observability of external grader operations inside the LMS. - `queue_key` was never passed into `ExternalGraderDetail.create_from_uuid`,   causing the field to always be saved as `NULL`. - As a result, the external grading pipeline was unable to deliver correct   score updates because `score_update` requires a valid queue_key. - The LMS received the EXTERNAL_GRADER_SCORE_SUBMITTED event, but the Block could not apply the score, leaving learners stuck on a loading spinner. - Adds `queue_key` to the external grader creation flow so it is properly persisted in the database. - Exposes `queue_key` in ExternalGraderDetailAdmin:   - Added to `list_display`   - Added to `readonly_fields`   - Added to `search_fields`   - Added to admin form fieldsets   - Field remains read-only to avoid accidental modification - External grader responses now carry a valid `queue_key`, allowing XBlocks   to successfully execute `handle_ajax('score_update', ...)` and update scores. - Learners no longer see an infinite loading spinner after submission. - Admins gain full visibility into queue_key for debugging and operational   auditing without risking accidental edits. This enables the event-driven approach for updating XBlocks with scoring data from the edx-submissions service, supporting the gradual migration away from HTTP-based XQueue callbacks.
1 parent d9cc3b6 commit 9f1a2f9

23 files changed

Lines changed: 717 additions & 545 deletions
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
Improving Transaction Handling in Get Submission
2+
================================================
3+
4+
Status
5+
------
6+
7+
**Provisional** *2025-03-19*
8+
9+
Context
10+
-------
11+
12+
We identified a concurrency issue in the submission processing flow. Multiple instances of the xqueue watcher were processing the same transaction simultaneously, causing duplicate handling of submissions. Additionally, submissions remained stuck in the "pulled" state indefinitely when errors or unexpected interruptions occurred during processing.
13+
14+
Decision
15+
--------
16+
17+
We introduced a robust locking mechanism at the database level to prevent concurrent processing of the same submission. Specifically, we employed database row-level locking (`select_for_update(nowait=True)`) on submissions to ensure only one xqueue watcher instance can process a transaction at a time. Additionally, we implemented a timeout-based fallback, marking submissions in the "pulled" state as available again if they exceed a defined timeout period (5 minutes).
18+
19+
Consequences
20+
------------
21+
22+
Positive
23+
~~~~~~~~
24+
25+
- Ensures data consistency by preventing duplicate transaction processing.
26+
- Implements automatic recovery for submissions stuck in the "pulled" state, improving overall system reliability.
27+
- Minimal impact on system performance due to efficient database-level locking.
28+
29+
Negative
30+
~~~~~~~~
31+
32+
- Slightly increased complexity in transaction handling logic.
33+
- Potential minor latency increase when contention occurs due to database locks.

pylintrc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ disable =
275275
unspecified-encoding,
276276
unused-wildcard-import,
277277
use-maxsplit-arg,
278-
278+
279279
logging-fstring-interpolation,
280280

281281
[REPORTS]

requirements/base.txt

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,27 @@
44
#
55
# make upgrade
66
#
7-
asgiref==3.9.1
7+
asgiref==3.10.0
88
# via django
99
django==4.2.25
1010
# via
11-
# -c common_constraints.txt
12-
# -r base.in
11+
# -c requirements/common_constraints.txt
12+
# -r requirements/base.in
1313
# django-model-utils
1414
# djangorestframework
1515
# edx-django-release-util
1616
# jsonfield
1717
django-model-utils==5.0.0
18-
# via -r base.in
18+
# via -r requirements/base.in
1919
djangorestframework==3.16.1
20-
# via -r base.in
20+
# via -r requirements/base.in
2121
edx-django-release-util==1.5.0
22-
# via -r base.in
22+
# via -r requirements/base.in
2323
jsonfield==3.2.0
24-
# via -r base.in
24+
# via -r requirements/base.in
2525
pytz==2025.2
26-
# via -r base.in
27-
pyyaml==6.0.2
26+
# via -r requirements/base.in
27+
pyyaml==6.0.3
2828
# via edx-django-release-util
2929
six==1.17.0
3030
# via edx-django-release-util

requirements/ci.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ cachetools==6.2.0
88
# via
99
# -r requirements/tox.txt
1010
# tox
11-
certifi==2025.8.3
11+
certifi==2025.10.5
1212
# via requests
1313
chardet==5.2.0
1414
# via
@@ -20,7 +20,7 @@ colorama==0.4.6
2020
# via
2121
# -r requirements/tox.txt
2222
# tox
23-
coverage[toml]==7.10.5
23+
coverage[toml]==7.10.7
2424
# via coveralls
2525
coveralls==4.0.1
2626
# via -r requirements/ci.in
@@ -57,7 +57,7 @@ pyproject-api==1.9.1
5757
# tox
5858
requests==2.32.5
5959
# via coveralls
60-
tox==4.28.4
60+
tox==4.30.3
6161
# via -r requirements/tox.txt
6262
urllib3==2.5.0
6363
# via requests

requirements/common_constraints.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,3 @@ Django<5.0
2323
# elastic search changelog: https://www.elastic.co/guide/en/enterprise-search/master/release-notes-7.14.0.html
2424
# See https://github.com/openedx/edx-platform/issues/35126 for more info
2525
elasticsearch<7.14.0
26-
27-
# Cause: https://github.com/openedx/edx-lint/issues/458
28-
# This can be unpinned once https://github.com/openedx/edx-lint/issues/459 has been resolved.
29-
pip<24.3

0 commit comments

Comments
 (0)