From f96a350dd2d644b347eb6e308a44a1c7f36221e5 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Fri, 10 Jul 2026 12:08:01 -0400 Subject: [PATCH 01/10] docs: competency ADR 4 --- .../0004-competency-mastery-concurrency.rst | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst new file mode 100644 index 000000000..1717033e7 --- /dev/null +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -0,0 +1,154 @@ +.. _openedx-learning-adr-0004: + +4. How should learner competency mastery be recorded concurrently and at scale? +================================================================================ + +Status +------ +Proposed (draft). + +Context +------- +When a learner is graded on a subsection, the platform must evaluate whether that grade +demonstrates any attached competencies and record the learner's mastery. Mastery is recorded at +three levels: the criterion (leaf), the criteria group, and the competency. Per +:ref:`openedx-learning-adr-0002`, the group and competency levels are *materialized* (stored), +not recomputed on read, so that dashboards and other read surfaces stay fast. A single grade +change therefore has to recompute and re-write the derived rows from the changed leaf up to the +competency root. Per :ref:`openedx-learning-adr-0003`, every status table is append-only: a change +is a new row, and the current status is the most recent row for a learner and entity. + +Two forces shape how this recording should happen: + +- **Same-learner correctness.** Grade-change events arrive asynchronously and can be delivered out + of order and processed on more than one worker. Because writes are append-only, two evaluations + for the same learner that overlap can each read a stale snapshot of the sibling leaf statuses and + each append a derived roll-up computed from an incomplete picture (a write-skew). Leaf rows are + always correct, since each leaf is a pure function of its own grade; only the derived + group/competency rows can be left wrong. Nothing crashes and no constraint is violated, but a + learner's stored competency status can be silently incorrect. + +- **Throughput.** Grading is bursty and spans a very large number of learners, so the recording + path must keep up without paying a serialization or per-event transaction cost that grows with + the number of learners being graded. + +The question is how to guarantee same-learner correctness while still recording at high aggregate +throughput. + +Decision +-------- +This decision is still open. Two approaches keep same-learner correctness while batching for +throughput. They share the entire pipeline and differ only in how they prevent same-learner +write-skew. The choice turns on expected volume, the event transport, and how much platform-side +coupling is acceptable. + +**Common to both approaches.** Grade-change events are consumed from the openedx-events event bus +and processed in windowed micro-batches. Resolving which competencies a subsection feeds is +learner-independent, so it is cached and de-duplicated across the batch. Each batch does one bulk +read of current statuses, evaluates in memory, and does one bulk append, collapsing per-event +transaction overhead into a small, fixed number of round-trips. The in-memory engine +(:ref:`openedx-learning-adr-0002`) is unchanged: to apply several of one learner's events, the +recorder folds them in edited-timestamp order against an evolving snapshot. Persistence stays +append-only and writes a new row only when the computed status differs from the current one; an +older, out-of-order grade is ignored by comparing its source edited timestamp against the current +leaf's, so a late arrival cannot regress a newer status. + +They differ only in the correctness mechanism. + +**Approach A — correctness by keyed partitioning.** +Events are partitioned by ``user_id``: the key is *hashed* onto a small, fixed number of partitions +(not one partition per learner), so every event for a learner lands on the same partition and is +consumed by exactly one consumer. Same-learner events are processed serially by construction, with +no database lock, while different learners are processed in parallel across partitions. + + - Pros: + - Correctness is structural; no lock and no lock lifecycle. + - The write path scales horizontally with the partition count. + - Cons: + - Relies on the transport routing by key. Kafka does this natively; Redis Streams consumer + groups do not, so on Redis this needs application-level sharding into per-shard streams. + - Couples core to a platform-side contract: the producer must set the partition key and keep + the partition count stable (owned by the platform-side ticket). + - Changing the partition count reshuffles learners and can briefly let two consumers touch + one learner, so it needs an on-demand reconciliation command (Ticket R) as a backstop. + +**Approach B — correctness by a single serializing batch lock.** +One lock guards the whole batch operation, so only one batch runs at a time across the deployment. +The read, evaluation, and append for a batch all happen under that lock, so no two workers ever +evaluate the same learner concurrently. + + - Pros: + - Correctness is total: no write-skew is possible, including across any reshuffle, so no + reconciliation backstop is needed. + - Self-contained: no keyed-partition contract, no platform-side partition key, no Ticket P + or Ticket R. + - Transport-agnostic; behaves the same on Kafka or Redis. + - Cons: + - The write path is a single pipeline: only one batch runs at a time, so recording does not + scale horizontally. Adequate only while one batch pipeline keeps up with peak grading. + - Re-introduces a lock and its lifecycle (acquisition, timeout, stale-lock recovery on + crash), the machinery this epic otherwise removes. + +Deciding factors: + + - **Peak throughput.** A single serialized pipeline (B) is adequate at modest volume; sustained + high-volume, bursty grading favors A's horizontal scaling. + - **Transport.** Kafka gives A its keyed routing for free; on Redis, A needs application-level + sharding, which narrows A's simplicity advantage. + - **Coupling tolerance.** B is self-contained; A trades a platform-side partition contract and a + reconciliation backstop for horizontal scale. + +Rejected Alternatives +--------------------- + +1. Shrink the batch lock to guard only the bulk read and bulk write, running the per-learner + evaluation in parallel outside the lock. + - Motivation: the database I/O is cheap (a few bulk statements), so locking only the I/O and + parallelizing the heavier evaluation looks like it would keep correctness while lifting + Approach B's single-pipeline cap. + - Why rejected: the lock exists to make each learner's read-evaluate-write atomic against other + writers, not to protect the I/O. Moving evaluation outside the lock reintroduces the exact + write-skew from the Context: two workers read the same snapshot for one learner, both + evaluate, and both append a roll-up from an incomplete picture. Making parallel evaluation + correct requires that each learner is only ever handled by one worker at a time, which is + per-learner isolation, i.e. Approach A. This is therefore not a third option: with the + isolation added it is Approach A; without it, it is incorrect. + +2. Per-learner database row lock with per-event recording. + - Pros: + - Correct regardless of delivery order or deployment topology, without depending on how + events are routed or partitioned. + - Conceptually simple and self-contained: it relies only on the database, with no + event-bus partitioning contract. + - Cons: + - Processes one event at a time behind a lock and a transaction, so it does not keep up + under bursty grading across many learners. + - Holds a database connection and a worker for each event while the lock is contended or + waited on. + - Requires an extra per-learner lock table and its locking machinery. + - Does not batch writes, so per-event transaction and commit overhead dominates at volume. + This was the earlier design for this epic; it is correct but the least performant. Both + Approach A and Approach B supersede it: A provides the same same-learner serialization as a + structural property while allowing parallelism and batching, and B provides it with a single + lock and batched writes instead of a per-learner lock and per-event writes. + +3. No lock; assume same-learner conflicts are rare and tolerate them. + - Pros: + - The least machinery of any option: no lock, no partitioning-for-correctness, relying on + append-only self-healing and a reconciliation job. + - Cons: + - Correctness becomes best-effort. A concurrent same-learner conflict can leave a transient + wrong derived status. + - A wrong status lingers indefinitely if the learner receives no further relevant event. + - Unacceptable where mastery feeds credentialing and learner- or instructor-facing + dashboards, in which an incorrect status is directly visible. + +4. Recompute derived levels on read instead of materializing them. + - Pros: + - Removes the write-skew hazard entirely: if nothing derived is stored, nothing derived can + drift, and the write path is trivial. + - Cons: + - Reopens :ref:`openedx-learning-adr-0002`, which deliberately materializes derived levels + for dashboard read performance. + - Moves the cost onto every read, which is the surface that decision was protecting. + - Out of scope for this epic. From 847b789307c6e81805a1ce70196c84b08778735c Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 13 Jul 2026 11:05:03 -0400 Subject: [PATCH 02/10] docs: reject partition-key variant --- .../0004-competency-mastery-concurrency.rst | 139 +++++++++--------- 1 file changed, 69 insertions(+), 70 deletions(-) diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst index 1717033e7..b231a86d1 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -5,7 +5,7 @@ Status ------ -Proposed (draft). +Proposed. Context ------- @@ -37,84 +37,82 @@ throughput. Decision -------- -This decision is still open. Two approaches keep same-learner correctness while batching for -throughput. They share the entire pipeline and differ only in how they prevent same-learner -write-skew. The choice turns on expected volume, the event transport, and how much platform-side -coupling is acceptable. - -**Common to both approaches.** Grade-change events are consumed from the openedx-events event bus -and processed in windowed micro-batches. Resolving which competencies a subsection feeds is -learner-independent, so it is cached and de-duplicated across the batch. Each batch does one bulk -read of current statuses, evaluates in memory, and does one bulk append, collapsing per-event -transaction overhead into a small, fixed number of round-trips. The in-memory engine -(:ref:`openedx-learning-adr-0002`) is unchanged: to apply several of one learner's events, the -recorder folds them in edited-timestamp order against an evolving snapshot. Persistence stays -append-only and writes a new row only when the computed status differs from the current one; an -older, out-of-order grade is ignored by comparing its source edited timestamp against the current -leaf's, so a late arrival cannot regress a newer status. - -They differ only in the correctness mechanism. - -**Approach A — correctness by keyed partitioning.** -Events are partitioned by ``user_id``: the key is *hashed* onto a small, fixed number of partitions -(not one partition per learner), so every event for a learner lands on the same partition and is -consumed by exactly one consumer. Same-learner events are processed serially by construction, with -no database lock, while different learners are processed in parallel across partitions. +Grade-change events are consumed from the openedx-events event bus, processed in windowed +micro-batches, and serialized by a single deployment-wide batch lock, so a learner's competency +mastery is always recorded correctly. Correctness is chosen over horizontal write throughput: +a single serialized pipeline is adequate while it keeps up with peak grading, and it keeps the +recorder self-contained and behaving identically on any Open edX deployment. + +**The recording pipeline.** Resolving which competencies a subsection feeds is learner-independent, +so it is cached and de-duplicated across the batch. Each batch does one bulk read of current +statuses, evaluates in memory, and does one bulk append, collapsing per-event transaction overhead +into a small, fixed number of round-trips. The in-memory engine (:ref:`openedx-learning-adr-0002`) +is unchanged: to apply several of one learner's events, the recorder folds them in edited-timestamp +order against an evolving snapshot. Persistence stays append-only (:ref:`openedx-learning-adr-0003`) +and writes a new row only when the computed status differs from the current one; an older, +out-of-order grade is ignored by comparing its source edited timestamp against the current leaf's, +so a late arrival cannot regress a newer status. + +**Correctness by a single serializing batch lock.** One deployment-wide lock guards the whole batch +operation, so only one batch runs at a time across the deployment. The read, evaluation, and append +for a batch all happen under that lock, so no two workers ever evaluate the same learner +concurrently and the same-learner write-skew described in the Context cannot occur. The lock is +realized on infrastructure every deployment already has (for example, a database-backed advisory +lock) rather than on the event transport, so the recorder behaves the same on any event-bus backend +and adds no new operational dependency. + +Accepted tradeoffs: + + - The write path is a single pipeline: only one batch runs at a time, so recording and evaluating does not + scale horizontally. This is adequate only while one batch pipeline keeps up with peak grading; + sustained high-volume growth would force a revisit. + - It introduces a lock and its lifecycle (acquisition, timeout, stale-lock recovery on crash) + that a lock-free partitioning design would avoid. +Rejected Alternatives +--------------------- + +1. Correctness by keyed partitioning instead of a lock. + Partition grade-change events by ``user_id``: the key is *hashed* onto a small, fixed number of + partitions (not one partition per learner), so every event for a learner lands on the same + partition and is consumed by exactly one consumer. Same-learner events are then processed serially + by construction, with no database lock, while different learners are processed in parallel across + partitions. - Pros: - Correctness is structural; no lock and no lock lifecycle. - The write path scales horizontally with the partition count. - Cons: - Relies on the transport routing by key. Kafka does this natively; Redis Streams consumer - groups do not, so on Redis this needs application-level sharding into per-shard streams. - - Couples core to a platform-side contract: the producer must set the partition key and keep - the partition count stable (owned by the platform-side ticket). + groups do not, so on Redis it needs application-level sharding into per-shard streams. + - Couples openedx-core to a platform-side contract in the wrong direction: the producer, in + openedx-platform, must set the partition key and keep the partition count stable, so the + core recorder's correctness depends on platform-side behavior. - Changing the partition count reshuffles learners and can briefly let two consumers touch - one learner, so it needs an on-demand reconciliation command (Ticket R) as a backstop. - -**Approach B — correctness by a single serializing batch lock.** -One lock guards the whole batch operation, so only one batch runs at a time across the deployment. -The read, evaluation, and append for a batch all happen under that lock, so no two workers ever -evaluate the same learner concurrently. - - - Pros: - - Correctness is total: no write-skew is possible, including across any reshuffle, so no - reconciliation backstop is needed. - - Self-contained: no keyed-partition contract, no platform-side partition key, no Ticket P - or Ticket R. - - Transport-agnostic; behaves the same on Kafka or Redis. - - Cons: - - The write path is a single pipeline: only one batch runs at a time, so recording does not - scale horizontally. Adequate only while one batch pipeline keeps up with peak grading. - - Re-introduces a lock and its lifecycle (acquisition, timeout, stale-lock recovery on - crash), the machinery this epic otherwise removes. - -Deciding factors: - - - **Peak throughput.** A single serialized pipeline (B) is adequate at modest volume; sustained - high-volume, bursty grading favors A's horizontal scaling. - - **Transport.** Kafka gives A its keyed routing for free; on Redis, A needs application-level - sharding, which narrows A's simplicity advantage. - - **Coupling tolerance.** B is self-contained; A trades a platform-side partition contract and a - reconciliation backstop for horizontal scale. - -Rejected Alternatives ---------------------- - -1. Shrink the batch lock to guard only the bulk read and bulk write, running the per-learner + one learner, so it needs an on-demand reconciliation command as a backstop. + - Why rejected: it optimizes for horizontal throughput at the cost of the two properties this + decision values most. It depends on the event transport (Kafka's keyed routing, or Redis-side + sharding that is not an established pattern in vanilla Open edX), so it would not behave + uniformly on a stock deployment, whereas the chosen approach is transport-agnostic. And it + inverts the intended dependency direction by making openedx-core rely on an openedx-platform + partition-key contract plus a reconciliation backstop. The horizontal scale it buys is not + needed while a single batch pipeline keeps up with peak grading, and accuracy is the priority + over the extra latency the single pipeline adds. + +2. Shrink the batch lock to guard only the bulk read and bulk write, running the per-learner evaluation in parallel outside the lock. - Motivation: the database I/O is cheap (a few bulk statements), so locking only the I/O and - parallelizing the heavier evaluation looks like it would keep correctness while lifting - Approach B's single-pipeline cap. + parallelizing the heavier evaluation looks like it would keep correctness while lifting the + chosen approach's single-pipeline cap. - Why rejected: the lock exists to make each learner's read-evaluate-write atomic against other writers, not to protect the I/O. Moving evaluation outside the lock reintroduces the exact write-skew from the Context: two workers read the same snapshot for one learner, both evaluate, and both append a roll-up from an incomplete picture. Making parallel evaluation correct requires that each learner is only ever handled by one worker at a time, which is - per-learner isolation, i.e. Approach A. This is therefore not a third option: with the - isolation added it is Approach A; without it, it is incorrect. + per-learner isolation, i.e. the keyed-partitioning alternative above. This is therefore not a + distinct option: with the isolation added it becomes keyed partitioning; without it, it is + incorrect. -2. Per-learner database row lock with per-event recording. +3. Per-learner database row lock with per-event recording. - Pros: - Correct regardless of delivery order or deployment topology, without depending on how events are routed or partitioned. @@ -127,12 +125,13 @@ Rejected Alternatives waited on. - Requires an extra per-learner lock table and its locking machinery. - Does not batch writes, so per-event transaction and commit overhead dominates at volume. - This was the earlier design for this epic; it is correct but the least performant. Both - Approach A and Approach B supersede it: A provides the same same-learner serialization as a - structural property while allowing parallelism and batching, and B provides it with a single + This was an earlier design for competency mastery recording; it is correct but the least + performant. Both the chosen batch-lock approach and the keyed-partitioning alternative + supersede it: keyed partitioning provides the same same-learner serialization as a structural + property while allowing parallelism and batching, and the batch lock provides it with a single lock and batched writes instead of a per-learner lock and per-event writes. -3. No lock; assume same-learner conflicts are rare and tolerate them. +4. No lock; assume same-learner conflicts are rare and tolerate them. - Pros: - The least machinery of any option: no lock, no partitioning-for-correctness, relying on append-only self-healing and a reconciliation job. @@ -143,7 +142,7 @@ Rejected Alternatives - Unacceptable where mastery feeds credentialing and learner- or instructor-facing dashboards, in which an incorrect status is directly visible. -4. Recompute derived levels on read instead of materializing them. +5. Recompute derived levels on read instead of materializing them. - Pros: - Removes the write-skew hazard entirely: if nothing derived is stored, nothing derived can drift, and the write path is trivial. @@ -151,4 +150,4 @@ Rejected Alternatives - Reopens :ref:`openedx-learning-adr-0002`, which deliberately materializes derived levels for dashboard read performance. - Moves the cost onto every read, which is the surface that decision was protecting. - - Out of scope for this epic. + - Out of scope for this decision. From a08bf5b06001a06098e54449431acb822909b493 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 13 Jul 2026 11:13:25 -0400 Subject: [PATCH 03/10] docs: fix RST indentation breaking readthedocs build Rejected-alternative items 1 and 2 nested Pros/Cons bullet lists directly under a continuation paragraph at a different indent with no blank line separator, and item 3's closing paragraph was indented to neither the list body nor its sub-bullets. docutils flagged these as errors under -W, failing the readthedocs build. --- .../0004-competency-mastery-concurrency.rst | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst index b231a86d1..1e54d1406 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -73,11 +73,13 @@ Rejected Alternatives --------------------- 1. Correctness by keyed partitioning instead of a lock. - Partition grade-change events by ``user_id``: the key is *hashed* onto a small, fixed number of - partitions (not one partition per learner), so every event for a learner lands on the same - partition and is consumed by exactly one consumer. Same-learner events are then processed serially - by construction, with no database lock, while different learners are processed in parallel across - partitions. + + Partition grade-change events by ``user_id``: the key is *hashed* onto a small, fixed number of + partitions (not one partition per learner), so every event for a learner lands on the same + partition and is consumed by exactly one consumer. Same-learner events are then processed serially + by construction, with no database lock, while different learners are processed in parallel across + partitions. + - Pros: - Correctness is structural; no lock and no lock lifecycle. - The write path scales horizontally with the partition count. @@ -100,6 +102,7 @@ Rejected Alternatives 2. Shrink the batch lock to guard only the bulk read and bulk write, running the per-learner evaluation in parallel outside the lock. + - Motivation: the database I/O is cheap (a few bulk statements), so locking only the I/O and parallelizing the heavier evaluation looks like it would keep correctness while lifting the chosen approach's single-pipeline cap. @@ -125,11 +128,12 @@ Rejected Alternatives waited on. - Requires an extra per-learner lock table and its locking machinery. - Does not batch writes, so per-event transaction and commit overhead dominates at volume. - This was an earlier design for competency mastery recording; it is correct but the least - performant. Both the chosen batch-lock approach and the keyed-partitioning alternative - supersede it: keyed partitioning provides the same same-learner serialization as a structural - property while allowing parallelism and batching, and the batch lock provides it with a single - lock and batched writes instead of a per-learner lock and per-event writes. + + This was an earlier design for competency mastery recording; it is correct but the least + performant. Both the chosen batch-lock approach and the keyed-partitioning alternative + supersede it: keyed partitioning provides the same same-learner serialization as a structural + property while allowing parallelism and batching, and the batch lock provides it with a single + lock and batched writes instead of a per-learner lock and per-event writes. 4. No lock; assume same-learner conflicts are rare and tolerate them. - Pros: From 7c77aea2f1bca4b7de79aba24a0a2edb46a01fac Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Fri, 17 Jul 2026 09:59:24 -0400 Subject: [PATCH 04/10] docs: make ADR 4 work without event bus --- .../0004-competency-mastery-concurrency.rst | 212 +++++++++++++++--- 1 file changed, 185 insertions(+), 27 deletions(-) diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst index 1e54d1406..ca390e847 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -5,7 +5,7 @@ Status ------ -Proposed. +Proposed. Contingent on a cross-repo prerequisite (see `Prerequisite`_). Context ------- @@ -18,6 +18,16 @@ change therefore has to recompute and re-write the derived rows from the changed competency root. Per :ref:`openedx-learning-adr-0003`, every status table is append-only: a change is a new row, and the current status is the most recent row for a learner and entity. +How grade changes can reach openedx-core is constrained. openedx-core must never import from +edx-platform and cannot read its grade tables, so grade changes can arrive only as +``openedx-events`` events. The only grade event that exists today is course-level and carries no +subsection identifier, so recording at subsection granularity needs a new event that edx-platform +does not yet emit (see `Prerequisite`_). ``openedx-events`` also delivers in-process by default, +because the event bus is not enabled in a stock deployment: a receiver runs synchronously in the +producer's worker and, in production, a receiver exception is swallowed and logged rather than +retried. Delivery is therefore best-effort, not durable, and the recorder cannot assume the event +bus is present. + Two forces shape how this recording should happen: - **Same-learner correctness.** Grade-change events arrive asynchronously and can be delivered out @@ -33,33 +43,74 @@ Two forces shape how this recording should happen: the number of learners being graded. The question is how to guarantee same-learner correctness while still recording at high aggregate -throughput. +throughput, over best-effort in-process delivery and without making the event bus mandatory. Decision -------- -Grade-change events are consumed from the openedx-events event bus, processed in windowed -micro-batches, and serialized by a single deployment-wide batch lock, so a learner's competency -mastery is always recorded correctly. Correctness is chosen over horizontal write throughput: -a single serialized pipeline is adequate while it keeps up with peak grading, and it keeps the -recorder self-contained and behaving identically on any Open edX deployment. - -**The recording pipeline.** Resolving which competencies a subsection feeds is learner-independent, -so it is cached and de-duplicated across the batch. Each batch does one bulk read of current -statuses, evaluates in memory, and does one bulk append, collapsing per-event transaction overhead -into a small, fixed number of round-trips. The in-memory engine (:ref:`openedx-learning-adr-0002`) -is unchanged: to apply several of one learner's events, the recorder folds them in edited-timestamp -order against an evolving snapshot. Persistence stays append-only (:ref:`openedx-learning-adr-0003`) -and writes a new row only when the computed status differs from the current one; an older, -out-of-order grade is ignored by comparing its source edited timestamp against the current leaf's, -so a late arrival cannot regress a newer status. - -**Correctness by a single serializing batch lock.** One deployment-wide lock guards the whole batch -operation, so only one batch runs at a time across the deployment. The read, evaluation, and append -for a batch all happen under that lock, so no two workers ever evaluate the same learner -concurrently and the same-learner write-skew described in the Context cannot occur. The lock is -realized on infrastructure every deployment already has (for example, a database-backed advisory -lock) rather than on the event transport, so the recorder behaves the same on any event-bus backend -and adds no new operational dependency. +Grade changes are delivered to openedx-core as batched ``openedx-events`` events produced by a +scheduled task on the edx-platform side, buffered into an openedx-core inbox, then recorded in +windowed micro-batches serialized by a single deployment-wide lock. Correctness is chosen over +horizontal write throughput: a single serialized pipeline is adequate while it keeps up with peak +grading, it keeps the recorder self-contained and behaving identically on any Open edX deployment, +and it adds no new mandatory infrastructure. This decision depends on the `Prerequisite`_ below and +stays Proposed until that work lands. + +**Producing grade changes (edx-platform side).** A scheduled task in edx-platform reads the +subsections whose grade changed since a stored ``modified``-timestamp watermark (an indexed range +query on the persisted subsection-grade table, joined with its separately stored overrides to get +the effective grade), selects only the fields the recorder needs, and emits them as an +``openedx-events`` event. Because an ``openedx-events`` payload is conventionally a single entity +and, when the event bus is enabled, is size-capped by the transport, a cycle's rows are split into +bounded, fixed-size batch events rather than one large payload; carrying a list of rows is a +deliberate exception to the single-entity convention, scoped to this producer. Each row carries an +effective source timestamp, computed as the later of the base grade's and the override's +``modified`` time, so an override-only correction, which does not touch the base row, still +registers as newer. + +**Buffering and batching (openedx-core side).** Because delivery is in-process and synchronous, the +receiver does the minimum: it writes the batch's rows into an openedx-core-owned inbox table, +idempotently keyed so a re-delivered row is a no-op, and returns. A separate openedx-core scheduled +task drains the inbox in windowed micro-batches. Keeping the evaluation and the lock out of the +receiver keeps them off edx-platform's scheduler, bounds what a slow or wedged recorder can do to +the producer to "one more row in a table," and makes the durable inbox write, not the evaluation, +the step that must succeed for an event not to be lost. + +**Recording pipeline.** Resolving which competencies a subsection feeds is learner-independent, so +it is cached and de-duplicated across the batch. Each batch does one bulk read of current statuses, +evaluates in memory, and does one bulk append, collapsing per-event transaction overhead into a +small, fixed number of round-trips. The in-memory engine (:ref:`openedx-learning-adr-0002`) is +unchanged: to apply several of one learner's events, the recorder folds them in +effective-source-timestamp order against an evolving snapshot. Persistence stays append-only +(:ref:`openedx-learning-adr-0003`). Two rules govern whether a new status row is written: + + - *Out-of-order defense.* A change is ignored when its effective source timestamp is older than + the current leaf's, so a late arrival cannot regress a newer status. This is a + delivery-ordering guarantee, distinct from the next rule. + - *Advance-only; no automatic regression.* Once a status reaches the demonstrated level it is + retained ("banked"). The recorder appends first attainment and advancements automatically but + does not auto-append a regression below an already-demonstrated status, even for a + legitimately newer downward grade correction. Reversing a banked status is a separate + administrative action, out of scope here. + +**Correctness by a single serializing batch lock.** One deployment-wide lock guards the whole +drain-batch operation, so only one batch runs at a time across the deployment. The read, +evaluation, and append for a batch all happen under that lock, so no two workers ever evaluate the +same learner concurrently and the same-learner write-skew described in the Context cannot occur. +The lock is realized on infrastructure every deployment already has (for example, a database-backed +advisory lock) rather than on the event transport, so the recorder behaves the same on any +event-bus backend and adds no new operational dependency. + +**Durability.** In-process delivery swallows receiver exceptions and the producer's watermark +advances regardless, so a dropped event could otherwise be lost silently. Two low-cost mechanisms +cover this: the producer re-scans a trailing overlap window behind its watermark each cycle, so a +row committed just behind the cursor or missed once is re-emitted, and the consumer is idempotent, +so re-delivery is harmless. An on-demand reconciliation command is provided as an operator escape +hatch for incidents. A permanent scheduled reconciliation sweep is deliberately not included: it is +the kind of always-on cost this decision otherwise avoids, and can be added if real drops are ever +observed. + +**Latency.** Recording is expected to lag grading by minutes, not to be real-time; the dashboards +this feeds tolerate that. The producer's interval is a deployment setting with a documented floor. Accepted tradeoffs: @@ -68,6 +119,19 @@ Accepted tradeoffs: sustained high-volume growth would force a revisit. - It introduces a lock and its lifecycle (acquisition, timeout, stale-lock recovery on crash) that a lock-free partitioning design would avoid. + - Recording lags grading by the producer interval plus drain time; mastery is not updated in + real time. + - It requires new edx-platform code and a new ``openedx-events`` event (see `Prerequisite`_), + which is cross-repo coordination, though it keeps the dependency direction correct. + +Prerequisite +------------ +This decision requires, in edx-platform, the scheduled producer task and the ``openedx-events`` +event or events it emits. Their edx-platform-side design (task location, Celery queue, +retry/backoff, watermark storage, and crash recovery) is out of scope here and belongs in that +companion work. :ref:`openedx-learning-adr-0001` rejected this migration (its rejected alternative +8) as out of scope at the time; this decision takes it up as a now-scheduled prerequisite, and any +project documentation listing the migration as a non-goal is correspondingly superseded. Rejected Alternatives --------------------- @@ -143,8 +207,8 @@ Rejected Alternatives - Correctness becomes best-effort. A concurrent same-learner conflict can leave a transient wrong derived status. - A wrong status lingers indefinitely if the learner receives no further relevant event. - - Unacceptable where mastery feeds credentialing and learner- or instructor-facing - dashboards, in which an incorrect status is directly visible. + - Unacceptable on the learner- and instructor-facing dashboards this feeds, where an + incorrect status is directly visible, and on any future credentialing that consumes it. 5. Recompute derived levels on read instead of materializing them. - Pros: @@ -155,3 +219,97 @@ Rejected Alternatives for dashboard read performance. - Moves the cost onto every read, which is the surface that decision was protecting. - Out of scope for this decision. + +6. Real-time per-subsection events with batching on the openedx-core side. + + edx-platform emits one ``openedx-events`` event per subsection grade change as it happens, and + openedx-core absorbs the per-event stream, buffering and batching on its own side instead of + having the producer batch first. + + - Pros: + - Lower latency: mastery can update within seconds of a grade change rather than within a + polling interval. + - No new scheduled producer task; it reuses the point where the grade is already written. + - Cons: + - The event rate is set by grading volume and scales with however widely competencies are + adopted, which is unknown, so the consumer must be sized for a firehose it cannot bound. + - Each event is an inter-process signal and, on the openedx-core side, a durable write on or + near the hot grading path. + - Why rejected: producer-side batching bounds the event rate at the source regardless of + adoption, so the recorder is scalable by default on a very large deployment without having to + predict how widely competencies will be enabled or whether per-event volume becomes a + problem. Trading a few minutes of latency for that bound is the priority here. + +7. openedx-core polls the persisted grade tables directly. + + Instead of consuming events, openedx-core runs its own periodic query against the persisted + subsection-grade and override tables, using their indexed ``modified`` columns to find changes. + + - Pros: + - Cheapest possible read: the ``modified`` indexes exist for exactly this timespan query, so + it is one indexed range scan per cycle with no per-event cost. + - No dependency on any event being emitted. + - Cons: + - It makes openedx-core depend on edx-platform's private grade schema, an implementation + detail with no stability guarantee, rather than on a versioned contract. + - The effective grade must be recomputed by re-implementing edx-platform's separate override + layering, which will drift as the platform changes it. + - Why rejected: it breaks the layering rule (openedx-core must not depend on edx-platform) and + this decision's value of behaving identically on any deployment. A table schema is an + implementation detail that changes by migration without notice; a versioned event is a + contract. The cost saving is real but does not justify coupling the recorder to platform + internals. + +8. Move the persisted subsection-grade model into openedx-core so edx-platform imports it. + + Follow this repo's established pattern (a library-owned model that edx-platform depends on) by + relocating the subsection-grade model into openedx-core. + + - Pros: + - openedx-core could read the data in-process with no cross-boundary contract at all. + - It is the same ownership pattern openedx-core already uses for content models. + - Cons: + - The subsection-grade model is a large, deeply woven edx-platform model with extensive + signal wiring, override semantics, and migration history. + - Relocating it inverts ownership of a central platform concern and is a major migration. + - Why rejected: that pattern fits models built new in openedx-core, not a mature, central + edx-platform model. The extraction would be a large, risky effort out of all proportion to + recording competency mastery. It may be the right long-term shape if Open edX decides grades + belong in the learning core, but that is a separate, much larger decision. + +9. A shared-contract or swappable grade model. + + openedx-core defines a minimal base model as a contract; edx-platform supplies the concrete + table (with any extra columns it needs) and openedx-core resolves it through a setting, in the + style of Django's swappable ``AUTH_USER_MODEL``. + + - Pros: + - openedx-core reads the data without importing edx-platform, and the platform keeps freedom + to extend its own table. + - Cons: + - Django's swappable-model machinery is, in practice, a one-off for the user model; the + third-party generalizations are niche and carry hard constraints (a swappable model must + exist from the app's first migration, and retrofitting an existing table is unsupported). + - It still couples the platform to an openedx-core-dictated schema, and there is no + precedent for it in this ecosystem beyond the user-model foreign key. + - Why rejected: it would be first-of-its-kind machinery for the project, applied to a table that + already exists and so hits exactly the retrofit constraints the pattern handles worst, for a + benefit a versioned event contract already provides more cleanly. + +10. Enable and rely on the ``openedx-events`` event bus as the delivery mechanism. + + Route grade events over the Kafka or Redis event bus and have openedx-core consume them as a bus + consumer. + + - Pros: + - The bus is a durable buffer with native batch polling and at-least-once delivery, which + would close the durability gap without an inbox or a reconciliation command. + - A bus consumer runs in its own process, off edx-platform's workers. + - Cons: + - The event bus is not enabled in a stock edx-platform deployment. + - Requiring it makes Kafka or Redis mandatory infrastructure for any deployment that wants + competencies. + - Why rejected: mandating event-bus infrastructure is a far larger operational decision than + this feature should force on operators. The chosen design runs over ordinary in-process + signals and is transport-agnostic: it can take advantage of the bus where a deployment already + runs one, but it does not require it. From dc01a87abf9d4bbfb7c09f9b5fee7d6180968e2c Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Fri, 17 Jul 2026 13:38:20 -0400 Subject: [PATCH 05/10] docs: finalize concurrency approach ADR --- ...competency-mastery-concurrency-diagrams.md | 61 ++++++++++++++++ .../0004-competency-mastery-concurrency.rst | 69 ++++++++++++------- 2 files changed, 107 insertions(+), 23 deletions(-) create mode 100644 docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md new file mode 100644 index 000000000..3f80fdda6 --- /dev/null +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md @@ -0,0 +1,61 @@ +# ADR 0004 diagrams: competency mastery recording and concurrency + +Companion diagrams for `0004-competency-mastery-concurrency.rst`. They are kept here as Markdown so +they render natively on GitHub; they are not part of the Sphinx/readthedocs build. Refer to the ADR +for the authoritative decision text. + +## 1. End-to-end recording pipeline + +A scheduled producer on the edx-platform side reads changed subsection grades and emits bounded +batch events. openedx-core receives them in-process, writes them idempotently into an inbox, and +returns. A separate scheduled task drains the inbox in windowed micro-batches under a single +deployment-wide lock, doing bulk reads and bulk writes. + +```mermaid +sequenceDiagram + autonumber + participant PG as edx-platform persisted grades + participant PROD as Scheduled producer (edx-platform) + participant EV as openedx-events + participant RCV as Receiver (openedx-core, in-process) + participant INBOX as Inbox table + participant DRAIN as Scheduled drain task + participant DB as Mastery tables (ACTIVE + HISTORY) + + PROD->>PG: range query by modified watermark (+ trailing overlap window) + PG-->>PROD: changed subsection grades (effective timestamp) + PROD->>EV: bounded, fixed-size batch events + EV->>RCV: in-process delivery + RCV->>INBOX: idempotent write of rows + RCV-->>EV: return (minimal work) + loop each drain cycle + DRAIN->>DRAIN: acquire deployment-wide batch lock + DRAIN->>INBOX: read a window of rows + DRAIN->>DB: bulk read current ACTIVE statuses + DRAIN->>DRAIN: evaluate in memory (fold per learner by effective timestamp) + DRAIN->>DB: bulk upsert ACTIVE + bulk append HISTORY + DRAIN->>DRAIN: release lock + end +``` + +## 2. Batch drain: correctness and performance + +The lock serializes whole batches (not rows), so no two workers evaluate the same learner at once +and the same-learner write-skew cannot occur. Writes are bulk, so the higher per-batch row count of +stored leaves adds bulk-write time within a batch rather than per-row overhead or lock contention +between batches. + +```mermaid +flowchart TD + START["Drain cycle"] --> LOCK{"Acquire deployment-wide batch lock"} + LOCK -->|"not acquired"| SKIP["Skip this cycle"] + LOCK -->|"acquired"| READ["Read inbox window +
bulk read current ACTIVE statuses"] + READ --> FOLD["Per learner: fold events in
effective-source-timestamp order"] + FOLD --> RULES["Per leaf, apply two rules"] + RULES --> OOO["Out-of-order: ignore if older
than current leaf timestamp"] + RULES --> ADV["Advance-only: never regress a banked
status; suppressed events go to HISTORY"] + OOO --> WRITE["Bulk upsert ACTIVE +
bulk append HISTORY
(leaf and rolled-up levels)"] + ADV --> WRITE + WRITE --> UNLOCK["Release lock"] + UNLOCK --> DUR["Durability: producer overlap re-scan +
idempotent inbox make re-delivery safe"] +``` diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst index ca390e847..df4563aa2 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -7,16 +7,21 @@ Status ------ Proposed. Contingent on a cross-repo prerequisite (see `Prerequisite`_). +A companion set of diagrams for this decision lives alongside it in +``0004-competency-mastery-concurrency-diagrams.md``. + Context ------- When a learner is graded on a subsection, the platform must evaluate whether that grade demonstrates any attached competencies and record the learner's mastery. Mastery is recorded at three levels: the criterion (leaf), the criteria group, and the competency. Per -:ref:`openedx-learning-adr-0002`, the group and competency levels are *materialized* (stored), -not recomputed on read, so that dashboards and other read surfaces stay fast. A single grade -change therefore has to recompute and re-write the derived rows from the changed leaf up to the -competency root. Per :ref:`openedx-learning-adr-0003`, every status table is append-only: a change -is a new row, and the current status is the most recent row for a learner and entity. +:ref:`openedx-learning-adr-0002` and :ref:`openedx-learning-adr-0005`, all three levels are +*materialized* (stored), not recomputed on read, so that dashboards and other read surfaces stay +fast. A single grade change therefore writes the changed leaf's status and then re-evaluates and +re-writes the derived rows from that leaf up to the competency root. Per +:ref:`openedx-learning-adr-0005`, each level is stored as an ACTIVE row updated in place, holding +the current status for a learner and node, plus an append-only HISTORY row per applied change; this +supersedes :ref:`openedx-learning-adr-0003`'s original all-append-only model. How grade changes can reach openedx-core is constrained. openedx-core must never import from edx-platform and cannot read its grade tables, so grade changes can arrive only as @@ -31,12 +36,12 @@ bus is present. Two forces shape how this recording should happen: - **Same-learner correctness.** Grade-change events arrive asynchronously and can be delivered out - of order and processed on more than one worker. Because writes are append-only, two evaluations - for the same learner that overlap can each read a stale snapshot of the sibling leaf statuses and - each append a derived roll-up computed from an incomplete picture (a write-skew). Leaf rows are - always correct, since each leaf is a pure function of its own grade; only the derived - group/competency rows can be left wrong. Nothing crashes and no constraint is violated, but a - learner's stored competency status can be silently incorrect. + of order and processed on more than one worker. Two evaluations for the same learner that overlap + can each read a stale snapshot of the sibling leaf statuses and each write a derived roll-up + computed from an incomplete picture (a write-skew). Leaf rows are always correct, since each leaf + is a pure function of its own grade; only the derived group/competency rows can be left wrong. + Nothing crashes and no constraint is violated, but a learner's stored competency status can be + silently incorrect. - **Throughput.** Grading is bursty and spans a very large number of learners, so the recording path must keep up without paying a serialization or per-event transaction cost that grows with @@ -76,21 +81,26 @@ the producer to "one more row in a table," and makes the durable inbox write, no the step that must succeed for an event not to be lost. **Recording pipeline.** Resolving which competencies a subsection feeds is learner-independent, so -it is cached and de-duplicated across the batch. Each batch does one bulk read of current statuses, -evaluates in memory, and does one bulk append, collapsing per-event transaction overhead into a -small, fixed number of round-trips. The in-memory engine (:ref:`openedx-learning-adr-0002`) is -unchanged: to apply several of one learner's events, the recorder folds them in -effective-source-timestamp order against an evolving snapshot. Persistence stays append-only -(:ref:`openedx-learning-adr-0003`). Two rules govern whether a new status row is written: +it is cached and de-duplicated across the batch. Each batch does one bulk read of current ACTIVE +statuses, evaluates in memory, and then, in a small fixed number of round-trips, bulk-upserts the +changed ACTIVE rows and bulk-appends the HISTORY rows, at the leaf level and every rolled-up level +it touches. The number of round-trips per batch is fixed regardless of how many leaves the batch +covers, which is what keeps the higher per-batch row count of stored leaves (see the stored-leaf +performance note below) from turning into per-row overhead. The in-memory engine +(:ref:`openedx-learning-adr-0002`) is unchanged: to apply several of one learner's events, the +recorder folds them in effective-source-timestamp order against an evolving snapshot. Persistence +follows :ref:`openedx-learning-adr-0005`'s ACTIVE-plus-HISTORY split. Two rules govern whether a +status is advanced and a HISTORY row written: - *Out-of-order defense.* A change is ignored when its effective source timestamp is older than the current leaf's, so a late arrival cannot regress a newer status. This is a delivery-ordering guarantee, distinct from the next rule. - *Advance-only; no automatic regression.* Once a status reaches the demonstrated level it is - retained ("banked"). The recorder appends first attainment and advancements automatically but - does not auto-append a regression below an already-demonstrated status, even for a - legitimately newer downward grade correction. Reversing a banked status is a separate - administrative action, out of scope here. + retained ("banked"), at every level including the leaf (:ref:`openedx-learning-adr-0005`). The + recorder records first attainment and advancements automatically but does not regress a banked + status below the demonstrated level, even for a legitimately newer downward grade correction; a + suppressed downward correction is still written to HISTORY as seen-and-suppressed. Reversing a + banked status is a separate administrative action, out of scope here. **Correctness by a single serializing batch lock.** One deployment-wide lock guards the whole drain-batch operation, so only one batch runs at a time across the deployment. The read, @@ -112,6 +122,19 @@ observed. **Latency.** Recording is expected to lag grading by minutes, not to be real-time; the dashboards this feeds tolerate that. The producer's interval is a deployment setting with a documented floor. +**Performance of stored leaves.** :ref:`openedx-learning-adr-0005` stores leaf mastery, so a batch +now writes on the order of the per-course leaf fan-out (up to ~200x) more rows than a +group-and-competency-only design would. This does not change the shape of the pipeline: the writes +are bulk (one bulk upsert of ACTIVE rows and one bulk append of HISTORY rows per level), so the +number of statements and the time held under the lock scale with batch size, not with the number of +learners or leaves inside a statement. The lock serializes whole batches, not rows, so the extra +rows add bulk-write time within a batch rather than lock contention between batches. The single +serialized pipeline remains adequate while one batch keeps up with peak grading; the higher leaf +write volume lowers the headroom before that ceiling relative to the transient-leaf design, and the +sustained-growth revisit noted below applies all the more. Batch size is the primary tuning knob, +and the mass recompute from a structural criteria edit is chunked and rate-limited +(:ref:`openedx-learning-adr-0005`) so it cannot crowd out live recording. + Accepted tradeoffs: - The write path is a single pipeline: only one batch runs at a time, so recording and evaluating does not @@ -173,7 +196,7 @@ Rejected Alternatives - Why rejected: the lock exists to make each learner's read-evaluate-write atomic against other writers, not to protect the I/O. Moving evaluation outside the lock reintroduces the exact write-skew from the Context: two workers read the same snapshot for one learner, both - evaluate, and both append a roll-up from an incomplete picture. Making parallel evaluation + evaluate, and both write a roll-up from an incomplete picture. Making parallel evaluation correct requires that each learner is only ever handled by one worker at a time, which is per-learner isolation, i.e. the keyed-partitioning alternative above. This is therefore not a distinct option: with the isolation added it becomes keyed partitioning; without it, it is @@ -202,7 +225,7 @@ Rejected Alternatives 4. No lock; assume same-learner conflicts are rare and tolerate them. - Pros: - The least machinery of any option: no lock, no partitioning-for-correctness, relying on - append-only self-healing and a reconciliation job. + eventual self-healing on the next event and a reconciliation job. - Cons: - Correctness becomes best-effort. A concurrent same-learner conflict can leave a transient wrong derived status. From 808a228563ee7885ae2aef4233b446c91b69a5f9 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Fri, 17 Jul 2026 14:49:55 -0400 Subject: [PATCH 06/10] docs: concurrency and storage docs and diagrams --- .../0002-competency-criteria-model-diagram.md | 150 ++++++++ .../0002-competency-criteria-model.rst | 127 +++++-- .../0003-competency-criteria-versioning.rst | 24 +- ...competency-mastery-concurrency-diagrams.md | 2 +- .../0004-competency-mastery-concurrency.rst | 12 +- ...005-competency-mastery-storage-diagrams.md | 90 +++++ .../0005-competency-mastery-storage.rst | 352 ++++++++++++++++++ 7 files changed, 709 insertions(+), 48 deletions(-) create mode 100644 docs/openedx_learning/decisions/0002-competency-criteria-model-diagram.md create mode 100644 docs/openedx_learning/decisions/0005-competency-mastery-storage-diagrams.md create mode 100644 docs/openedx_learning/decisions/0005-competency-mastery-storage.rst diff --git a/docs/openedx_learning/decisions/0002-competency-criteria-model-diagram.md b/docs/openedx_learning/decisions/0002-competency-criteria-model-diagram.md new file mode 100644 index 000000000..911272b29 --- /dev/null +++ b/docs/openedx_learning/decisions/0002-competency-criteria-model-diagram.md @@ -0,0 +1,150 @@ +# Competency data model (updated) + +The authoritative data model for CBE competency criteria and learner mastery. It builds on the +original `images/CompetencyCriteriaModel.png` overview, corrected against ADRs 0001-0003 (which are +the source of truth where the PNG diverges) and updated for the storage decisions in +`0005-competency-mastery-storage.rst`. + +Notable differences from the PNG: + +- Each learner-status level is now two tables: an **ACTIVE** table (one current row per learner and + node, updated in place) and an append-only **HISTORY** table (one row per genuine status advance, + bounded by monotonicity). The PNG showed a single table per level. +- The `mastery_level_id` column the PNG drew on `StudentCompetencyCriteriaStatus` is **not** part of + the model (mastery level is a future rule type; see ADR 0002). +- Learner-status tables use 64-bit primary keys and are reachable through a dedicated database alias + (defaulting to the main database). Their references to the definition tables and to the user are + **logical foreign keys without database-level constraints**, so the tables can live in a separate + database (see ADR 0005). + +Existing tagging tables (`oel_tagging_*`) are shown minimally, only enough to anchor the +relationships they participate in. + +```mermaid +erDiagram + oel_tagging_taxonomy ||--|| CompetencyTaxonomy : "MTI subtype" + oel_tagging_taxonomy ||--o{ oel_tagging_tag : "taxonomy_id" + oel_tagging_tag ||--o{ oel_tagging_objecttag : "tag_id" + CompetencyTaxonomy ||--o{ CompetencyRuleProfile : "scopes" + oel_tagging_tag ||--o{ CompetencyCriteriaGroup : "competency" + CompetencyCriteriaGroup ||--o{ CompetencyCriteriaGroup : "parent_id (nesting)" + CompetencyCriteriaGroup ||--o{ CompetencyCriteria : "contains" + oel_tagging_objecttag ||--o{ CompetencyCriteria : "association" + CompetencyRuleProfile ||--o{ CompetencyCriteria : "default rule" + CompetencyCriteria ||--o{ StudentCompetencyCriteriaStatus : "leaf ACTIVE (no DB FK)" + StudentCompetencyCriteriaStatus ||--o{ StudentCompetencyCriteriaStatusHistory : "advances (same key)" + CompetencyCriteriaGroup ||--o{ StudentCompetencyCriteriaGroupStatus : "group ACTIVE (no DB FK)" + StudentCompetencyCriteriaGroupStatus ||--o{ StudentCompetencyCriteriaGroupStatusHistory : "advances (same key)" + oel_tagging_tag ||--o{ StudentCompetencyStatus : "competency ACTIVE (no DB FK)" + StudentCompetencyStatus ||--o{ StudentCompetencyStatusHistory : "advances (same key)" + CompetencyMasteryStatuses ||--o{ StudentCompetencyCriteriaStatus : "status_id" + CompetencyMasteryStatuses ||--o{ StudentCompetencyCriteriaGroupStatus : "status_id" + CompetencyMasteryStatuses ||--o{ StudentCompetencyStatus : "status_id" + + oel_tagging_taxonomy { + int id PK + } + oel_tagging_tag { + int id PK + int taxonomy_id FK + } + oel_tagging_objecttag { + int id PK + int tag_id FK + } + CompetencyTaxonomy { + int taxonomy_ptr_id PK "1:1 to oel_tagging_taxonomy" + } + CompetencyRuleProfile { + int id PK + int organization_id "nullable scope" + varchar course_id "nullable scope" + int competency_taxonomy_id FK "nullable scope" + varchar rule_type "Grade (View, MasteryLevel future)" + json rule_payload + } + CompetencyCriteriaGroup { + int id PK + int parent_id FK "self; null = root" + int oel_tagging_tag_id FK "competency" + varchar course_id "nullable course scope" + varchar name + int ordering + varchar logic_operator "AND / OR / null" + } + CompetencyCriteria { + int id PK + int competency_criteria_group_id FK + int oel_tagging_objecttag_id FK + int competency_rule_profile_id FK "nullable" + varchar rule_type_override "nullable" + json rule_payload_override "nullable" + } + CompetencyMasteryStatuses { + int id PK + varchar status UK "Demonstrated / AttemptedNotDemonstrated / PartiallyAttempted" + } + StudentCompetencyCriteriaStatus { + bigint id PK "64-bit" + int user_id "logical FK, no DB constraint" + int competency_criteria_id "logical FK, no DB constraint" + int status_id FK "leaf: Demonstrated / AttemptedNotDemonstrated" + datetime effective_source_timestamp + datetime created + datetime modified + } + StudentCompetencyCriteriaStatusHistory { + bigint id PK "64-bit" + int user_id + int competency_criteria_id + int status_id FK + datetime effective_source_timestamp + datetime created "one row per advance" + } + StudentCompetencyCriteriaGroupStatus { + bigint id PK "64-bit" + int user_id "logical FK, no DB constraint" + int competency_criteria_group_id "logical FK, no DB constraint" + int status_id FK + datetime effective_source_timestamp + datetime created + datetime modified + } + StudentCompetencyCriteriaGroupStatusHistory { + bigint id PK "64-bit" + int user_id + int competency_criteria_group_id + int status_id FK + datetime effective_source_timestamp + datetime created "one row per advance" + } + StudentCompetencyStatus { + bigint id PK "64-bit" + int user_id "logical FK, no DB constraint" + int oel_tagging_tag_id "logical FK, no DB constraint" + int status_id FK "Demonstrated / PartiallyAttempted only" + datetime effective_source_timestamp + datetime created + datetime modified + } + StudentCompetencyStatusHistory { + bigint id PK "64-bit" + int user_id + int oel_tagging_tag_id + int status_id FK + datetime effective_source_timestamp + datetime created "one row per advance" + } +``` + +Notes on the diagram: + +- Each ACTIVE table is **unique on `(user_id, node_id)`** (one current row per learner and node): + `(user_id, competency_criteria_id)`, `(user_id, competency_criteria_group_id)`, and + `(user_id, oel_tagging_tag_id)` respectively. HISTORY tables have no such uniqueness; they carry + one row per advance. +- The ACTIVE-to-HISTORY relationship is drawn as one-to-many but is not a literal foreign key: a + HISTORY row shares the ACTIVE row's `(user_id, node_id)` identity rather than pointing at its + primary key. +- `status_id` references the shared `CompetencyMasteryStatuses` lookup so status semantics live in + one place. diff --git a/docs/openedx_learning/decisions/0002-competency-criteria-model.rst b/docs/openedx_learning/decisions/0002-competency-criteria-model.rst index b41dca757..ef124f6ae 100644 --- a/docs/openedx_learning/decisions/0002-competency-criteria-model.rst +++ b/docs/openedx_learning/decisions/0002-competency-criteria-model.rst @@ -155,29 +155,58 @@ Decision 3. ``oel_tagging_objecttag(object_id)`` 4. ``CompetencyCriteria(oel_tagging_objecttag_id)`` 5. ``CompetencyCriteria(competency_criteria_group_id)`` - 6. ``StudentCompetencyCriteriaStatus(user_id, competency_criteria_id)`` - 7. ``StudentCompetencyCriteriaGroupStatus(user_id, competency_criteria_group_id)`` - 8. ``StudentCompetencyStatus(user_id, oel_tagging_tag_id)`` - 9. ``CompetencyRuleProfile(competency_taxonomy_id, course_id, organization_id)`` - 10. ``CompetencyMasteryStatuses(status)`` (unique) + 6. ``StudentCompetencyCriteriaStatus(user_id, competency_criteria_id)`` (unique; ACTIVE current row) + 7. ``StudentCompetencyCriteriaGroupStatus(user_id, competency_criteria_group_id)`` (unique; ACTIVE current row) + 8. ``StudentCompetencyStatus(user_id, oel_tagging_tag_id)`` (unique; ACTIVE current row) + 9. ``StudentCompetencyCriteriaStatusHistory(user_id, competency_criteria_id, created)`` (point-in-time reconstruction) + 10. ``StudentCompetencyCriteriaGroupStatusHistory(user_id, competency_criteria_group_id, created)`` + 11. ``StudentCompetencyStatusHistory(user_id, oel_tagging_tag_id, created)`` + 12. ``CompetencyRuleProfile(competency_taxonomy_id, course_id, organization_id)`` + 13. ``CompetencyMasteryStatuses(status)`` (unique) 6. Learner progress status concepts (``StudentCompetency*Status`` database tables) - When a completion event (graded, completed, mastered, etc.) occurs for an object, determine and track the learner's status in earning the competency. To reduce recalculation frequency, store results at each level. + When a completion event (a subsection grade change, delivered as described in + :ref:`openedx-learning-adr-0004`) is evaluated for an object, determine and track the learner's + status in earning the competency. Results are stored at each level (leaf, group, competency) so + reads do not recompute. Each level is stored as two tables: an ACTIVE table holding one current + row per learner and node, updated in place, and an append-only HISTORY table that records one row + per genuine status advance. Current status is a direct lookup on the ACTIVE row; the HISTORY table + is the audit and point-in-time record. Because mastery is monotonic + (:ref:`openedx-learning-adr-0005`), the number of advances per node is bounded by the status + lattice, so the HISTORY tables grow with learners and nodes, not with time. + + These are the large per-learner tables in this model, so they follow the scale posture of + :ref:`openedx-learning-adr-0005` (mirroring edx-platform's persistent grades): a 64-bit + ``BigAutoField`` primary key, narrow rows, per-read-path composite indexes (Decision 5), and a + dedicated database alias reached through a router that defaults to the main database. Their foreign + keys to the criteria-definition tables and to the user are logical references declared without + database-level constraints, so the tables can live in a different database from the definitions; + referential integrity and the delete protection of Decision 7 are enforced in application code. Relationship to other concepts: - - ``StudentCompetencyCriteriaStatus`` tracks status at ``CompetencyCriterion`` leaf level. - - ``StudentCompetencyCriteriaGroupStatus`` tracks status at ``CompetencyCriteriaGroup`` node level. - - ``StudentCompetencyStatus`` tracks top-level competency demonstration state. - - All learner status rows use a shared lookup table (``CompetencyMasteryStatuses``) so status semantics live in one place and student status tables stay structurally consistent. - - Intended update flow (bottom-up materialization): - - - A learner event updates one ``StudentCompetencyCriteriaStatus`` row. - - Recompute ancestor ``CompetencyCriteriaGroup`` statuses upward to the root. - - At each group, evaluate children in ``ordering`` sequence and short-circuit when the group's result is already determined by its ``logic_operator``. - - Persist only rows whose status changed. + - ``StudentCompetencyCriteriaStatus`` / ``StudentCompetencyCriteriaStatusHistory`` track status at + the ``CompetencyCriterion`` leaf level. A leaf is atomic, so its status is only ``Demonstrated`` + or ``AttemptedNotDemonstrated``. + - ``StudentCompetencyCriteriaGroupStatus`` / ``StudentCompetencyCriteriaGroupStatusHistory`` track + status at the ``CompetencyCriteriaGroup`` node level. + - ``StudentCompetencyStatus`` / ``StudentCompetencyStatusHistory`` track top-level competency + demonstration state. + - All learner status rows use a shared lookup table (``CompetencyMasteryStatuses``) so status + semantics live in one place and the status tables stay structurally consistent. + + Intended update flow (bottom-up roll-up): + + - A learner event evaluates one leaf; if its status advances, its ACTIVE row is updated in place + and a leaf HISTORY row is appended. + - Recompute ancestor ``CompetencyCriteriaGroup`` statuses upward to the root, reading the stored + child rows directly. + - At each group, evaluate children in ``ordering`` sequence and short-circuit when the group's + result is already determined by its ``logic_operator``. + - Persist only nodes whose status changed: update the ACTIVE row in place and append a HISTORY row. + A banked (``Demonstrated``) status is never regressed, so a downward grade correction advances + nothing and writes no HISTORY row. 1. Add new database table for ``CompetencyMasteryStatuses`` with these columns: @@ -188,34 +217,51 @@ Decision - This table is system-owned lookup data and should be treated as immutable configuration, not user-authored rows. - 2. Add new database table for ``StudentCompetencyCriteriaStatus`` with these columns: + 2. Add the leaf-level tables ``StudentCompetencyCriteriaStatus`` (ACTIVE) and + ``StudentCompetencyCriteriaStatusHistory`` (HISTORY). Both tables have these columns: - 1. ``id``: unique primary key - 2. ``competency_criteria_id``: Foreign key to ``CompetencyCriterion.id`` - 3. ``user_id``: Foreign key pointing to user_id (presumably the learner's id, although it appears that it is possible for staff to get grades as well) in ``auth_user`` table - 4. ``status_id``: Foreign key to ``CompetencyMasteryStatuses.id`` - 5. ``created``: The timestamp at which the student's criterion status was set. + 1. ``id``: 64-bit ``BigAutoField`` primary key + 2. ``competency_criteria_id``: logical foreign key to ``CompetencyCriterion.id`` (no database-level constraint) + 3. ``user_id``: logical foreign key to the learner's user id (no database-level constraint) + 4. ``status_id``: foreign key to ``CompetencyMasteryStatuses.id`` + 5. ``effective_source_timestamp``: the timestamp of the grade change this status was computed from, used for the out-of-order defense in :ref:`openedx-learning-adr-0004` + 6. ``created``: on ACTIVE, when the row was first written; on HISTORY, when this advance was appended - 3. Add a new database table for ``StudentCompetencyCriteriaGroupStatus`` with these columns: + The ACTIVE table additionally has a ``modified`` timestamp (last in-place update) and is unique + on ``(user_id, competency_criteria_id)`` (one current row per learner and leaf). The HISTORY + table has no such uniqueness constraint: it holds one row per advance and is not written for a + suppressed downward correction. - 1. ``id``: unique primary key - 2. ``competency_criteria_group_id``: Foreign key to ``CompetencyCriteriaGroup.id`` - 3. ``user_id``: Foreign key pointing to user_id (presumably the learner's id, although it appears that it is possible for staff to get grades as well) in ``auth_user`` table - 4. ``status_id``: Foreign key to ``CompetencyMasteryStatuses.id`` - 5. ``created``: The timestamp at which the student's criteria-group status was set. + 3. Add the group-level tables ``StudentCompetencyCriteriaGroupStatus`` (ACTIVE) and + ``StudentCompetencyCriteriaGroupStatusHistory`` (HISTORY). Both tables have these columns: - 4. Add a new database table for ``StudentCompetencyStatus`` with these columns: + 1. ``id``: 64-bit ``BigAutoField`` primary key + 2. ``competency_criteria_group_id``: logical foreign key to ``CompetencyCriteriaGroup.id`` (no database-level constraint) + 3. ``user_id``: logical foreign key to the learner's user id (no database-level constraint) + 4. ``status_id``: foreign key to ``CompetencyMasteryStatuses.id`` + 5. ``effective_source_timestamp``: as in the leaf tables + 6. ``created``: as in the leaf tables - 1. ``id``: unique primary key - 2. ``oel_tagging_tag_id``: Foreign key pointing to Tag id - 3. ``user_id``: Foreign key pointing to user_id (presumably the learner's id, although it appears that it is possible for staff to get grades as well) in ``auth_user`` table - 4. ``status_id``: Foreign key to ``CompetencyMasteryStatuses.id``. This table should have a constraint to only allow status values of “Demonstrated” and “PartiallyAttempted” since it represents overall competency demonstration state, not in-progress states. - 5. ``created``: The timestamp at which the student's competency status was set. + The ACTIVE table additionally has a ``modified`` timestamp and is unique on + ``(user_id, competency_criteria_group_id)``. + + 4. Add the competency-level tables ``StudentCompetencyStatus`` (ACTIVE) and + ``StudentCompetencyStatusHistory`` (HISTORY). Both tables have these columns: + + 1. ``id``: 64-bit ``BigAutoField`` primary key + 2. ``oel_tagging_tag_id``: logical foreign key to the competency ``Tag`` id (no database-level constraint) + 3. ``user_id``: logical foreign key to the learner's user id (no database-level constraint) + 4. ``status_id``: foreign key to ``CompetencyMasteryStatuses.id``. Constrained to “Demonstrated” and “PartiallyAttempted” only, since this represents overall competency demonstration state, not in-progress states. + 5. ``effective_source_timestamp``: as in the leaf tables + 6. ``created``: as in the leaf tables + + The ACTIVE table additionally has a ``modified`` timestamp and is unique on + ``(user_id, oel_tagging_tag_id)``. 7. Delete protection boundaries - If no learner status rows exist for a competency definition, hard delete is allowed and cascades through competency metadata tables. - - Once any related learner status exists in ``StudentCompetencyStatus``, ``StudentCompetencyCriteriaGroupStatus``, or ``StudentCompetencyCriteriaStatus``, deletion of associated competency definition rows is blocked. + - Once any related learner status exists (an ACTIVE or HISTORY row at any level), deletion of associated competency definition rows is blocked. Because the learner-status foreign keys carry no database-level constraint (Decision 6), this protection is enforced in application code, not by a database cascade or constraint. - This delete protection applies to ``oel_tagging_taxonomy``, ``CompetencyTaxonomy``, ``oel_tagging_tag``, ``oel_tagging_objecttag``, ``CompetencyCriteriaGroup``, ``CompetencyCriteria``, and ``CompetencyRuleProfile``. - Once any related learner status exists, retiring definitions may be archive-only (hidden from authoring and new associations), not hard delete. @@ -224,6 +270,15 @@ Decision :width: 80% :align: center +.. note:: + + The PNG above is the original overview. It predates the ACTIVE/HISTORY split introduced here and + in :ref:`openedx-learning-adr-0005`, and it shows a ``mastery_level_id`` column on + ``StudentCompetencyCriteriaStatus`` that is not part of this decision (mastery level is a future + rule type). The current data model, including the ACTIVE and HISTORY tables, is diagrammed in + ``0002-competency-criteria-model-diagram.md``, which is authoritative where it differs from the + PNG. + Example ------- diff --git a/docs/openedx_learning/decisions/0003-competency-criteria-versioning.rst b/docs/openedx_learning/decisions/0003-competency-criteria-versioning.rst index 60f4f5864..9edca17f8 100644 --- a/docs/openedx_learning/decisions/0003-competency-criteria-versioning.rst +++ b/docs/openedx_learning/decisions/0003-competency-criteria-versioning.rst @@ -39,15 +39,15 @@ For the initial implementation, versioning and traceability of competency achiev 4. Authoring guardrails must warn on potentially impactful edits: - - If a user edits competency criteria definitions or competency object/tag associations after related learner status exists, Studio must display an explicit warning that student statuses have already been set, and these changes will be applied going forward, so existing learner statuses will not be retroactively updated. + - If a user edits competency criteria definitions or competency object/tag associations after related learner status exists, Studio must display an explicit warning that student statuses have already been set. Such edits are monotonic for the learner (:ref:`openedx-learning-adr-0005`): they never lower or revoke an existing status; a structural edit triggers a recompute that can only raise statuses (for example, eased criteria a learner already meets); and a rule or threshold change takes effect only going forward, as new grades arrive. - Applying these changes requires explicit user confirmation. -5. Learner status models/tables are append-only history and do not use ``django-simple-history``: +5. Learner status is stored as an ACTIVE table plus an append-only HISTORY table, using explicit tables rather than ``django-simple-history``: - - For ``StudentCompetencyCriteriaStatus``, ``StudentCompetencyCriteriaGroupStatus``, and ``StudentCompetencyStatus``, each status change is stored as a new row with ``created`` as the write timestamp. - - Existing learner status rows are not updated in place. - - Current status is determined by the most recent row for a given learner + target entity (ordered by ``created``, with ``id`` as a tie-breaker). - - Older rows represent the learner status history and remain available for audit/tracing. + - For each level (``StudentCompetencyCriteriaStatus``, ``StudentCompetencyCriteriaGroupStatus``, and ``StudentCompetencyStatus``), an ACTIVE table holds one current row per learner and node, updated in place, and a parallel append-only HISTORY table records one row per genuine status advance. The columns are defined in :ref:`openedx-learning-adr-0002`, Decision 6. + - Current status is a direct lookup on the ACTIVE row, not the most recent of many rows. + - A HISTORY row is written only when a status advances. Because mastery is monotonic and advance-only (:ref:`openedx-learning-adr-0005`), the number of advances per node is bounded by the status lattice, so HISTORY grows with learners and nodes, not with time, and still supports point-in-time reconstruction: the status at any past time is the latest advance at or before that time. + - These tables do not use ``django-simple-history``. The advance-only HISTORY is written explicitly by the recorder (:ref:`openedx-learning-adr-0004`) only on a genuine advance, and the recorder writes the ACTIVE and HISTORY tables in bulk. ``django-simple-history`` snapshots on every ``save`` and is oriented to per-row saves, which fits neither the advance-only bounding nor the bulk write path, and it adds history-metadata columns and per-save signal overhead this decision does not want on billion-row tables. It remains the right tool for the low-volume definition tables in Decision 1. Rejected Alternatives @@ -83,3 +83,15 @@ Rejected Alternatives - Cons: - Requires custom tooling to reconstruct past versions - Does not align with existing publishable versioning patterns +5. Append-only learner status only, with current status as the most recent row (the earlier form of this decision, before the ACTIVE/HISTORY split). + - Pros: + - No in-place mutation; a single table per level. + - Cons: + - Current-status reads must resolve the latest of several rows instead of a single-row lookup, on the dashboard hot path. + - There is no in-place current row to anchor per-learner concurrency (:ref:`openedx-learning-adr-0004`). +6. Use ``django-simple-history`` for the learner status tables (as Decision 1 does for definition tables). + - Pros: + - Automatic shadow-table history with no hand-written HISTORY writes, consistent with the definition tables. + - Cons: + - It snapshots on every ``save``, so it cannot express advance-only HISTORY without extra suppression logic, and it does not fit the recorder's bulk write path. + - It adds history-metadata columns and per-save signal overhead that this decision avoids on billion-row tables. diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md index 3f80fdda6..c17a682d9 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md @@ -53,7 +53,7 @@ flowchart TD READ --> FOLD["Per learner: fold events in
effective-source-timestamp order"] FOLD --> RULES["Per leaf, apply two rules"] RULES --> OOO["Out-of-order: ignore if older
than current leaf timestamp"] - RULES --> ADV["Advance-only: never regress a banked
status; suppressed events go to HISTORY"] + RULES --> ADV["Advance-only: never regress a banked
status; downward corrections write no HISTORY row"] OOO --> WRITE["Bulk upsert ACTIVE +
bulk append HISTORY
(leaf and rolled-up levels)"] ADV --> WRITE WRITE --> UNLOCK["Release lock"] diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst index df4563aa2..21d431c94 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -20,8 +20,9 @@ three levels: the criterion (leaf), the criteria group, and the competency. Per fast. A single grade change therefore writes the changed leaf's status and then re-evaluates and re-writes the derived rows from that leaf up to the competency root. Per :ref:`openedx-learning-adr-0005`, each level is stored as an ACTIVE row updated in place, holding -the current status for a learner and node, plus an append-only HISTORY row per applied change; this -supersedes :ref:`openedx-learning-adr-0003`'s original all-append-only model. +the current status for a learner and node, plus an append-only HISTORY row per genuine status +advance (bounded by monotonicity); this supersedes :ref:`openedx-learning-adr-0003`'s original +all-append-only model. How grade changes can reach openedx-core is constrained. openedx-core must never import from edx-platform and cannot read its grade tables, so grade changes can arrive only as @@ -98,9 +99,10 @@ status is advanced and a HISTORY row written: - *Advance-only; no automatic regression.* Once a status reaches the demonstrated level it is retained ("banked"), at every level including the leaf (:ref:`openedx-learning-adr-0005`). The recorder records first attainment and advancements automatically but does not regress a banked - status below the demonstrated level, even for a legitimately newer downward grade correction; a - suppressed downward correction is still written to HISTORY as seen-and-suppressed. Reversing a - banked status is a separate administrative action, out of scope here. + status below the demonstrated level, even for a legitimately newer downward grade correction. A + downward correction does not advance the status, so it writes no HISTORY row; HISTORY records + only advances, which is what bounds it by monotonicity (:ref:`openedx-learning-adr-0005`). + Reversing a banked status is a separate administrative action, out of scope here. **Correctness by a single serializing batch lock.** One deployment-wide lock guards the whole drain-batch operation, so only one batch runs at a time across the deployment. The read, diff --git a/docs/openedx_learning/decisions/0005-competency-mastery-storage-diagrams.md b/docs/openedx_learning/decisions/0005-competency-mastery-storage-diagrams.md new file mode 100644 index 000000000..8c043b98c --- /dev/null +++ b/docs/openedx_learning/decisions/0005-competency-mastery-storage-diagrams.md @@ -0,0 +1,90 @@ +# ADR 0005 diagrams: competency mastery storage + +Companion diagrams for `0005-competency-mastery-storage.rst`. They are kept here as Markdown so +they render natively on GitHub; they are not part of the Sphinx/readthedocs build. Refer to the ADR +for the authoritative decision text. + +## 1. Data model and the ACTIVE/HISTORY split + +Criteria definitions live in the main database. Learner status is stored at every level (leaf, +group, competency), each split into an ACTIVE table (current status, updated in place) and an +append-only HISTORY table. All learner-status tables are assigned to a dedicated `competency_mastery` +database alias through a router; that alias defaults to the main database, so a stock deployment runs +on one database and a large deployment can point the alias at a separate database with no migration. +Foreign keys that could cross the alias boundary are declared without database-level constraints. + +```mermaid +flowchart TB + subgraph DEF["Criteria definitions (main database)"] + CT["CompetencyTaxonomy"] + CRP["CompetencyRuleProfile"] + CCG["CompetencyCriteriaGroup
AND/OR, nestable, course-scoped"] + CC["CompetencyCriterion (leaf)
tag/object association + rule"] + CCG -->|"parent_id self-nest"| CCG + CCG -->|"contains"| CC + CRP -.->|"default rule"| CC + CT -.->|"scopes"| CRP + end + + subgraph MAST["Learner status: competency_mastery alias (defaults to the main database)"] + LA["Leaf ACTIVE
StudentCompetencyCriteriaStatus
one row per learner + criterion"] + LH["Leaf HISTORY
append-only, one row per status advance (bounded)"] + GA["Group ACTIVE
StudentCompetencyCriteriaGroupStatus"] + GH["Group HISTORY
append-only"] + CA["Competency ACTIVE
StudentCompetencyStatus"] + CH["Competency HISTORY
append-only"] + CMS["CompetencyMasteryStatuses (lookup)
Demonstrated / PartiallyAttempted /
AttemptedNotDemonstrated"] + LA -.->|"append on change"| LH + GA -.->|"append on change"| GH + CA -.->|"append on change"| CH + LA -->|"rolls up to"| GA + GA -->|"rolls up to"| CA + CMS -.->|"status_id"| LA + end + + CC -.->|"FK, no DB constraint (may cross alias)"| LA +``` + +## 2. Recording a grade change: incremental roll-up with banking + +A grade-change event resolves the criteria fed by the subsection, evaluates the leaf as a pure +function of the grade and the rule, applies the out-of-order and advance-only rules, then rolls the +result up the tree, writing only the rows whose status changed. + +```mermaid +flowchart TD + E["Grade-change event
(carries effective source timestamp)"] --> EV["Evaluate leaf = f(grade, rule)"] + EV --> OOO{"Event older than the
leaf's current timestamp?"} + OOO -->|"yes"| DROP["Ignore: out-of-order defense"] + OOO -->|"no"| BANK{"Leaf already banked Demonstrated
and event is a downward correction?"} + BANK -->|"yes"| SUP["Keep ACTIVE banked;
no HISTORY row (advances only)"] + BANK -->|"no / upward"| WLEAF["Upsert leaf ACTIVE;
append leaf HISTORY"] + WLEAF --> RU{"Leaf now Demonstrated?"} + RU -->|"no"| ENSURE["Ensure ancestor ACTIVE rows exist
as AttemptedNotDemonstrated"] + RU -->|"yes"| GEVAL["Re-evaluate parent group
from stored child rows"] + GEVAL --> GCHG{"Group status changed?"} + GCHG -->|"no"| STOP["Stop: short-circuit"] + GCHG -->|"yes"| GWRITE["Upsert group ACTIVE + HISTORY"] + GWRITE --> UP["Continue up to the competency root"] +``` + +## 3. Status lifecycle for a node (advance-only) + +A node advances through statuses and is banked once Demonstrated: it never auto-regresses, so late +or duplicate events are safe. A leaf is atomic, so it only ever uses NotStarted (absent row), +AttemptedNotDemonstrated, or Demonstrated; PartiallyAttempted is a group-level state. + +```mermaid +stateDiagram-v2 + [*] --> NotStarted: no row + NotStarted --> AttemptedNotDemonstrated: first unsuccessful attempt + AttemptedNotDemonstrated --> PartiallyAttempted: some children attained (AND / mixed groups) + AttemptedNotDemonstrated --> Demonstrated: logic satisfied + PartiallyAttempted --> Demonstrated: logic satisfied + Demonstrated --> Demonstrated: banked; downward corrections write no HISTORY row + note right of Demonstrated + Advance-only: never auto-regresses. + Competency level surfaces only + Demonstrated / PartiallyAttempted. + end note +``` diff --git a/docs/openedx_learning/decisions/0005-competency-mastery-storage.rst b/docs/openedx_learning/decisions/0005-competency-mastery-storage.rst new file mode 100644 index 000000000..e93a6de6e --- /dev/null +++ b/docs/openedx_learning/decisions/0005-competency-mastery-storage.rst @@ -0,0 +1,352 @@ +.. _openedx-learning-adr-0005: + +5. How should learner competency mastery be stored at scale? +============================================================= + +Status +------ +Proposed. + +Relationship to prior decisions +-------------------------------- +This decision refines two prior ADRs and reverses an earlier draft of this one: + +- It refines :ref:`openedx-learning-adr-0002`'s learner-status model. ADR 0002 materializes + (stores) mastery at all three levels: the criterion (leaf), the criteria group, and the + competency. This decision keeps all three stored, but changes *how*: each level is split into an + ACTIVE table (one current row per learner and node, updated in place) and an append-only HISTORY + table, and the large per-learner leaf tables adopt the large-table techniques edx-platform uses + for persistent grades. +- It supersedes :ref:`openedx-learning-adr-0003`'s rule that the learner-status tables are + append-only with the current status being the most recent row. Those tables move to an ACTIVE + table updated in place plus an append-only HISTORY table. +- It refines :ref:`openedx-learning-adr-0003`'s guardrail that "edits apply going forward; existing + statuses are not retroactively updated," to the monotonic rule described under "Retroactive + criteria changes are monotonic for the learner" below. +- It reverses an earlier draft of this ADR that computed leaves transiently and never stored them. + That design is retained as the first rejected alternative below. + +A companion set of diagrams for this decision lives alongside it in +``0005-competency-mastery-storage-diagrams.md``. + +Context +------- +Per :ref:`openedx-learning-adr-0002`, competency achievement criteria form a boolean tree. An +internal ``CompetencyCriteriaGroup`` node combines child nodes with an ``AND``/``OR`` +``logic_operator``, can be scoped to a course run, and can nest under a parent group. A +``CompetencyCriterion`` leaf is the tree's terminal node: it points at one tag/object association +and a rule (``CompetencyRuleProfile``; only the ``Grade`` rule type is in scope today), and is a +pure function of one subsection grade and that rule. Mastery status is one of ``Demonstrated``, +``PartiallyAttempted``, or ``AttemptedNotDemonstrated``; an absent row means the learner has not +started. A leaf is atomic, so it is only ever ``Demonstrated`` or ``AttemptedNotDemonstrated``; +``PartiallyAttempted`` is a group-level state. + +The learner-facing and instructor-facing progress dashboards need per-assignment detail: for a +given competency they show which individual criteria a learner has and has not demonstrated, not +just the rolled-up group and competency verdicts. That per-criterion view is the primary driver for +storing leaf mastery rather than deriving it on the fly. A durable, append-only record of each +leaf's transitions over time is a secondary benefit, used for audit and tracing. + +Storing every leaf multiplies out at scale. A course can carry on the order of 200 leaf criteria, +so the leaf level is where the row count concentrates: the ACTIVE leaf table (learners x attempted +leaves) reaches the low billions for an Open edX instance with millions of learners. The dominant +multiplier is this per-leaf breadth (roughly 200x per course), not time. Mastery is monotonic (see +"Advance-only banking" below): a node can only advance through the small status lattice +``AttemptedNotDemonstrated`` to ``PartiallyAttempted`` to ``Demonstrated``, at most a handful of +forward steps ever. The append-only HISTORY table records only those genuine advances, so it holds a +small constant number of rows per learner and node, bounded by monotonicity rather than growing +without limit as grades are corrected or re-evaluated over time. HISTORY is therefore about the same +order of size as ACTIVE, not a multiple that grows with a learner's history. This per-leaf scale is +what this decision has to make workable. + +That scale is not, on its own, what makes a relational database struggle. A point lookup against a +billion-row table backed by the right composite index is a logarithmic-time index seek regardless +of the table's size; the dashboard reads this feature performs are exactly such point lookups. What +billions of rows makes painful is schema migrations, backups, and any non-indexed or aggregate +query. The design's job is therefore to keep rows narrow, keep every read on an index, keep the +append-only history off the hot path, and push cross-learner analytics to an analytics store rather +than the relational table. + +openedx-core also cannot read edx-platform's grade tables directly and receives grade changes only +as ``openedx-events`` events (:ref:`openedx-learning-adr-0004`). Leaf mastery is therefore recorded +from the grade carried by the event when it arrives; the relational tables here are the record of +that computation, not a cache that can be rebuilt by re-reading a grade later. + +Decision +-------- +**Store mastery at every level, each split into ACTIVE and HISTORY.** The leaf, group, and +competency levels each keep one ACTIVE row per learner and node, updated in place, so reading a +learner's current status is a direct indexed lookup rather than a scan for the most recent of many +rows. Each level also has a parallel append-only HISTORY table that records one row per genuine +status advance, for audit and point-in-time reconstruction. Because status only advances, the status +at any past time is the latest recorded advance at or before that time, so point-in-time is fully +reconstructable from HISTORY. Because status is monotonic, the number of advances per node is bounded +by the status lattice (a small constant), so HISTORY grows with learners and nodes, not with time, +and stays about the same order of size as ACTIVE. Keeping ACTIVE and HISTORY separate still pays: +ACTIVE is a single in-place current row optimized for the dashboard point lookup and is the row +per-learner concurrency is anchored on (:ref:`openedx-learning-adr-0004`), while HISTORY is +append-only and read only for audit. + +**Leaf mastery is stored per learner and criterion.** The leaf ACTIVE row keys on +``(user_id, competency_criteria_id)`` and carries the derived status plus an effective source +timestamp (the timestamp of the grade change it was computed from; see +:ref:`openedx-learning-adr-0004`). Dashboards read per-criterion status straight from this table. +The leaf HISTORY row is the append-only ledger of those changes. A leaf stores only the derived +status, not the grade value: openedx-core does not persist grades (see the non-goals), so the leaf +is a record of *what was demonstrated*, not a re-evaluable copy of the input. + +**Group and competency roll-ups read stored leaf rows.** Because leaves are now stored, a group +re-evaluates its ``logic_operator`` by reading its child leaf (and child group) ACTIVE rows +directly. The prior draft carried a per-learner "attained-set" on each group row precisely because +leaves were not stored and siblings could not be read; that mechanism is removed. Roll-up is +incremental: when a grade event changes a leaf, the recorder re-evaluates the parent group, and if +the group's status changes, its parent in turn, up to the competency root, writing only the rows +whose status actually changed. Per-group fan-out is small, so reading a group's children is a cheap +indexed range read. + +**Status semantics.** ``Demonstrated`` means the node's logic is satisfied. ``PartiallyAttempted`` +means at least one child is demonstrated but the group's logic is not yet satisfied; it is +meaningful for ``AND`` groups and mixed trees, never for a leaf and never for an ``OR`` group (one +attained child already satisfies an ``OR``). ``AttemptedNotDemonstrated`` means the node has seen at +least one relevant grade event but nothing is yet demonstrated. An absent row means not started. At +the competency level only ``Demonstrated`` and ``PartiallyAttempted`` are surfaced, consistent with +:ref:`openedx-learning-adr-0002`: a ``Demonstrated`` root surfaces as ``Demonstrated``, a +``PartiallyAttempted`` root as ``PartiallyAttempted``, and a root that has only seen unsuccessful +attempts produces no competency-level row at all. + +**The large leaf tables adopt edx-platform's persistent-grade playbook.** The leaf ACTIVE table has +the same shape and cardinality as edx-platform's ``PersistentSubsectionGrade`` (one row per learner +per graded subsection), which runs at production scale in the main database with no separate +database, no partitioning, and no sharding. This decision mirrors the techniques that make that +work, and consciously declines the ones edx-platform did not need: + + - *64-bit primary keys from the start.* The leaf ACTIVE and HISTORY tables use a 64-bit + ``BigAutoField`` primary key, chosen up front, mirroring edx-platform's + ``UnsignedBigIntAutoField`` on ``PersistentSubsectionGrade`` ("primary key will need to be + large for this table"). Changing a primary-key type on a billion-row table later is + prohibitively expensive. + - *Narrow rows; content-hash dedup for any repeated blob.* edx-platform keeps grade rows narrow + by storing only a fixed-width hash of the visible-block set on the hot row and holding the + large JSON once in a ``VisibleBlocks`` dedup table (canonical JSON, SHA-1, base64, + unique-constrained, referenced by hash rather than a surrogate id). Our leaf rows are already + narrow (user, criterion, status, timestamps), so there is no per-row blob to deduplicate + today. The rule adopted here is that if a large, mostly-repeated payload is ever added to + these rows, it must be stored once behind a content hash rather than copied per row. + - *Composite indexes derived from documented read paths.* Indexes are added per named read path, + not by guessing: ``(user_id, competency_criteria_id)`` unique for the recorder's point read + and write; ``(user_id, ...)`` for the learner dashboard; a course-scoped index for + instructor and reporting reads. New read paths add exactly the index they need. + - *In-place current row; history only where audit needs it.* The ACTIVE row is written with an + upsert (``update_or_create`` / bulk upsert). edx-platform keeps no history table on its hot + grade tables and pays for history only on the low-volume override table; this decision + deliberately does keep a leaf HISTORY table, because leaf-transition traceability is an + explicit (if secondary) goal, and isolates it from the ACTIVE path so the hot reads and + writes never touch it. The cost of that choice is called out under accepted tradeoffs. + - *Idempotency in domain logic, not a generic version column.* Correctness under duplicate and + out-of-order delivery comes from two domain rules, not an optimistic-lock column: the + monotonic banking below (a status is set once and only advances, mirroring edx-platform's + write-once ``first_attempted`` and its "only persist a rescore if it is higher") and the + effective-source-timestamp out-of-order defense in :ref:`openedx-learning-adr-0004` + (mirroring edx-platform's check of the source score's timestamp before trusting a queued + recompute). + - *Batch the read and write paths.* The recorder reads current statuses in bulk and writes leaf + ACTIVE (bulk upsert), leaf HISTORY (``bulk_create``), and the rolled-up group/competency rows + in a small, fixed number of round-trips per batch, independent of the number of rows in the + batch. This is the same "prefetch per cohort, bulk-write" posture edx-platform uses and is the + mechanism by which :ref:`openedx-learning-adr-0004`'s single serialized pipeline keeps up. + - *Chunk mass recompute and provide a kill switch.* The bulk recompute triggered by a + structural criteria edit is chunked by a configurable batch size and can be halted by an + operational switch, mirroring edx-platform's ``ComputeGradesSetting`` batch size and its + ``DISABLE_REGRADE_ON_POLICY_CHANGE`` switch, so a large edit cannot become an unbounded + recompute storm. + - *A dedicated database alias and router, baked in from the start.* The learner-status tables + (leaf, group, and competency ACTIVE and HISTORY) are assigned to a dedicated Django database + alias through a database router, mirroring edx-platform's courseware-history router + (``StudentModuleHistoryExtended``). The alias defaults to the main database, so a stock + deployment runs everything on one database and gains no new mandatory infrastructure, keeping + faith with :ref:`openedx-learning-adr-0004`'s no-new-mandatory-infrastructure value; a large + deployment points the alias at a separate physical database purely through settings, with no + schema change and no data migration. Building the router now, rather than deferring it, is what + makes that later split a configuration change instead of a migration on a billion-row table. + Foreign keys that would cross the alias boundary (a learner-status row referencing a + criteria-definition row, and the reference to the user) are declared without database-level + constraints, as edx-platform does for ``PersistentSubsectionGrade.user_id``, so the tables can + live in different databases; the delete protection of :ref:`openedx-learning-adr-0002` is + therefore enforced in application code, not by a database constraint. + - *No table partitioning, sharding, or read-replica routing.* These remain consciously rejected: + the edx-platform grades app operates at scale without any of them, and narrow rows plus + indexes plus dedup were sufficient. They can be revisited if a specific need is proven. + +**Advance-only banking, monotonic.** Once a node reaches ``Demonstrated`` its ACTIVE row is retained +("banked"): the recorder never automatically regresses it, not on a later downward grade correction +and not on a criteria change. This applies at every level, including the leaf. A genuine downward +grade correction does not advance the status, so it writes no HISTORY row and leaves the banked +ACTIVE status unchanged; because HISTORY records only advances, it never carries suppressed +regressions. Reversing a banked status is a separate administrative action, out of scope here. +This monotonicity is what makes out-of-order and duplicate delivery safe, since a late or replayed +event can never lower a status, and :ref:`openedx-learning-adr-0004` relies on it. + +**Retroactive criteria changes are monotonic for the learner.** A retroactive edit can newly grant +or preserve mastery, but it never silently revokes it, and it never rewrites a learner's recorded +leaf mastery downward, consistent with :ref:`openedx-learning-adr-0003`'s rule that edits apply +going forward. For STRUCTURAL edits (a leaf added to or removed from a group, an ``AND``/``OR`` +flip, or retagged content), a chunked bulk recompute runs when the edit is published: for each +affected learner it re-evaluates the new group logic against the learner's stored leaf rows and +applies only upward transitions. This recompute is self-contained: it reads the learner's stored +leaf statuses, not the grades, which is why a learner who now meets eased criteria becomes +``Demonstrated`` with no further activity required, while a tightened edit leaves already-banked +learners untouched. A RULE or threshold change (for example, raising a passing bar) cannot be +recomputed from stored state, because whether a leaf was attained depends on the grade, which this +decision deliberately does not store; such a change takes effect only through a re-emission of the +affected grades via the normal recording pipeline, or lazily, the next time the learner produces a +relevant grade event. + +**Non-goals (deferred/out of scope).** openedx-core does not store grades, so re-evaluating leaves +under a changed rule or threshold from stored state is not supported (see above). Administrative +revocation or correction of banked mastery (errata) is deferred to future work; the recorder here +is strictly advance-only. Cross-learner analytics and reporting are served by an analytics store +(ClickHouse/Aspects), not by aggregate queries over these relational tables. Because HISTORY records +only advances, its growth is already bounded by monotonicity (learners x nodes x a small constant), +so no retention or tiering policy is required to keep it workable; one may still be added later, and +the database router above lets the HISTORY tables be relocated to a separate database if a deployment +prefers. Emitting a status-change event for downstream notification is also out of scope here. + +Accepted tradeoffs +------------------ + + - The leaf tables are large by design (billions of rows at the top end), so schema migrations, + backups, and any non-indexed query on them are expensive. This is the same posture + edx-platform accepts for persistent grades; it is mitigated by the 64-bit primary key, narrow + rows, and index-only read paths, but not eliminated. + - Keeping a leaf HISTORY table is a deliberate departure from edx-platform, which keeps no + history on its hot grade tables. Because it records only status advances, it is bounded by + monotonicity to about the same order of size as the ACTIVE leaf table, rather than an unbounded + ledger that grows as grades are re-evaluated. It is justified by the audit and point-in-time + goal, isolated from the hot path, and the dedicated database alias and router above let it be + split onto its own database when a deployment needs to. + - Growth is managed structurally (narrow rows, big keys, indexes) and bounded by monotonicity + (HISTORY records only advances), not by retention or deletion. A retention or tiering policy is + therefore not required to keep the tables workable, and is left as optional future work. + - This decision re-incurs the per-leaf breadth multiplier that the earlier transient-leaf draft + avoided. That cost is accepted in exchange for the per-assignment dashboard detail that + motivates it. + +Rejected Alternatives +--------------------- + +1. Compute leaves transiently, never store them (the earlier draft of this ADR). + + Leaves are derived on the fly from the grade and the rule and never persisted; only group and + competency mastery is stored, in ACTIVE and HISTORY tables, with a per-learner "attained-set" on + each group row so an ``AND`` group can be evaluated without re-reading grades. + + - Pros: + - Removes the ~200x per-leaf breadth multiplier from stored data entirely; the hot store is + hundreds of millions of rows rather than billions. + - No large-table machinery, no separate-database question, cheaper migrations and backups. + - Lower operational burden on small and medium deployers. + - Cons: + - No stored per-criterion status, so the per-assignment dashboard detail that drives this + decision would have to be recomputed on every read, or approximated from group-level + state, rather than read directly. + - No leaf-level history for audit or tracing. + - The attained-set is a bespoke mechanism that exists only because leaves are not stored, and + it must be kept correct under retroactive edits and out-of-order delivery. + - Why rejected: the primary requirement is a live per-criterion dashboard view, which is a direct + read of stored leaf status. Deriving it transiently either moves that cost onto every read or + gives up the detail. Storing leaves also lets group roll-up read sibling rows directly and + removes the attained-set entirely. The storage cost is real but is the same cost edx-platform + already carries for persistent grades, and it is what the feature is for. + +2. Keep everything append-only (no ACTIVE table); current status is the latest row. + + - Pros: + - No in-place mutation; uniform with the original :ref:`openedx-learning-adr-0003` model. + - Cons: + - Current-status reads become "latest advance per node" queries (a group-by or ordered scan) + rather than a single-row point lookup, on the dashboard hot path. + - There is no in-place current row to anchor per-learner concurrency on, which + :ref:`openedx-learning-adr-0004` relies on. + - Why rejected: even with HISTORY bounded by monotonicity, a single in-place ACTIVE row is a + cheaper and simpler dashboard read than resolving the latest advance out of a node's history, + and it is the row per-learner concurrency in :ref:`openedx-learning-adr-0004` is anchored on. + +3. Recompute group and competency status on read instead of storing them. + + - Pros: + - No stored derived state above the leaf. + - Cons: + - Reopens :ref:`openedx-learning-adr-0002`, which materialized these levels for dashboard read + performance, and moves the aggregate cost onto every read. + - Why rejected: the read surface this feature serves is the one that decision protected; leaves + are already stored here, so group and competency roll-ups are cheap incremental writes rather + than repeated read-time aggregation. + +4. Make a separate physical database mandatory, or partition/shard the leaf tables, up front. + + - Pros: + - Isolates the large tables' write, backup, and migration load from the main database for + every deployment. + - Cons: + - Makes separate database infrastructure mandatory for every deployment that enables + competencies, in tension with :ref:`openedx-learning-adr-0004`'s value of adding no new + mandatory infrastructure, and burdens small and medium deployers with a database they may + barely use. + - Partitioning and sharding add operational complexity the edx-platform grades app never + needed at scale. + - Why rejected: the chosen design already bakes in a database router and a dedicated alias, so a + deployment that needs isolation gets it through settings with no migration, while a stock + deployment keeps a single database. Forcing a separate database or a partitioning scheme on + everyone buys nothing the router does not, at a real operational cost. Partitioning and + sharding remain available to revisit if a specific need is proven. + +5. Store a durable per-learner grade-input projection (effective grade per learner and subsection) + and derive leaves on demand. + + - Pros: + - Fully re-evaluable, including for rule/threshold changes. + - Supports leaf-level audit. + - Cons: + - Duplicates grade data into openedx-core with its own PII and retention profile. + - Is a table of the same cardinality as the leaf table, so it does not avoid the scale + question; it adds grade governance on top of it. + - Why rejected: it recreates the leaf-level cardinality and adds a grade-data governance problem, + to buy rule-change recompute that is a documented, accepted limitation of not storing grades. + +6. Store child evaluations on the parent group row instead of a leaf ACTIVE table (an enriched + attained-set). + + Keep the leaf HISTORY table but drop the leaf ACTIVE table. Each group ACTIVE row carries an + array of its direct children's frozen evaluations, one tuple per child of + ``(competency_criteria_id, status, effective_source_timestamp)``. The current per-criterion + status a dashboard shows is read from the parent group's array rather than from a leaf row, and + the leaf HISTORY table remains the append-only audit trail. This is the "attained-set" of the + transient-leaf design (alternative 1) enriched from a set of demonstrated ids to a full per-child + status snapshot, so that it can serve the per-assignment dashboard the bare attained-set could + not. + + - Pros: + - Removes the ~200x leaf multiplier from the hot current-state store: the current-state + tables are group-granular, roughly 7x fewer rows, and a group's whole child set is read in + a single row. + - The write path is slightly leaner: a leaf event appends HISTORY and rewrites the parent's + array in memory, with no leaf ACTIVE upsert and no sibling-row read. + - Preserves the frozen-evaluation, monotonic, and banking properties. + - Cons: + - Reintroduces a denormalized field that must be kept correct under out-of-order delivery and + structural edits, updated by read-modify-write. This is the bespoke-mechanism cost that + storing leaves as first-class rows was chosen to remove. + - Couples a leaf's frozen evaluation to its position in the tree: the eval lives inside the + parent group's row, so a criterion reparented by a later structural edit strands its frozen + eval in the old group's array. A first-class leaf row is keyed by the criterion, + independent of tree position, and is immune to this. + - Gives up an indexed per-criterion query; cross-learner per-leaf reads must scan arrays or go + to the analytics store. + - The dominant table, leaf HISTORY, is unchanged, so the headline migration and backup cost + is not reduced; only the hot store shrinks. + - Why rejected: structural robustness is valued over the hot-store saving. First-class leaf rows + keep a leaf's frozen mastery independent of how the criteria tree is later restructured, and + avoid re-incurring the denormalized-array correctness burden that this decision removed by + storing leaves. The hot-store reduction is real but does not address the largest table, and the + single-row group read it optimizes is served acceptably by an indexed range read of a learner's + leaf rows. From 78bc16ce96e99aca2e0d8a55652d97c60dc12a76 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Fri, 17 Jul 2026 15:18:54 -0400 Subject: [PATCH 07/10] docs: add reasoning for rejecting per-learner batching --- .../0004-competency-mastery-concurrency.rst | 58 ++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst index 21d431c94..84e49f163 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -204,25 +204,53 @@ Rejected Alternatives distinct option: with the isolation added it becomes keyed partitioning; without it, it is incorrect. -3. Per-learner database row lock with per-event recording. +3. A per-learner lock (a lock keyed on the learner) instead of the single deployment-wide lock. + + A per-learner lock is the minimal primitive that satisfies the correctness requirement: it + guarantees no two workers evaluate the same learner at once, which is exactly what prevents the + same-learner write-skew in the Context. It is therefore *not* rejected for being incorrect. It is + rejected because it is dominated on both sides: for correctness alone the single deployment-wide + lock is simpler, and for the one thing a per-learner lock adds over that (letting different + learners record in parallel) keyed partitioning (alternative 1) is strictly better. The + per-learner lock is the awkward middle between the two. + - Pros: - Correct regardless of delivery order or deployment topology, without depending on how events are routed or partitioned. - - Conceptually simple and self-contained: it relies only on the database, with no - event-bus partitioning contract. + - Self-contained: it relies only on the database, with no event-bus partitioning contract. + - Unlike the single deployment-wide lock, it lets different learners be recorded in parallel. - Cons: - - Processes one event at a time behind a lock and a transaction, so it does not keep up - under bursty grading across many learners. - - Holds a database connection and a worker for each event while the lock is contended or - waited on. - - Requires an extra per-learner lock table and its locking machinery. - - Does not batch writes, so per-event transaction and commit overhead dominates at volume. - - This was an earlier design for competency mastery recording; it is correct but the least - performant. Both the chosen batch-lock approach and the keyed-partitioning alternative - supersede it: keyed partitioning provides the same same-learner serialization as a structural - property while allowing parallelism and batching, and the batch lock provides it with a single - lock and batched writes instead of a per-learner lock and per-event writes. + - *It fights the batching the throughput depends on.* The chosen pipeline is fast because it + batches across learners: one bulk read, one in-memory evaluation, one bulk write per batch, + amortized over every learner in the batch. A per-learner lock is taken per learner, so work + is serialized and committed per learner (in the original per-event form, per event), which + reintroduces exactly the per-unit transaction, commit, and lock acquire/release overhead + that batching removes. Under bursty grading across many learners that per-unit overhead + dominates. + - *Its lock lifecycle is multiplied across every learner.* The single lock has one lifecycle + to operate (acquisition, timeout, stale-lock recovery on a crashed worker). A per-learner + lock needs a per-learner lock table (or keyed advisory locks) and that same lifecycle + replicated across potentially millions of learner keys, held and waited on concurrently, + with a database connection and worker tied up per contended lock. + - *The batched variant becomes a multi-lock ordering problem.* Trying to keep the batching by + locking every learner in a batch at once means holding many locks simultaneously and + acquiring them in a consistent order to avoid deadlock between batches whose learner sets + overlap: more failure modes than a single lock, for no correctness gain. + - Storing leaves (:ref:`openedx-learning-adr-0005`) widens this gap rather than narrowing it: + the per-learner write volume is now roughly the per-course leaf fan-out larger, so + processing per learner without amortizing across learners costs more than it did before. + - Why rejected: the per-learner lock buys exactly one thing over the single deployment-wide lock, + per-learner parallelism, and pays for it with per-learner lock machinery and the loss of + cross-learner batching. While parallelism is not the binding constraint (the current + expectation, since one batch pipeline keeps up with peak grading) the single lock is simpler and + correct. When parallelism does become the binding constraint, keyed partitioning (alternative 1) + provides the same per-learner serialization as a structural property, lock-free and horizontally + scalable, which strictly beats a per-learner lock. There is no operating point at which the + per-learner lock is the best option, which is why it is not the chosen design and not the + escalation path. This was an earlier design for competency mastery recording; both the chosen + batch lock and keyed partitioning supersede it. See also alternative 2, which shows that keeping + the batching while moving evaluation outside a single lock collapses into either this + per-learner isolation or the write-skew. 4. No lock; assume same-learner conflicts are rare and tolerate them. - Pros: From 4cfb0923c4b50e9105dd8f7aaf7809c364ef7910 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 20 Jul 2026 11:34:34 -0400 Subject: [PATCH 08/10] docs: choose monotonic non-locking option --- ...competency-mastery-concurrency-diagrams.md | 88 +-- .../0004-competency-mastery-concurrency.rst | 548 ++++++++---------- 2 files changed, 287 insertions(+), 349 deletions(-) diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md index c17a682d9..0ec2f085a 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md @@ -4,58 +4,62 @@ Companion diagrams for `0004-competency-mastery-concurrency.rst`. They are kept they render natively on GitHub; they are not part of the Sphinx/readthedocs build. Refer to the ADR for the authoritative decision text. -## 1. End-to-end recording pipeline +## 1. Entry points: two options (no preference) -A scheduled producer on the edx-platform side reads changed subsection grades and emits bounded -batch events. openedx-core receives them in-process, writes them idempotently into an inbox, and -returns. A separate scheduled task drains the inbox in windowed micro-batches under a single -deployment-wide lock, doing bulk reads and bulk writes. +Both options push a grade change from edx-platform into openedx-core, which records it with a +monotone merge and re-evaluates the parents. They differ only in where the leaf write happens and +whether it is atomic with the grade write. Batching (the dashed producer) is optional and applies to +Option B. + +```mermaid +flowchart TD + subgraph EP["edx-platform (higher layer)"] + GRADE["Subsection grade write"] + PROD["Optional producer worker:
poll changed grades, coalesce,
emit bounded batch events"] + end + subgraph CORE["openedx-core (lower layer)"] + API["Recording API:
monotone merge + re-evaluate parents"] + RCV["Signal/event receiver"] + end + + GRADE -->|"Option A: synchronous call
inside the same transaction
(shared DB, atomic)"| API + GRADE -->|"Option B: emit subsection-grade-changed
signal/event"| RCV + PROD -.->|"optional batched events"| RCV + RCV --> API +``` + +## 2. Why it is correct without a lock + +Every write is `status := max(stored, computed)`, so writes commute, repeat harmlessly, and never +regress. A conjunctive parent computed from a stale sibling view can be too low but never too high; +the re-evaluation triggered by the last child to commit reads every committed sibling and merges the +parent up to the correct value. Result: eventually correct, never over-stated, no coordination. ```mermaid sequenceDiagram autonumber - participant PG as edx-platform persisted grades - participant PROD as Scheduled producer (edx-platform) - participant EV as openedx-events - participant RCV as Receiver (openedx-core, in-process) - participant INBOX as Inbox table - participant DRAIN as Scheduled drain task + participant WA as Worker A (child L1) + participant WB as Worker B (child L2) participant DB as Mastery tables (ACTIVE + HISTORY) - PROD->>PG: range query by modified watermark (+ trailing overlap window) - PG-->>PROD: changed subsection grades (effective timestamp) - PROD->>EV: bounded, fixed-size batch events - EV->>RCV: in-process delivery - RCV->>INBOX: idempotent write of rows - RCV-->>EV: return (minimal work) - loop each drain cycle - DRAIN->>DRAIN: acquire deployment-wide batch lock - DRAIN->>INBOX: read a window of rows - DRAIN->>DB: bulk read current ACTIVE statuses - DRAIN->>DRAIN: evaluate in memory (fold per learner by effective timestamp) - DRAIN->>DB: bulk upsert ACTIVE + bulk append HISTORY - DRAIN->>DRAIN: release lock - end + Note over WA,WB: L1 and L2 are siblings under conjunctive parent G + WA->>DB: merge L1 up; commit + WB->>DB: merge L2 up; commit (last committer) + WA->>DB: re-evaluate G from committed children
(sees L1 new, L2 maybe stale) → may be too low + WB->>DB: re-evaluate G from committed children
(sees L1 and L2 committed) → correct + Note over DB: max-merge keeps the higher value → G converges to correct, never over-stated ``` -## 2. Batch drain: correctness and performance +## 3. Optional monotone backstop -The lock serializes whole batches (not rows), so no two workers evaluate the same learner at once -and the same-learner write-skew cannot occur. Writes are bulk, so the higher per-batch row count of -stored leaves adds bulk-write time within a batch rather than per-row overhead or lock contention -between batches. +The only way step 2 leaves a parent low is a lost re-evaluation trigger. A periodic monotone +re-derivation repairs that: because it can only advance a status, it fixes a stalled parent without +ever corrupting one. It is not a correcting reconciliation (which a non-monotone design would need), +and can be omitted until lost triggers are actually observed. ```mermaid -flowchart TD - START["Drain cycle"] --> LOCK{"Acquire deployment-wide batch lock"} - LOCK -->|"not acquired"| SKIP["Skip this cycle"] - LOCK -->|"acquired"| READ["Read inbox window +
bulk read current ACTIVE statuses"] - READ --> FOLD["Per learner: fold events in
effective-source-timestamp order"] - FOLD --> RULES["Per leaf, apply two rules"] - RULES --> OOO["Out-of-order: ignore if older
than current leaf timestamp"] - RULES --> ADV["Advance-only: never regress a banked
status; downward corrections write no HISTORY row"] - OOO --> WRITE["Bulk upsert ACTIVE +
bulk append HISTORY
(leaf and rolled-up levels)"] - ADV --> WRITE - WRITE --> UNLOCK["Release lock"] - UNLOCK --> DUR["Durability: producer overlap re-scan +
idempotent inbox make re-delivery safe"] +flowchart LR + SWEEP["Periodic re-derivation
(optional)"] --> READ["Read committed children"] + READ --> MERGE["status := max(stored, recomputed)"] + MERGE --> SAFE["Advances a stalled parent;
never regresses a correct one"] ``` diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst index 84e49f163..c53916c16 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -21,348 +21,282 @@ fast. A single grade change therefore writes the changed leaf's status and then re-writes the derived rows from that leaf up to the competency root. Per :ref:`openedx-learning-adr-0005`, each level is stored as an ACTIVE row updated in place, holding the current status for a learner and node, plus an append-only HISTORY row per genuine status -advance (bounded by monotonicity); this supersedes :ref:`openedx-learning-adr-0003`'s original -all-append-only model. - -How grade changes can reach openedx-core is constrained. openedx-core must never import from -edx-platform and cannot read its grade tables, so grade changes can arrive only as -``openedx-events`` events. The only grade event that exists today is course-level and carries no -subsection identifier, so recording at subsection granularity needs a new event that edx-platform -does not yet emit (see `Prerequisite`_). ``openedx-events`` also delivers in-process by default, -because the event bus is not enabled in a stock deployment: a receiver runs synchronously in the -producer's worker and, in production, a receiver exception is swallowed and logged rather than -retried. Delivery is therefore best-effort, not durable, and the recorder cannot assume the event -bus is present. - -Two forces shape how this recording should happen: - -- **Same-learner correctness.** Grade-change events arrive asynchronously and can be delivered out - of order and processed on more than one worker. Two evaluations for the same learner that overlap - can each read a stale snapshot of the sibling leaf statuses and each write a derived roll-up - computed from an incomplete picture (a write-skew). Leaf rows are always correct, since each leaf - is a pure function of its own grade; only the derived group/competency rows can be left wrong. - Nothing crashes and no constraint is violated, but a learner's stored competency status can be - silently incorrect. +advance. + +**The property this decision rests on: mastery only ever moves forward.** Per +:ref:`openedx-learning-adr-0005`, every node, at every level, advances through a small status +lattice (``AttemptedNotDemonstrated`` to ``PartiallyAttempted`` to ``Demonstrated``) and is never +regressed automatically. A status is a high-water mark: once banked it stays, even when a later +grade correction is lower. This is a domain rule, not an implementation convenience, and the rest +of this decision is built on it. It holds only while the criteria that combine child statuses into +a parent are *monotone* boolean functions (AND, OR, k-of-n thresholds), so that advancing any child +can only advance the parent. :ref:`openedx-learning-adr-0002`'s criteria trees contain no negation, +which is the assumption that keeps parents monotone; a ``NOT`` node would break it. + +How grade changes reach openedx-core is constrained by layering. openedx-core must never import from +edx-platform and cannot read its grade tables. edx-platform is the higher layer and may depend on +openedx-core, so a grade change is *pushed* into openedx-core by edx-platform, either as a +synchronous call into an openedx-core recording API or as an ``openedx-events`` event. What +openedx-core may not do is reach back to read grades itself. ``openedx-events`` also delivers +in-process by default, because the event bus is not enabled in a stock deployment: a receiver runs +synchronously in the producer's worker and, in production, a receiver exception is swallowed and +logged rather than retried. In-process delivery is therefore best-effort, not durable, and the +recorder cannot assume the event bus is present. + +Two forces shape how recording should happen: + +- **Same-learner correctness.** A grade change writes the changed leaf and then re-derives the + group and competency rows above it. Leaf rows are always correct, since each leaf is a pure + function of its own grade. The derived rows are the hazard: two evaluations for the same learner + that overlap can each read a stale snapshot of the sibling leaf statuses and each write a derived + roll-up computed from an incomplete picture (a *write-skew*). Nothing crashes and no constraint is + violated, but a learner's stored competency status can be silently wrong. - **Throughput.** Grading is bursty and spans a very large number of learners, so the recording - path must keep up without paying a serialization or per-event transaction cost that grows with - the number of learners being graded. + path must keep up under peak load. -The question is how to guarantee same-learner correctness while still recording at high aggregate -throughput, over best-effort in-process delivery and without making the event bus mandatory. +The question is how to guarantee same-learner correctness at high throughput, over best-effort +in-process delivery, without making the event bus mandatory. Decision -------- -Grade changes are delivered to openedx-core as batched ``openedx-events`` events produced by a -scheduled task on the edx-platform side, buffered into an openedx-core inbox, then recorded in -windowed micro-batches serialized by a single deployment-wide lock. Correctness is chosen over -horizontal write throughput: a single serialized pipeline is adequate while it keeps up with peak -grading, it keeps the recorder self-contained and behaving identically on any Open edX deployment, -and it adds no new mandatory infrastructure. This decision depends on the `Prerequisite`_ below and -stays Proposed until that work lands. - -**Producing grade changes (edx-platform side).** A scheduled task in edx-platform reads the -subsections whose grade changed since a stored ``modified``-timestamp watermark (an indexed range -query on the persisted subsection-grade table, joined with its separately stored overrides to get -the effective grade), selects only the fields the recorder needs, and emits them as an -``openedx-events`` event. Because an ``openedx-events`` payload is conventionally a single entity -and, when the event bus is enabled, is size-capped by the transport, a cycle's rows are split into -bounded, fixed-size batch events rather than one large payload; carrying a list of rows is a -deliberate exception to the single-entity convention, scoped to this producer. Each row carries an -effective source timestamp, computed as the later of the base grade's and the override's -``modified`` time, so an override-only correction, which does not touch the base row, still -registers as newer. - -**Buffering and batching (openedx-core side).** Because delivery is in-process and synchronous, the -receiver does the minimum: it writes the batch's rows into an openedx-core-owned inbox table, -idempotently keyed so a re-delivered row is a no-op, and returns. A separate openedx-core scheduled -task drains the inbox in windowed micro-batches. Keeping the evaluation and the lock out of the -receiver keeps them off edx-platform's scheduler, bounds what a slow or wedged recorder can do to -the producer to "one more row in a table," and makes the durable inbox write, not the evaluation, -the step that must succeed for an event not to be lost. - -**Recording pipeline.** Resolving which competencies a subsection feeds is learner-independent, so -it is cached and de-duplicated across the batch. Each batch does one bulk read of current ACTIVE -statuses, evaluates in memory, and then, in a small fixed number of round-trips, bulk-upserts the -changed ACTIVE rows and bulk-appends the HISTORY rows, at the leaf level and every rolled-up level -it touches. The number of round-trips per batch is fixed regardless of how many leaves the batch -covers, which is what keeps the higher per-batch row count of stored leaves (see the stored-leaf -performance note below) from turning into per-row overhead. The in-memory engine -(:ref:`openedx-learning-adr-0002`) is unchanged: to apply several of one learner's events, the -recorder folds them in effective-source-timestamp order against an evolving snapshot. Persistence -follows :ref:`openedx-learning-adr-0005`'s ACTIVE-plus-HISTORY split. Two rules govern whether a -status is advanced and a HISTORY row written: - - - *Out-of-order defense.* A change is ignored when its effective source timestamp is older than - the current leaf's, so a late arrival cannot regress a newer status. This is a - delivery-ordering guarantee, distinct from the next rule. - - *Advance-only; no automatic regression.* Once a status reaches the demonstrated level it is - retained ("banked"), at every level including the leaf (:ref:`openedx-learning-adr-0005`). The - recorder records first attainment and advancements automatically but does not regress a banked - status below the demonstrated level, even for a legitimately newer downward grade correction. A - downward correction does not advance the status, so it writes no HISTORY row; HISTORY records - only advances, which is what bounds it by monotonicity (:ref:`openedx-learning-adr-0005`). - Reversing a banked status is a separate administrative action, out of scope here. - -**Correctness by a single serializing batch lock.** One deployment-wide lock guards the whole -drain-batch operation, so only one batch runs at a time across the deployment. The read, -evaluation, and append for a batch all happen under that lock, so no two workers ever evaluate the -same learner concurrently and the same-learner write-skew described in the Context cannot occur. -The lock is realized on infrastructure every deployment already has (for example, a database-backed -advisory lock) rather than on the event transport, so the recorder behaves the same on any -event-bus backend and adds no new operational dependency. - -**Durability.** In-process delivery swallows receiver exceptions and the producer's watermark -advances regardless, so a dropped event could otherwise be lost silently. Two low-cost mechanisms -cover this: the producer re-scans a trailing overlap window behind its watermark each cycle, so a -row committed just behind the cursor or missed once is re-emitted, and the consumer is idempotent, -so re-delivery is harmless. An on-demand reconciliation command is provided as an operator escape -hatch for incidents. A permanent scheduled reconciliation sweep is deliberately not included: it is -the kind of always-on cost this decision otherwise avoids, and can be added if real drops are ever -observed. - -**Latency.** Recording is expected to lag grading by minutes, not to be real-time; the dashboards -this feeds tolerate that. The producer's interval is a deployment setting with a documented floor. - -**Performance of stored leaves.** :ref:`openedx-learning-adr-0005` stores leaf mastery, so a batch -now writes on the order of the per-course leaf fan-out (up to ~200x) more rows than a -group-and-competency-only design would. This does not change the shape of the pipeline: the writes -are bulk (one bulk upsert of ACTIVE rows and one bulk append of HISTORY rows per level), so the -number of statements and the time held under the lock scale with batch size, not with the number of -learners or leaves inside a statement. The lock serializes whole batches, not rows, so the extra -rows add bulk-write time within a batch rather than lock contention between batches. The single -serialized pipeline remains adequate while one batch keeps up with peak grading; the higher leaf -write volume lowers the headroom before that ceiling relative to the transient-leaf design, and the -sustained-growth revisit noted below applies all the more. Batch size is the primary tuning knob, -and the mass recompute from a structural criteria edit is chunked and rate-limited -(:ref:`openedx-learning-adr-0005`) so it cannot crowd out live recording. +Recording is made **coordination-free** by leaning on the monotonicity above: no lock, no +serialized pipeline, no partitioning-for-correctness. Two mechanisms provide correctness, and they +are what any entry point below must implement. + +**1. Every write is a monotone merge, never a blind overwrite.** A node's status is written as +``status := max(stored status, newly computed status)`` (a single ``GREATEST``-style ``UPDATE``, +atomic at the row for the duration of that one statement, with no application-level lock). Because +the merge takes the higher of the two values, it is commutative, idempotent, and insensitive to +order: applying the same set of advances in any order, and re-applying any of them, yields the same +result. A late or duplicate delivery carries a status no higher than what is stored, so it is a +no-op. This is why out-of-order delivery and re-delivery are harmless without sequence tracking. + +**2. When a child advances, its parent is re-evaluated from the committed children.** The merge in +mechanism 1 makes a single write safe, but a *conjunctive* parent (for example "demonstrated only +when all children are demonstrated") can still be computed from a stale view of its siblings. Two +overlapping evaluations for one learner can each see only their own child's advance and each +compute a parent value that is too low, never too high. To close that gap, every child advance +re-evaluates the parent reading the *committed* child rows, and propagates upward if the parent +advances. Commits are serialized by the database, so there is always a last committer, and the +re-evaluation it triggers reads every sibling's committed value and merges the parent up to the +correct result. Derived rows are therefore *eventually correct and never over-stated*, with no +coordination between workers. In the language of the CALM theorem, a monotone computation needs no +coordination. + +The only way mechanism 2 can leave a parent low is if the last re-evaluation is never run (a lost +trigger). An optional monotone re-derivation sweep is a sound backstop: because it can only advance +a status, it repairs a stalled parent without ever corrupting one. It is not the "reconciliation to +fix wrong values" that a non-monotone design would need; it is belt-and-suspenders for a dropped +trigger, and can be omitted until lost triggers are actually observed. + +Two rules govern whether a status advances and a HISTORY row is written; both fall out of +monotonicity: + + - *Out-of-order defense.* A change older than the current leaf's effective source timestamp is + ignored, so a late arrival cannot regress a newer status. The monotone merge already enforces + this for the stored status; the timestamp check avoids writing a spurious HISTORY row for a + stale advance. + - *Advance-only; no automatic regression.* Once a status reaches a level it is banked at every + level including the leaf (:ref:`openedx-learning-adr-0005`). A downward grade correction does + not advance the status and writes no HISTORY row, which is what bounds HISTORY by + monotonicity. Reversing a banked status is a separate administrative action, out of scope + here. + +**Entry point: two options, presented without a preference.** Both implement the two mechanisms +above; they differ only in *where the leaf write happens* and whether it is atomic with the grade +write. The choice is a follow-up decision. + + **Option A: record inside the subsection-grade transaction.** edx-platform wraps its subsection + grade write and a synchronous call into an openedx-core recording API in one + ``transaction.atomic()``. This assumes mastery tables and the subsection-grade table share one + database, which makes grade and mastery genuinely atomic. + + - Pros: grade and mastery can never diverge; recording is real-time; no new event type is + needed, only a call from code that already runs. + - Cons: mastery work sits on the synchronous grading path, so a slow mastery query or a bug + adds latency to, or rolls back, the grade write (grades are the more critical data); + requires the shared database; and a conjunctive parent still needs a post-commit + re-evaluation (mechanism 2) to be airtight, so Option A is not purely inline unless it + accepts "heals on the learner's next event." + + **Option B: record from a subsection-grade-changed signal received in openedx-core.** + edx-platform emits a subsection-granular signal or event; an openedx-core receiver does the + monotone merge and the upward re-evaluation. + + - Pros: keeps mastery off the grade transaction, so mastery failures or latency never touch + grade writes; all mastery logic lives in openedx-core, which is the correct owner; no + shared-database requirement. + - Cons: not atomic with the grade write, so there is a brief window where the grade is + written but mastery is not yet (acceptable because mastery is already eventually + consistent and monotone); needs a new subsection-granular event + (see `Prerequisite`_); and in-process delivery is best-effort, so it relies on the + durability backstop below. + +**Throughput and optional batching.** Because writes commute (mechanism 1), throughput no longer +depends on a single serialized pipeline. Under Option B, more consumer workers can run in parallel; +under Option A, throughput is bounded by the grade path itself. Horizontal scale, which a lock-based +design would sacrifice, is available for free. Batching, a scheduled producer worker on the +edx-platform side that polls changed grades, coalesces them, and emits bounded batch events, is +therefore *optional*: it is one way to raise throughput, now competing with simply adding parallel +consumers rather than being the only route to it. Without batching, Option B receives one signal +per change and Option A needs no producer at all. Batching is *necessary* rather than merely +nice-to-have when: + + - Sustained per-change volume exceeds what parallel per-change consumers (Option B) or the + grade path (Option A) can absorb, so the fixed per-change overhead (competency resolution, + round-trips, task scheduling) dominates and only coalescing into bulk operations keeps up. + - The stored-leaf write amplification (:ref:`openedx-learning-adr-0005`; up to ~200x the + per-course leaf fan-out) makes per-change writes individually expensive, so bulk upserts + across a batch are needed to bound the statement count. + - Grading bursts (an exam closing, a bulk regrade) produce spikes that would otherwise saturate + consumers or the grade path. + +Resolving which competencies a subsection feeds is learner-independent, so when batching is used it +is cached and de-duplicated across the batch, and each batch does one bulk read of current ACTIVE +statuses, evaluates in memory, and bulk-writes the changed ACTIVE and HISTORY rows per level. + +**Latency.** Option A records in real time. Option B without batching lags by receiver processing; +with batching it lags by the producer interval plus drain time. The dashboards this feeds tolerate +minutes. + +**Durability.** In-process delivery swallows receiver exceptions, so a dropped event could be lost +silently. Under Option B the producer (batched or per-change) re-scans a trailing overlap window +behind its watermark each cycle so a missed row is re-emitted, and the monotone merge makes +re-delivery a no-op. Under Option A the atomic transaction is the durability guarantee. An on-demand +reconciliation command is provided as an operator escape hatch; a permanent scheduled sweep is not +required (see the optional monotone backstop above). Accepted tradeoffs: - - The write path is a single pipeline: only one batch runs at a time, so recording and evaluating does not - scale horizontally. This is adequate only while one batch pipeline keeps up with peak grading; - sustained high-volume growth would force a revisit. - - It introduces a lock and its lifecycle (acquisition, timeout, stale-lock recovery on crash) - that a lock-free partitioning design would avoid. - - Recording lags grading by the producer interval plus drain time; mastery is not updated in - real time. - - It requires new edx-platform code and a new ``openedx-events`` event (see `Prerequisite`_), - which is cross-repo coordination, though it keeps the dependency direction correct. + - Correctness is *eventual*, not point-in-time: a reader can briefly observe a derived status + that is too low, between a child's commit and its parent's re-evaluation. It is never too + high, and it converges. Consumers that need an exactly-consistent-at-all-times roll-up are + not served by this design. + - Mastery is a high-water mark. A downward or revoked grade never lowers it; un-mastering is a + deliberate out-of-band action. This is inherited from :ref:`openedx-learning-adr-0005` and is + the property that buys the coordination-free write path. + - Correctness depends on criteria staying monotone (no negation in + :ref:`openedx-learning-adr-0002`'s trees). If a future criteria feature introduces negation, + that node's parent is no longer monotone and this design does not cover it. + - Both options need edx-platform code, and Option B needs a new ``openedx-events`` event + (see `Prerequisite`_), which is cross-repo coordination, though it keeps the dependency + direction correct. Prerequisite ------------ -This decision requires, in edx-platform, the scheduled producer task and the ``openedx-events`` -event or events it emits. Their edx-platform-side design (task location, Celery queue, -retry/backoff, watermark storage, and crash recovery) is out of scope here and belongs in that -companion work. :ref:`openedx-learning-adr-0001` rejected this migration (its rejected alternative -8) as out of scope at the time; this decision takes it up as a now-scheduled prerequisite, and any -project documentation listing the migration as a non-goal is correspondingly superseded. +This decision requires cross-repo work in edx-platform, differing by option. Option A requires the +subsection grade write to call an openedx-core recording API within its transaction, and the mastery +tables to be routed to the same database. Option B requires a scheduled or signal-driven producer +and the ``openedx-events`` event or events it emits; the batched variant additionally requires the +polling-and-coalescing producer worker. The edx-platform-side design (task location, Celery queue, +retry/backoff, watermark storage, crash recovery) is out of scope here and belongs in that companion +work. :ref:`openedx-learning-adr-0001` rejected this migration (its rejected alternative 8) as out of +scope at the time; this decision takes it up as a now-scheduled prerequisite, and any project +documentation listing the migration as a non-goal is correspondingly superseded. Rejected Alternatives --------------------- -1. Correctness by keyed partitioning instead of a lock. +1. Serialize all writers with a lock (the previous form of this decision). - Partition grade-change events by ``user_id``: the key is *hashed* onto a small, fixed number of - partitions (not one partition per learner), so every event for a learner lands on the same - partition and is consumed by exactly one consumer. Same-learner events are then processed serially - by construction, with no database lock, while different learners are processed in parallel across - partitions. + Record under a single deployment-wide lock (or a per-learner lock), so no two workers ever + evaluate the same learner at once and the write-skew cannot occur. This was the earlier design + for competency mastery recording. - Pros: - - Correctness is structural; no lock and no lock lifecycle. - - The write path scales horizontally with the partition count. + - Correct regardless of delivery order or topology, without depending on monotonicity. + - Self-contained: relies only on the database. - Cons: - - Relies on the transport routing by key. Kafka does this natively; Redis Streams consumer - groups do not, so on Redis it needs application-level sharding into per-shard streams. - - Couples openedx-core to a platform-side contract in the wrong direction: the producer, in - openedx-platform, must set the partition key and keep the partition count stable, so the - core recorder's correctness depends on platform-side behavior. - - Changing the partition count reshuffles learners and can briefly let two consumers touch - one learner, so it needs an on-demand reconciliation command as a backstop. - - Why rejected: it optimizes for horizontal throughput at the cost of the two properties this - decision values most. It depends on the event transport (Kafka's keyed routing, or Redis-side - sharding that is not an established pattern in vanilla Open edX), so it would not behave - uniformly on a stock deployment, whereas the chosen approach is transport-agnostic. And it - inverts the intended dependency direction by making openedx-core rely on an openedx-platform - partition-key contract plus a reconciliation backstop. The horizontal scale it buys is not - needed while a single batch pipeline keeps up with peak grading, and accuracy is the priority - over the extra latency the single pipeline adds. - -2. Shrink the batch lock to guard only the bulk read and bulk write, running the per-learner - evaluation in parallel outside the lock. - - - Motivation: the database I/O is cheap (a few bulk statements), so locking only the I/O and - parallelizing the heavier evaluation looks like it would keep correctness while lifting the - chosen approach's single-pipeline cap. - - Why rejected: the lock exists to make each learner's read-evaluate-write atomic against other - writers, not to protect the I/O. Moving evaluation outside the lock reintroduces the exact - write-skew from the Context: two workers read the same snapshot for one learner, both - evaluate, and both write a roll-up from an incomplete picture. Making parallel evaluation - correct requires that each learner is only ever handled by one worker at a time, which is - per-learner isolation, i.e. the keyed-partitioning alternative above. This is therefore not a - distinct option: with the isolation added it becomes keyed partitioning; without it, it is - incorrect. - -3. A per-learner lock (a lock keyed on the learner) instead of the single deployment-wide lock. - - A per-learner lock is the minimal primitive that satisfies the correctness requirement: it - guarantees no two workers evaluate the same learner at once, which is exactly what prevents the - same-learner write-skew in the Context. It is therefore *not* rejected for being incorrect. It is - rejected because it is dominated on both sides: for correctness alone the single deployment-wide - lock is simpler, and for the one thing a per-learner lock adds over that (letting different - learners record in parallel) keyed partitioning (alternative 1) is strictly better. The - per-learner lock is the awkward middle between the two. + - A single deployment-wide lock forces a single serialized pipeline: recording does not + scale horizontally, which is only adequate while one pipeline keeps up with peak grading. + - A per-learner lock removes that cap but multiplies the lock lifecycle (acquisition, + timeout, stale-lock recovery) across potentially millions of learner keys, and fights the + cross-learner batching that throughput would otherwise rely on. + - Either way it introduces a lock and its failure modes for a guarantee monotonicity already + provides. + - Why rejected: the write-skew a lock exists to prevent is a consequence of computing derived + rows non-monotonically. Once every write is a monotone merge and every child advance + re-evaluates its parent, concurrent same-learner evaluations converge on their own, so the + lock guards against a hazard that no longer exists. Removing it also restores horizontal + scalability that the locked design gave up. The lock is strictly more machinery for strictly + less throughput. + +2. Allow non-monotonic mastery and repair drift with a reconciliation sweep. + + Let a status move up or down freely with the latest grade, accept that concurrency can leave + derived rows wrong, and run a scheduled job that recomputes each learner from scratch to correct + them. - Pros: - - Correct regardless of delivery order or deployment topology, without depending on how - events are routed or partitioned. - - Self-contained: it relies only on the database, with no event-bus partitioning contract. - - Unlike the single deployment-wide lock, it lets different learners be recorded in parallel. + - Mastery tracks the current grade exactly, including downward corrections, with no + high-water-mark surprise. - Cons: - - *It fights the batching the throughput depends on.* The chosen pipeline is fast because it - batches across learners: one bulk read, one in-memory evaluation, one bulk write per batch, - amortized over every learner in the batch. A per-learner lock is taken per learner, so work - is serialized and committed per learner (in the original per-event form, per event), which - reintroduces exactly the per-unit transaction, commit, and lock acquire/release overhead - that batching removes. Under bursty grading across many learners that per-unit overhead - dominates. - - *Its lock lifecycle is multiplied across every learner.* The single lock has one lifecycle - to operate (acquisition, timeout, stale-lock recovery on a crashed worker). A per-learner - lock needs a per-learner lock table (or keyed advisory locks) and that same lifecycle - replicated across potentially millions of learner keys, held and waited on concurrently, - with a database connection and worker tied up per contended lock. - - *The batched variant becomes a multi-lock ordering problem.* Trying to keep the batching by - locking every learner in a batch at once means holding many locks simultaneously and - acquiring them in a consistent order to avoid deadlock between batches whose learner sets - overlap: more failure modes than a single lock, for no correctness gain. - - Storing leaves (:ref:`openedx-learning-adr-0005`) widens this gap rather than narrowing it: - the per-learner write volume is now roughly the per-course leaf fan-out larger, so - processing per learner without amortizing across learners costs more than it did before. - - Why rejected: the per-learner lock buys exactly one thing over the single deployment-wide lock, - per-learner parallelism, and pays for it with per-learner lock machinery and the loss of - cross-learner batching. While parallelism is not the binding constraint (the current - expectation, since one batch pipeline keeps up with peak grading) the single lock is simpler and - correct. When parallelism does become the binding constraint, keyed partitioning (alternative 1) - provides the same per-learner serialization as a structural property, lock-free and horizontally - scalable, which strictly beats a per-learner lock. There is no operating point at which the - per-learner lock is the best option, which is why it is not the chosen design and not the - escalation path. This was an earlier design for competency mastery recording; both the chosen - batch lock and keyed partitioning supersede it. See also alternative 2, which shows that keeping - the batching while moving evaluation outside a single lock collapses into either this - per-learner isolation or the write-skew. - -4. No lock; assume same-learner conflicts are rare and tolerate them. - - Pros: - - The least machinery of any option: no lock, no partitioning-for-correctness, relying on - eventual self-healing on the next event and a reconciliation job. - - Cons: - - Correctness becomes best-effort. A concurrent same-learner conflict can leave a transient - wrong derived status. - - A wrong status lingers indefinitely if the learner receives no further relevant event. - - Unacceptable on the learner- and instructor-facing dashboards this feeds, where an - incorrect status is directly visible, and on any future credentialing that consumes it. - -5. Recompute derived levels on read instead of materializing them. - - Pros: - - Removes the write-skew hazard entirely: if nothing derived is stored, nothing derived can - drift, and the write path is trivial. - - Cons: - - Reopens :ref:`openedx-learning-adr-0002`, which deliberately materializes derived levels - for dashboard read performance. - - Moves the cost onto every read, which is the surface that decision was protecting. - - Out of scope for this decision. - -6. Real-time per-subsection events with batching on the openedx-core side. - - edx-platform emits one ``openedx-events`` event per subsection grade change as it happens, and - openedx-core absorbs the per-event stream, buffering and batching on its own side instead of - having the producer batch first. + - Gives up the coordination-free property: a non-monotone merge is not order-insensitive, so + concurrent writers can corrupt (not merely understate) a derived row. + - Requires an always-on correcting reconciliation process to converge, the kind of standing + cost this decision avoids. + - Why rejected: monotonicity is the linchpin that makes the lock-free write path correct. + Trading it away to track downward corrections reintroduces exactly the coordination problem + this decision removes, and the product treats mastery as banked anyway + (:ref:`openedx-learning-adr-0005`). - - Pros: - - Lower latency: mastery can update within seconds of a grade change rather than within a - polling interval. - - No new scheduled producer task; it reuses the point where the grade is already written. - - Cons: - - The event rate is set by grading volume and scales with however widely competencies are - adopted, which is unknown, so the consumer must be sized for a firehose it cannot bound. - - Each event is an inter-process signal and, on the openedx-core side, a durable write on or - near the hot grading path. - - Why rejected: producer-side batching bounds the event rate at the source regardless of - adoption, so the recorder is scalable by default on a very large deployment without having to - predict how widely competencies will be enabled or whether per-event volume becomes a - problem. Trading a few minutes of latency for that bound is the priority here. +3. Overwrite derived rows with the freshly computed value instead of a monotone merge. -7. openedx-core polls the persisted grade tables directly. + - Why rejected: a blind ``status := computed`` write is a lost update under concurrency, since a + worker computing from a stale sibling snapshot can overwrite a higher value another worker + just wrote. The monotone ``max`` merge is precisely what makes concurrent writes safe; without + it, mechanism 1 does not hold and a lock would be back in scope. - Instead of consuming events, openedx-core runs its own periodic query against the persisted - subsection-grade and override tables, using their indexed ``modified`` columns to find changes. +4. Recompute derived levels on read instead of materializing them. - Pros: - - Cheapest possible read: the ``modified`` indexes exist for exactly this timespan query, so - it is one indexed range scan per cycle with no per-event cost. - - No dependency on any event being emitted. + - Removes the write-skew hazard entirely: if nothing derived is stored, nothing derived can + drift, and the write path is trivial. - Cons: - - It makes openedx-core depend on edx-platform's private grade schema, an implementation - detail with no stability guarantee, rather than on a versioned contract. - - The effective grade must be recomputed by re-implementing edx-platform's separate override - layering, which will drift as the platform changes it. - - Why rejected: it breaks the layering rule (openedx-core must not depend on edx-platform) and - this decision's value of behaving identically on any deployment. A table schema is an - implementation detail that changes by migration without notice; a versioned event is a - contract. The cost saving is real but does not justify coupling the recorder to platform - internals. - -8. Move the persisted subsection-grade model into openedx-core so edx-platform imports it. + - Reopens :ref:`openedx-learning-adr-0002`, which deliberately materializes derived levels + for dashboard read performance, and moves the cost onto every read. + - Why rejected: out of scope for this decision; the read-performance tradeoff was settled in + :ref:`openedx-learning-adr-0002`. - Follow this repo's established pattern (a library-owned model that edx-platform depends on) by - relocating the subsection-grade model into openedx-core. +5. Let openedx-core obtain grades directly instead of having edx-platform push them. - - Pros: - - openedx-core could read the data in-process with no cross-boundary contract at all. - - It is the same ownership pattern openedx-core already uses for content models. - - Cons: - - The subsection-grade model is a large, deeply woven edx-platform model with extensive - signal wiring, override semantics, and migration history. - - Relocating it inverts ownership of a central platform concern and is a major migration. - - Why rejected: that pattern fits models built new in openedx-core, not a mature, central - edx-platform model. The extraction would be a large, risky effort out of all proportion to - recording competency mastery. It may be the right long-term shape if Open edX decides grades - belong in the learning core, but that is a separate, much larger decision. + Variants: openedx-core polls the persisted subsection-grade and override tables on their indexed + ``modified`` columns; or the subsection-grade model is relocated into openedx-core so edx-platform + imports it; or a swappable base model (in the style of ``AUTH_USER_MODEL``) is defined in + openedx-core and supplied by edx-platform. -9. A shared-contract or swappable grade model. + - Why rejected: all three break the layering rule that openedx-core must not depend on + edx-platform. Polling couples the recorder to a private grade schema that changes by migration + without notice and re-implements the override layering it would drift from. Relocating a + mature, deeply woven platform model is a large, risky migration out of proportion to this + feature. The swappable-model pattern is effectively a one-off for the user model, cannot be + retrofitted onto an existing table, and would be first-of-its-kind machinery here. A push from + edx-platform (Option A's call or Option B's versioned event) gets the data across the boundary + in the correct direction without any of this. - openedx-core defines a minimal base model as a contract; edx-platform supplies the concrete - table (with any extra columns it needs) and openedx-core resolves it through a setting, in the - style of Django's swappable ``AUTH_USER_MODEL``. +6. Deliver over a mandatory event bus. - - Pros: - - openedx-core reads the data without importing edx-platform, and the platform keeps freedom - to extend its own table. - - Cons: - - Django's swappable-model machinery is, in practice, a one-off for the user model; the - third-party generalizations are niche and carry hard constraints (a swappable model must - exist from the app's first migration, and retrofitting an existing table is unsupported). - - It still couples the platform to an openedx-core-dictated schema, and there is no - precedent for it in this ecosystem beyond the user-model foreign key. - - Why rejected: it would be first-of-its-kind machinery for the project, applied to a table that - already exists and so hits exactly the retrofit constraints the pattern handles worst, for a - benefit a versioned event contract already provides more cleanly. - -10. Enable and rely on the ``openedx-events`` event bus as the delivery mechanism. - - Route grade events over the Kafka or Redis event bus and have openedx-core consume them as a bus + Route grade events over the Kafka or Redis event bus and consume them in openedx-core as a bus consumer. - Pros: - - The bus is a durable buffer with native batch polling and at-least-once delivery, which - would close the durability gap without an inbox or a reconciliation command. - - A bus consumer runs in its own process, off edx-platform's workers. + - The bus is a durable buffer with native batch polling and at-least-once delivery, closing + the durability gap without an inbox or reconciliation command. - Cons: - - The event bus is not enabled in a stock edx-platform deployment. - - Requiring it makes Kafka or Redis mandatory infrastructure for any deployment that wants - competencies. - - Why rejected: mandating event-bus infrastructure is a far larger operational decision than - this feature should force on operators. The chosen design runs over ordinary in-process - signals and is transport-agnostic: it can take advantage of the bus where a deployment already - runs one, but it does not require it. + - The event bus is not enabled in a stock edx-platform deployment, so requiring it makes + Kafka or Redis mandatory infrastructure for any deployment that wants competencies. + - Why rejected: mandating event-bus infrastructure is a far larger operational imposition than + this feature should force on operators. Both options run over ordinary in-process delivery and + can take advantage of the bus where a deployment already runs one, without requiring it. + +7. Correctness by keyed partitioning. + + Partition grade-change events by ``user_id`` so every event for a learner is consumed by exactly + one worker, making same-learner events serial by construction with no lock. + + - Why rejected: this was the lock-free alternative worth considering only while correctness + required serialization. Under monotone merge, correctness no longer requires that any two + same-learner events be serialized at all, so partitioning solves a problem this decision no + longer has. It would also couple the recorder to a transport-level routing contract (native on + Kafka, application-level sharding on Redis) that monotonicity makes unnecessary. From c4354d59a32068163868e1f0af099a9697728135 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 20 Jul 2026 13:48:35 -0400 Subject: [PATCH 09/10] docs: improve atomicity pattern --- .../0002-competency-criteria-model.rst | 2 +- ...competency-mastery-concurrency-diagrams.md | 45 +++--- .../0004-competency-mastery-concurrency.rst | 133 ++++++++++-------- .../0005-competency-mastery-storage.rst | 13 +- 4 files changed, 112 insertions(+), 81 deletions(-) diff --git a/docs/openedx_learning/decisions/0002-competency-criteria-model.rst b/docs/openedx_learning/decisions/0002-competency-criteria-model.rst index ef124f6ae..87e8664c8 100644 --- a/docs/openedx_learning/decisions/0002-competency-criteria-model.rst +++ b/docs/openedx_learning/decisions/0002-competency-criteria-model.rst @@ -81,7 +81,7 @@ Decision 4. ``course_id``: Nullable foreign key to ``openedx_catalog_courserun.id`` for the course that scopes this criteria tree. 5. ``name``: string 6. ``ordering``: Indicates evaluation sequence number for this criteria group. This defines deterministic evaluation order for siblings during read-time evaluation and event-driven recomputation, and enables short-circuit evaluation. - 7. ``logic_operator``: Either “AND” or “OR” or null. This determines how children are combined at a group node ("AND" or "OR"). + 7. ``logic_operator``: Either “AND” or “OR” or null. This determines how children are combined at a group node ("AND" or "OR"). Deliberately no negation operator: keeping every combinator monotone (advancing a child status can only advance or leave unchanged its parent, never regress it) is what lets :ref:`openedx-learning-adr-0004` record concurrently without coordination. Example: A root group uses "OR" with two child groups. diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md index 0ec2f085a..fed673fea 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md @@ -28,38 +28,43 @@ flowchart TD RCV --> API ``` -## 2. Why it is correct without a lock +## 2. Why it is correct: one transaction, monotone merge, brief per-node row lock Every write is `status := max(stored, computed)`, so writes commute, repeat harmlessly, and never -regress. A conjunctive parent computed from a stale sibling view can be too low but never too high; -the re-evaluation triggered by the last child to commit reads every committed sibling and merges the -parent up to the correct value. Result: eventually correct, never over-stated, no coordination. +regress. A leaf is a single value, so its atomic merge needs no extra lock. A conjunctive parent is +computed by reading several children first, so recomputing it takes a brief `SELECT ... FOR UPDATE` +on the parent row: two updates that touch the same parent for the same learner take turns, and the +second reads the first's committed children and computes from the complete picture. This is an +ordinary single-row lock, not the deployment-wide lock the previous design used. ```mermaid sequenceDiagram autonumber - participant WA as Worker A (child L1) - participant WB as Worker B (child L2) + participant WA as Worker A (child L1 advances) + participant WB as Worker B (child L2 advances) participant DB as Mastery tables (ACTIVE + HISTORY) - Note over WA,WB: L1 and L2 are siblings under conjunctive parent G - WA->>DB: merge L1 up; commit - WB->>DB: merge L2 up; commit (last committer) - WA->>DB: re-evaluate G from committed children
(sees L1 new, L2 maybe stale) → may be too low - WB->>DB: re-evaluate G from committed children
(sees L1 and L2 committed) → correct - Note over DB: max-merge keeps the higher value → G converges to correct, never over-stated + Note over WA,WB: L1 and L2 are siblings under conjunctive parent G, same learner + WA->>DB: merge L1 (atomic max) + WB->>DB: merge L2 (atomic max) + WA->>DB: SELECT G FOR UPDATE (acquires row lock) + WB->>DB: SELECT G FOR UPDATE (waits for A) + WA->>DB: read children (L1, L2), merge G, commit → releases lock + WB->>DB: now reads L1 and L2 committed, merges G → correct + Note over DB: locks taken child-before-parent up to root → no deadlock ``` -## 3. Optional monotone backstop +## 3. Recovering a lost delivery (Option B): trailing-overlap re-scan -The only way step 2 leaves a parent low is a lost re-evaluation trigger. A periodic monotone -re-derivation repairs that: because it can only advance a status, it fixes a stalled parent without -ever corrupting one. It is not a correcting reconciliation (which a non-monotone design would need), -and can be omitted until lost triggers are actually observed. +Option A cannot lose a change (mastery shares the grade transaction). Option B carries it over an +in-process signal that can be dropped silently. The producer re-reads a short overlap window behind +its watermark each cycle, so a dropped change is re-emitted next cycle; the monotone merge makes the +re-delivery a no-op if it was already recorded. No reconciliation command and no correcting sweep. ```mermaid flowchart LR - SWEEP["Periodic re-derivation
(optional)"] --> READ["Read committed children"] - READ --> MERGE["status := max(stored, recomputed)"] - MERGE --> SAFE["Advances a stalled parent;
never regresses a correct one"] + WM["Watermark T"] --> SCAN["Query grades changed since (T - overlap)"] + SCAN --> EMIT["Emit signal/event per change"] + EMIT --> MERGE["Recorder: monotone merge
(re-delivery of a recorded change is a no-op)"] + SCAN --> ADV["Advance watermark → next scan stays a short-window seek"] ``` diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst index c53916c16..2bb55929a 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -60,9 +60,11 @@ in-process delivery, without making the event bus mandatory. Decision -------- -Recording is made **coordination-free** by leaning on the monotonicity above: no lock, no -serialized pipeline, no partitioning-for-correctness. Two mechanisms provide correctness, and they -are what any entry point below must implement. +Recording avoids global coordination by leaning on the monotonicity above: no deployment-wide lock, +no serialized pipeline, no partitioning-for-correctness. Concurrent same-learner writes are made +safe by a monotone merge plus a brief row-level lock on the one node being recomputed, not by +serializing the whole recorder. Two mechanisms provide correctness, and they are what any entry +point below must implement. **1. Every write is a monotone merge, never a blind overwrite.** A node's status is written as ``status := max(stored status, newly computed status)`` (a single ``GREATEST``-style ``UPDATE``, @@ -72,23 +74,19 @@ order: applying the same set of advances in any order, and re-applying any of th result. A late or duplicate delivery carries a status no higher than what is stored, so it is a no-op. This is why out-of-order delivery and re-delivery are harmless without sequence tracking. -**2. When a child advances, its parent is re-evaluated from the committed children.** The merge in -mechanism 1 makes a single write safe, but a *conjunctive* parent (for example "demonstrated only -when all children are demonstrated") can still be computed from a stale view of its siblings. Two -overlapping evaluations for one learner can each see only their own child's advance and each -compute a parent value that is too low, never too high. To close that gap, every child advance -re-evaluates the parent reading the *committed* child rows, and propagates upward if the parent -advances. Commits are serialized by the database, so there is always a last committer, and the -re-evaluation it triggers reads every sibling's committed value and merges the parent up to the -correct result. Derived rows are therefore *eventually correct and never over-stated*, with no -coordination between workers. In the language of the CALM theorem, a monotone computation needs no -coordination. - -The only way mechanism 2 can leave a parent low is if the last re-evaluation is never run (a lost -trigger). An optional monotone re-derivation sweep is a sound backstop: because it can only advance -a status, it repairs a stalled parent without ever corrupting one. It is not the "reconciliation to -fix wrong values" that a non-monotone design would need; it is belt-and-suspenders for a dropped -trigger, and can be omitted until lost triggers are actually observed. +**2. When a child advances, its parent is recomputed in the same transaction, under a brief row +lock on that parent.** The merge in mechanism 1 makes a single-row write safe, but a *conjunctive* +parent (for example "demonstrated only when all children are demonstrated") is computed by reading +several child rows first, so two overlapping evaluations for one learner could each read a stale +sibling and compute a parent that is too low. To prevent that, recomputing a parent takes a +row-level lock on the parent row (a ``SELECT ... FOR UPDATE``) before reading its children: two +updates that touch the same parent for the same learner take turns, and the second reads the first's +committed children and computes from the complete picture. Locks are taken child-before-parent up +the path to the root, a consistent order, so concurrent updates cannot deadlock. This is an ordinary +single-row lock, the same serialization any two concurrent writes to one row already incur; it is +*not* the deployment-wide lock the previous design used, and it only ever contends when two grade +changes for the *same learner and same node* land at once, which is rare. Because every write only +advances a status, the result is correct and never over-stated, with no global coordination. Two rules govern whether a status advances and a HISTORY row is written; both fall out of monotonicity: @@ -113,12 +111,12 @@ write. The choice is a follow-up decision. database, which makes grade and mastery genuinely atomic. - Pros: grade and mastery can never diverge; recording is real-time; no new event type is - needed, only a call from code that already runs. + needed, only a call from code that already runs; and because the leaf and its ancestors + recompute in that same transaction (mechanism 2), the whole subtree is airtight inline with + no follow-up step. - Cons: mastery work sits on the synchronous grading path, so a slow mastery query or a bug - adds latency to, or rolls back, the grade write (grades are the more critical data); - requires the shared database; and a conjunctive parent still needs a post-commit - re-evaluation (mechanism 2) to be airtight, so Option A is not purely inline unless it - accepts "heals on the learner's next event." + adds latency to, or rolls back, the grade write (grades are the more critical data); and it + requires the shared database. **Option B: record from a subsection-grade-changed signal received in openedx-core.** edx-platform emits a subsection-granular signal or event; an openedx-core receiver does the @@ -128,10 +126,10 @@ write. The choice is a follow-up decision. grade writes; all mastery logic lives in openedx-core, which is the correct owner; no shared-database requirement. - Cons: not atomic with the grade write, so there is a brief window where the grade is - written but mastery is not yet (acceptable because mastery is already eventually - consistent and monotone); needs a new subsection-granular event - (see `Prerequisite`_); and in-process delivery is best-effort, so it relies on the - durability backstop below. + written but mastery is not yet (acceptable because mastery lags but never contradicts the + grade); needs a new subsection-granular event (see `Prerequisite`_); and in-process + delivery is best-effort, so a dropped signal must be recovered by the re-scan in decision 3 + below. **Throughput and optional batching.** Because writes commute (mechanism 1), throughput no longer depends on a single serialized pipeline. Under Option B, more consumer workers can run in parallel; @@ -160,22 +158,44 @@ statuses, evaluates in memory, and bulk-writes the changed ACTIVE and HISTORY ro with batching it lags by the producer interval plus drain time. The dashboards this feeds tolerate minutes. -**Durability.** In-process delivery swallows receiver exceptions, so a dropped event could be lost -silently. Under Option B the producer (batched or per-change) re-scans a trailing overlap window -behind its watermark each cycle so a missed row is re-emitted, and the monotone merge makes -re-delivery a no-op. Under Option A the atomic transaction is the durability guarantee. An on-demand -reconciliation command is provided as an operator escape hatch; a permanent scheduled sweep is not -required (see the optional monotone backstop above). +**Transactions and durability.** Three points, each a deliberate part of this decision: + + 1. *One atomic transaction per grade change, in both options.* A change is recorded in a single + transaction that merges the leaf and recomputes its ancestors to the root (mechanism 2). The + options differ only in what else shares that transaction: under Option A the subsection grade + write is inside it, so grade and mastery commit or roll back together; under Option B the + grade is already committed and the transaction covers the mastery write alone. Either way the + mastery write is never half-applied. + + 2. *No reconciliation for correctness.* Same-learner concurrency is fully handled by mechanisms 1 + and 2, so there is nothing to reconcile after the fact: a monotone merge cannot be corrupted + by a concurrent writer, and the row-locked parent recompute always reads a complete, committed + picture. This is the key difference from a non-monotone design, which would need a correcting + sweep to converge. No such sweep, and no on-demand fix command, is part of this decision. + + 3. *Recovering a lost delivery (Option B only).* Option A cannot lose a change, because the + mastery write shares the grade transaction. Option B carries the change over a separate + in-process signal, and in production a receiver exception is swallowed and logged, so a signal + can be dropped silently. Recovery is a *trailing-overlap re-scan*: the producer tracks a + watermark (how far it has read) and each cycle queries grades changed since a few minutes + *before* the watermark, not exactly since it, so the most recent few minutes of changes are + always re-read. A change whose signal was dropped is re-emitted on the next cycle, and the + monotone merge makes the re-delivery a no-op if it was in fact already recorded. The watermark + still advances, so the query stays a cheap short-window scan, not a full-table scan. A + deployment that runs Option B with no producer at all (pure per-change signals) instead relies + on the change healing the next time that leaf is graded; if that is not acceptable it runs the + lightweight periodic re-scan. Accepted tradeoffs: - - Correctness is *eventual*, not point-in-time: a reader can briefly observe a derived status - that is too low, between a child's commit and its parent's re-evaluation. It is never too - high, and it converges. Consumers that need an exactly-consistent-at-all-times roll-up are - not served by this design. + - Mastery is internally consistent, since the leaf and its roll-ups move together in one + transaction (mechanism 2), but under Option B it lags the grade: a reader can briefly see an + updated grade whose mastery is not yet recorded. Mastery is never over-stated and never + contradicts a committed grade; it only trails it. Option A has no such lag. Consumers that need + mastery exactly in step with the grade at all times should use Option A. - Mastery is a high-water mark. A downward or revoked grade never lowers it; un-mastering is a deliberate out-of-band action. This is inherited from :ref:`openedx-learning-adr-0005` and is - the property that buys the coordination-free write path. + the property that removes the deployment-wide lock from the write path. - Correctness depends on criteria staying monotone (no negation in :ref:`openedx-learning-adr-0002`'s trees). If a future criteria feature introduces negation, that node's parent is no longer monotone and this design does not cover it. @@ -198,11 +218,13 @@ documentation listing the migration as a non-goal is correspondingly superseded. Rejected Alternatives --------------------- -1. Serialize all writers with a lock (the previous form of this decision). +1. Serialize all writers with a deployment-wide lock (the previous form of this decision). Record under a single deployment-wide lock (or a per-learner lock), so no two workers ever evaluate the same learner at once and the write-skew cannot occur. This was the earlier design - for competency mastery recording. + for competency mastery recording. This alternative is about the *global* serialization lock, not + the brief per-node row lock the chosen design uses in mechanism 2, which is a different and far + cheaper primitive. - Pros: - Correct regardless of delivery order or topology, without depending on monotonicity. @@ -215,12 +237,12 @@ Rejected Alternatives cross-learner batching that throughput would otherwise rely on. - Either way it introduces a lock and its failure modes for a guarantee monotonicity already provides. - - Why rejected: the write-skew a lock exists to prevent is a consequence of computing derived - rows non-monotonically. Once every write is a monotone merge and every child advance - re-evaluates its parent, concurrent same-learner evaluations converge on their own, so the - lock guards against a hazard that no longer exists. Removing it also restores horizontal - scalability that the locked design gave up. The lock is strictly more machinery for strictly - less throughput. + - Why rejected: the write-skew a global lock exists to prevent is a consequence of computing + derived rows non-monotonically. Once every write is a monotone merge (mechanism 1) and each + parent recompute is serialized only against concurrent writers of that same node by a brief row + lock (mechanism 2), correctness holds without serializing the whole recorder. The deployment-wide + lock forces a single pipeline and gives up horizontal scale to buy a guarantee that a per-node + row lock already provides at a fraction of the cost. 2. Allow non-monotonic mastery and repair drift with a reconciliation sweep. @@ -232,14 +254,15 @@ Rejected Alternatives - Mastery tracks the current grade exactly, including downward corrections, with no high-water-mark surprise. - Cons: - - Gives up the coordination-free property: a non-monotone merge is not order-insensitive, so - concurrent writers can corrupt (not merely understate) a derived row. + - Gives up order-insensitivity: a non-monotone merge depends on the order writes arrive, so + concurrent writers can corrupt (not merely understate) a derived row, and a brief row lock + no longer suffices to keep them correct. - Requires an always-on correcting reconciliation process to converge, the kind of standing cost this decision avoids. - - Why rejected: monotonicity is the linchpin that makes the lock-free write path correct. - Trading it away to track downward corrections reintroduces exactly the coordination problem - this decision removes, and the product treats mastery as banked anyway - (:ref:`openedx-learning-adr-0005`). + - Why rejected: monotonicity is the linchpin that keeps the write path correct with only a brief + per-node row lock and no global serialization. Trading it away to track downward corrections + reintroduces exactly the coordination problem this decision removes, and the product treats + mastery as banked anyway (:ref:`openedx-learning-adr-0005`). 3. Overwrite derived rows with the freshly computed value instead of a monotone merge. @@ -282,7 +305,7 @@ Rejected Alternatives - Pros: - The bus is a durable buffer with native batch polling and at-least-once delivery, closing - the durability gap without an inbox or reconciliation command. + Option B's delivery gap without relying on the trailing-overlap re-scan. - Cons: - The event bus is not enabled in a stock edx-platform deployment, so requiring it makes Kafka or Redis mandatory infrastructure for any deployment that wants competencies. diff --git a/docs/openedx_learning/decisions/0005-competency-mastery-storage.rst b/docs/openedx_learning/decisions/0005-competency-mastery-storage.rst index e93a6de6e..a03e1a929 100644 --- a/docs/openedx_learning/decisions/0005-competency-mastery-storage.rst +++ b/docs/openedx_learning/decisions/0005-competency-mastery-storage.rst @@ -149,11 +149,14 @@ work, and consciously declines the ones edx-platform did not need: effective-source-timestamp out-of-order defense in :ref:`openedx-learning-adr-0004` (mirroring edx-platform's check of the source score's timestamp before trusting a queued recompute). - - *Batch the read and write paths.* The recorder reads current statuses in bulk and writes leaf - ACTIVE (bulk upsert), leaf HISTORY (``bulk_create``), and the rolled-up group/competency rows - in a small, fixed number of round-trips per batch, independent of the number of rows in the - batch. This is the same "prefetch per cohort, bulk-write" posture edx-platform uses and is the - mechanism by which :ref:`openedx-learning-adr-0004`'s single serialized pipeline keeps up. + - *Batch the read and write paths, where batching is used.* When a deployment adopts the + optional batching described in :ref:`openedx-learning-adr-0004`, the recorder reads current + statuses in bulk and writes leaf ACTIVE (bulk upsert), leaf HISTORY (``bulk_create``), and the + rolled-up group/competency rows in a small, fixed number of round-trips per batch, independent + of the number of rows in the batch. This is the same "prefetch per cohort, bulk-write" posture + edx-platform uses. Recording is correct without batching too (:ref:`openedx-learning-adr-0004`'s + monotone merge does not depend on it); batching is a throughput optimization for high-volume + deployments, not a correctness mechanism. - *Chunk mass recompute and provide a kill switch.* The bulk recompute triggered by a structural criteria edit is chunked by a configurable batch size and can be halted by an operational switch, mirroring edx-platform's ``ComputeGradesSetting`` batch size and its From 59c5467bc5a2f5b5428e096cde53476cd475bb5d Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 20 Jul 2026 16:01:03 -0400 Subject: [PATCH 10/10] docs: clean up ADR --- ...competency-mastery-concurrency-diagrams.md | 43 ++-- .../0004-competency-mastery-concurrency.rst | 225 +++++------------- 2 files changed, 88 insertions(+), 180 deletions(-) diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md index fed673fea..4ce4545dc 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency-diagrams.md @@ -4,28 +4,29 @@ Companion diagrams for `0004-competency-mastery-concurrency.rst`. They are kept they render natively on GitHub; they are not part of the Sphinx/readthedocs build. Refer to the ADR for the authoritative decision text. -## 1. Entry points: two options (no preference) +## 1. Entry point and durable hand-off -Both options push a grade change from edx-platform into openedx-core, which records it with a -monotone merge and re-evaluates the parents. They differ only in where the leaf write happens and -whether it is atomic with the grade write. Batching (the dashed producer) is optional and applies to -Option B. +edx-platform emits a subsection-grade-changed event; an openedx-core receiver only enqueues an +idempotent, retriable task, and that task does the monotone merge and re-evaluates the parents. The +thin receiver plus retriable task is edx-platform's own grade-recompute pattern, and it is what makes +a dropped in-process signal survivable. Batching (the dashed producer) is optional (decision 6). ```mermaid flowchart TD subgraph EP["edx-platform (higher layer)"] GRADE["Subsection grade write"] + SIG["Emit subsection-grade-changed event"] PROD["Optional producer worker:
poll changed grades, coalesce,
emit bounded batch events"] end subgraph CORE["openedx-core (lower layer)"] - API["Recording API:
monotone merge + re-evaluate parents"] - RCV["Signal/event receiver"] + RCV["Receiver: enqueue task only"] + TASK["Task (retriable, idempotent):
monotone merge + re-evaluate parents
in one transaction"] end - GRADE -->|"Option A: synchronous call
inside the same transaction
(shared DB, atomic)"| API - GRADE -->|"Option B: emit subsection-grade-changed
signal/event"| RCV + GRADE --> SIG + SIG -->|"in-process event"| RCV PROD -.->|"optional batched events"| RCV - RCV --> API + RCV -->|"apply_async"| TASK ``` ## 2. Why it is correct: one transaction, monotone merge, brief per-node row lock @@ -54,17 +55,21 @@ sequenceDiagram Note over DB: locks taken child-before-parent up to root → no deadlock ``` -## 3. Recovering a lost delivery (Option B): trailing-overlap re-scan +## 3. Recovering a lost delivery: retriable task, not a re-scan -Option A cannot lose a change (mastery shares the grade transaction). Option B carries it over an -in-process signal that can be dropped silently. The producer re-reads a short overlap window behind -its watermark each cycle, so a dropped change is re-emitted next cycle; the monotone merge makes the -re-delivery a no-op if it was already recorded. No reconciliation command and no correcting sweep. +The in-process event can be dropped silently (``send_robust`` catches and logs a receiver +exception). Durability follows edx-platform's grades pattern: the receiver only enqueues, and the +task queue's at-least-once delivery plus task retries plus the monotone merge's idempotency carry the +work through. No scheduled reconciliation sweep; a genuine loss (enqueue failed during a broker +outage) is recovered by an operator-run bulk recompute, exactly as edx-platform recovers a missed +grade recompute. ```mermaid flowchart LR - WM["Watermark T"] --> SCAN["Query grades changed since (T - overlap)"] - SCAN --> EMIT["Emit signal/event per change"] - EMIT --> MERGE["Recorder: monotone merge
(re-delivery of a recorded change is a no-op)"] - SCAN --> ADV["Advance watermark → next scan stays a short-window seek"] + RCV["Receiver (send_robust:
exception logged, not fatal)"] -->|"apply_async"| Q["Task queue
(at-least-once, persisted)"] + Q --> TASK["Task: monotone merge + roll-up"] + TASK -->|"transient error"| RETRY["self.retry"] + RETRY --> Q + TASK -->|"stale / duplicate"| NOOP["no-op via max-merge +
effective-source-timestamp check"] + BROKER["Rare: enqueue lost in broker outage"] -.->|"operator escape hatch"| CMD["Manual bulk recompute
(monotone; only advances)"] ``` diff --git a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst index 2bb55929a..c27d8a4d3 100644 --- a/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst +++ b/docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst @@ -5,7 +5,7 @@ Status ------ -Proposed. Contingent on a cross-repo prerequisite (see `Prerequisite`_). +Proposed. A companion set of diagrams for this decision lives alongside it in ``0004-competency-mastery-concurrency-diagrams.md``. @@ -23,25 +23,10 @@ re-writes the derived rows from that leaf up to the competency root. Per the current status for a learner and node, plus an append-only HISTORY row per genuine status advance. -**The property this decision rests on: mastery only ever moves forward.** Per +**Monotonicity: competency statuses only ever move forward.** Per :ref:`openedx-learning-adr-0005`, every node, at every level, advances through a small status lattice (``AttemptedNotDemonstrated`` to ``PartiallyAttempted`` to ``Demonstrated``) and is never -regressed automatically. A status is a high-water mark: once banked it stays, even when a later -grade correction is lower. This is a domain rule, not an implementation convenience, and the rest -of this decision is built on it. It holds only while the criteria that combine child statuses into -a parent are *monotone* boolean functions (AND, OR, k-of-n thresholds), so that advancing any child -can only advance the parent. :ref:`openedx-learning-adr-0002`'s criteria trees contain no negation, -which is the assumption that keeps parents monotone; a ``NOT`` node would break it. - -How grade changes reach openedx-core is constrained by layering. openedx-core must never import from -edx-platform and cannot read its grade tables. edx-platform is the higher layer and may depend on -openedx-core, so a grade change is *pushed* into openedx-core by edx-platform, either as a -synchronous call into an openedx-core recording API or as an ``openedx-events`` event. What -openedx-core may not do is reach back to read grades itself. ``openedx-events`` also delivers -in-process by default, because the event bus is not enabled in a stock deployment: a receiver runs -synchronously in the producer's worker and, in production, a receiver exception is swallowed and -logged rather than retried. In-process delivery is therefore best-effort, not durable, and the -recorder cannot assume the event bus is present. +lowered later. This holds for leaf nodes, group nodes, and top-level competency masteries. Two forces shape how recording should happen: @@ -60,22 +45,16 @@ in-process delivery, without making the event bus mandatory. Decision -------- -Recording avoids global coordination by leaning on the monotonicity above: no deployment-wide lock, -no serialized pipeline, no partitioning-for-correctness. Concurrent same-learner writes are made -safe by a monotone merge plus a brief row-level lock on the one node being recomputed, not by -serializing the whole recorder. Two mechanisms provide correctness, and they are what any entry -point below must implement. **1. Every write is a monotone merge, never a blind overwrite.** A node's status is written as ``status := max(stored status, newly computed status)`` (a single ``GREATEST``-style ``UPDATE``, atomic at the row for the duration of that one statement, with no application-level lock). Because the merge takes the higher of the two values, it is commutative, idempotent, and insensitive to -order: applying the same set of advances in any order, and re-applying any of them, yields the same -result. A late or duplicate delivery carries a status no higher than what is stored, so it is a -no-op. This is why out-of-order delivery and re-delivery are harmless without sequence tracking. +order. This is why out-of-order delivery and re-delivery are harmless without sequence tracking. -**2. When a child advances, its parent is recomputed in the same transaction, under a brief row -lock on that parent.** The merge in mechanism 1 makes a single-row write safe, but a *conjunctive* + +**2. When a child advances, its parent is recomputed in the same transaction, under a brief row lock on that parent.** +The merge in mechanism 1 makes a single-row write safe, but a *conjunctive* parent (for example "demonstrated only when all children are demonstrated") is computed by reading several child rows first, so two overlapping evaluations for one learner could each read a stale sibling and compute a parent that is too low. To prevent that, recomputing a parent takes a @@ -83,137 +62,41 @@ row-level lock on the parent row (a ``SELECT ... FOR UPDATE``) before reading it updates that touch the same parent for the same learner take turns, and the second reads the first's committed children and computes from the complete picture. Locks are taken child-before-parent up the path to the root, a consistent order, so concurrent updates cannot deadlock. This is an ordinary -single-row lock, the same serialization any two concurrent writes to one row already incur; it is -*not* the deployment-wide lock the previous design used, and it only ever contends when two grade -changes for the *same learner and same node* land at once, which is rare. Because every write only -advances a status, the result is correct and never over-stated, with no global coordination. - -Two rules govern whether a status advances and a HISTORY row is written; both fall out of -monotonicity: - - - *Out-of-order defense.* A change older than the current leaf's effective source timestamp is - ignored, so a late arrival cannot regress a newer status. The monotone merge already enforces - this for the stored status; the timestamp check avoids writing a spurious HISTORY row for a - stale advance. - - *Advance-only; no automatic regression.* Once a status reaches a level it is banked at every - level including the leaf (:ref:`openedx-learning-adr-0005`). A downward grade correction does - not advance the status and writes no HISTORY row, which is what bounds HISTORY by - monotonicity. Reversing a banked status is a separate administrative action, out of scope - here. - -**Entry point: two options, presented without a preference.** Both implement the two mechanisms -above; they differ only in *where the leaf write happens* and whether it is atomic with the grade -write. The choice is a follow-up decision. - - **Option A: record inside the subsection-grade transaction.** edx-platform wraps its subsection - grade write and a synchronous call into an openedx-core recording API in one - ``transaction.atomic()``. This assumes mastery tables and the subsection-grade table share one - database, which makes grade and mastery genuinely atomic. - - - Pros: grade and mastery can never diverge; recording is real-time; no new event type is - needed, only a call from code that already runs; and because the leaf and its ancestors - recompute in that same transaction (mechanism 2), the whole subtree is airtight inline with - no follow-up step. - - Cons: mastery work sits on the synchronous grading path, so a slow mastery query or a bug - adds latency to, or rolls back, the grade write (grades are the more critical data); and it - requires the shared database. - - **Option B: record from a subsection-grade-changed signal received in openedx-core.** - edx-platform emits a subsection-granular signal or event; an openedx-core receiver does the - monotone merge and the upward re-evaluation. - - - Pros: keeps mastery off the grade transaction, so mastery failures or latency never touch - grade writes; all mastery logic lives in openedx-core, which is the correct owner; no - shared-database requirement. - - Cons: not atomic with the grade write, so there is a brief window where the grade is - written but mastery is not yet (acceptable because mastery lags but never contradicts the - grade); needs a new subsection-granular event (see `Prerequisite`_); and in-process - delivery is best-effort, so a dropped signal must be recovered by the re-scan in decision 3 - below. - -**Throughput and optional batching.** Because writes commute (mechanism 1), throughput no longer -depends on a single serialized pipeline. Under Option B, more consumer workers can run in parallel; -under Option A, throughput is bounded by the grade path itself. Horizontal scale, which a lock-based -design would sacrifice, is available for free. Batching, a scheduled producer worker on the -edx-platform side that polls changed grades, coalesces them, and emits bounded batch events, is -therefore *optional*: it is one way to raise throughput, now competing with simply adding parallel -consumers rather than being the only route to it. Without batching, Option B receives one signal -per change and Option A needs no producer at all. Batching is *necessary* rather than merely -nice-to-have when: - - - Sustained per-change volume exceeds what parallel per-change consumers (Option B) or the - grade path (Option A) can absorb, so the fixed per-change overhead (competency resolution, - round-trips, task scheduling) dominates and only coalescing into bulk operations keeps up. - - The stored-leaf write amplification (:ref:`openedx-learning-adr-0005`; up to ~200x the - per-course leaf fan-out) makes per-change writes individually expensive, so bulk upserts - across a batch are needed to bound the statement count. - - Grading bursts (an exam closing, a bulk regrade) produce spikes that would otherwise saturate - consumers or the grade path. - +single-row lock. + +**3. Out-of-order defense.** A change older than the current leaf's effective source timestamp is +ignored, so a late arrival cannot regress a newer status. The monotone merge already enforces +this for the stored status; the timestamp check avoids writing a spurious HISTORY row for a +stale advance. + +**4. Advance-only; no automatic regression.** Once a status reaches a level it is banked at every +level including the leaf (:ref:`openedx-learning-adr-0005`). A downward grade correction does +not advance the status and writes no HISTORY row, which is what bounds HISTORY by +monotonicity. Reversing a banked status is a separate administrative action, out of scope +here. + +**5. Entry point: record from a subsection-grade-changed signal received in openedx-core.** +edx-platform emits a subsection-granular openedx-event signal; an openedx-core receiver enqueues a +task that does the monotone merge and the upward re-evaluation (see decision 8 for why the receiver +only enqueues). + +**6. Optional batching.** With this ADR, consumer workers can run in parallel, +scaling horizontally. If further performance increase is needed, batching can be introduced: +In that case, a scheduled producer worker on the +edx-platform side polls changed grades, coalesces them, and emits bounded batch events. Resolving which competencies a subsection feeds is learner-independent, so when batching is used it is cached and de-duplicated across the batch, and each batch does one bulk read of current ACTIVE statuses, evaluates in memory, and bulk-writes the changed ACTIVE and HISTORY rows per level. -**Latency.** Option A records in real time. Option B without batching lags by receiver processing; -with batching it lags by the producer interval plus drain time. The dashboards this feeds tolerate -minutes. - -**Transactions and durability.** Three points, each a deliberate part of this decision: - - 1. *One atomic transaction per grade change, in both options.* A change is recorded in a single - transaction that merges the leaf and recomputes its ancestors to the root (mechanism 2). The - options differ only in what else shares that transaction: under Option A the subsection grade - write is inside it, so grade and mastery commit or roll back together; under Option B the - grade is already committed and the transaction covers the mastery write alone. Either way the - mastery write is never half-applied. - - 2. *No reconciliation for correctness.* Same-learner concurrency is fully handled by mechanisms 1 - and 2, so there is nothing to reconcile after the fact: a monotone merge cannot be corrupted - by a concurrent writer, and the row-locked parent recompute always reads a complete, committed - picture. This is the key difference from a non-monotone design, which would need a correcting - sweep to converge. No such sweep, and no on-demand fix command, is part of this decision. - - 3. *Recovering a lost delivery (Option B only).* Option A cannot lose a change, because the - mastery write shares the grade transaction. Option B carries the change over a separate - in-process signal, and in production a receiver exception is swallowed and logged, so a signal - can be dropped silently. Recovery is a *trailing-overlap re-scan*: the producer tracks a - watermark (how far it has read) and each cycle queries grades changed since a few minutes - *before* the watermark, not exactly since it, so the most recent few minutes of changes are - always re-read. A change whose signal was dropped is re-emitted on the next cycle, and the - monotone merge makes the re-delivery a no-op if it was in fact already recorded. The watermark - still advances, so the query stays a cheap short-window scan, not a full-table scan. A - deployment that runs Option B with no producer at all (pure per-change signals) instead relies - on the change healing the next time that leaf is graded; if that is not acceptable it runs the - lightweight periodic re-scan. - -Accepted tradeoffs: - - - Mastery is internally consistent, since the leaf and its roll-ups move together in one - transaction (mechanism 2), but under Option B it lags the grade: a reader can briefly see an - updated grade whose mastery is not yet recorded. Mastery is never over-stated and never - contradicts a committed grade; it only trails it. Option A has no such lag. Consumers that need - mastery exactly in step with the grade at all times should use Option A. - - Mastery is a high-water mark. A downward or revoked grade never lowers it; un-mastering is a - deliberate out-of-band action. This is inherited from :ref:`openedx-learning-adr-0005` and is - the property that removes the deployment-wide lock from the write path. - - Correctness depends on criteria staying monotone (no negation in - :ref:`openedx-learning-adr-0002`'s trees). If a future criteria feature introduces negation, - that node's parent is no longer monotone and this design does not cover it. - - Both options need edx-platform code, and Option B needs a new ``openedx-events`` event - (see `Prerequisite`_), which is cross-repo coordination, though it keeps the dependency - direction correct. - -Prerequisite ------------- -This decision requires cross-repo work in edx-platform, differing by option. Option A requires the -subsection grade write to call an openedx-core recording API within its transaction, and the mastery -tables to be routed to the same database. Option B requires a scheduled or signal-driven producer -and the ``openedx-events`` event or events it emits; the batched variant additionally requires the -polling-and-coalescing producer worker. The edx-platform-side design (task location, Celery queue, -retry/backoff, watermark storage, crash recovery) is out of scope here and belongs in that companion -work. :ref:`openedx-learning-adr-0001` rejected this migration (its rejected alternative 8) as out of -scope at the time; this decision takes it up as a now-scheduled prerequisite, and any project -documentation listing the migration as a non-goal is correspondingly superseded. +**7. One atomic transaction per grade change.** The recording task does the leaf merge and the +upward roll-up (mechanism 2) in one database transaction, so the leaf and every affected ancestor +for a change commit together or not at all. This transaction is the task's own; it is not shared +with edx-platform's grade write (contrast rejected alternative 8), so mastery trails the grade by +the enqueue-and-run delay rather than committing atomically with it. + +**8. Enqueuing a celery task.** The receiver enqueues an async celery task +to do the work to ensure durability. + Rejected Alternatives --------------------- @@ -295,8 +178,8 @@ Rejected Alternatives mature, deeply woven platform model is a large, risky migration out of proportion to this feature. The swappable-model pattern is effectively a one-off for the user model, cannot be retrofitted onto an existing table, and would be first-of-its-kind machinery here. A push from - edx-platform (Option A's call or Option B's versioned event) gets the data across the boundary - in the correct direction without any of this. + edx-platform (the versioned event this decision uses, or the synchronous call of rejected + alternative 8) gets the data across the boundary in the correct direction without any of this. 6. Deliver over a mandatory event bus. @@ -304,14 +187,15 @@ Rejected Alternatives consumer. - Pros: - - The bus is a durable buffer with native batch polling and at-least-once delivery, closing - Option B's delivery gap without relying on the trailing-overlap re-scan. + - The bus is a durable buffer with native batch polling and at-least-once delivery, as a + persistent log rather than the in-process signal plus task-queue retries this decision uses. - Cons: - The event bus is not enabled in a stock edx-platform deployment, so requiring it makes Kafka or Redis mandatory infrastructure for any deployment that wants competencies. - Why rejected: mandating event-bus infrastructure is a far larger operational imposition than - this feature should force on operators. Both options run over ordinary in-process delivery and - can take advantage of the bus where a deployment already runs one, without requiring it. + this feature should force on operators. The chosen design runs over ordinary in-process + delivery and can take advantage of the bus where a deployment already runs one, without + requiring it. 7. Correctness by keyed partitioning. @@ -323,3 +207,22 @@ Rejected Alternatives same-learner events be serialized at all, so partitioning solves a problem this decision no longer has. It would also couple the recorder to a transport-level routing contract (native on Kafka, application-level sharding on Redis) that monotonicity makes unnecessary. + +8. Record inside a subsection-grade transaction. + + edx-platform already synchronously, in-process, handles subsection grade changes. + This would wrap a subsection grade change in a transaction that includes competency + mastery updates. + + - Pros: grade and mastery can never diverge; recording is real-time; no new event type is + needed, only a call from code that already runs; and because the leaf and its ancestors + recompute in that same transaction (mechanism 2), the whole subtree is airtight inline with + no follow-up step. + - Cons: mastery work sits on the synchronous grading path, so a slow mastery query or a bug + adds latency to, or rolls back, the grade write (grades are the more critical data); and it + requires the shared database. + - Why rejected: we do not intend for mastery to always be tied to a subsection grade. + We've actually proposed tying mastery to several other things (course completion, unit grade + (a concept that doesn't exist in the platform currently), problem grade, and rubric criterion grade + (also a concept that doesn't exist in the platform currently)), and tying a mastery to a subsection + grade would contradict that.