mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Clarify restart-safe batches and scripted UI handler tests
Address Nikola's batching and UI-handler feedback on PR #132. Preserve existing false-positive guards and defer the unconfirmed IsHandled policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f841a18b-535a-498e-96f2-f279b5378da5
This commit is contained in:
parent
35d0966a8d
commit
0561b12c96
7 changed files with 155 additions and 46 deletions
|
|
@ -1,30 +1,36 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [commit, commit-in-loop, per-row-commit, checkpoint, bounded-checkpoint, watermark, topnumberofrows]
|
||||
keywords: [commit, commit-in-loop, checkpoint, watermark, retry, idempotent, elapsed-time, batch, topnumberofrows]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not Commit inside loops
|
||||
# Commit batches at restart-safe business boundaries
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Commit ends the current write transaction. Calling it inside a per-row loop usually produces one transaction per iteration and loses the ability to roll back the whole operation atomically; it also interferes with batching. Most loops need no explicit Commit at all — AL auto-commits the enclosing code module on successful completion (see `understand-implicit-transaction-boundary.md`).
|
||||
[Commit ends the current write transaction](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/database/database-commit-method). Committing every row can add transaction overhead and prevent whole-operation rollback, but deliberate commits after N completed business units or an elapsed-time threshold can be valid for long-running work. Most loops need no explicit Commit at all; see [implicit transaction boundaries](understand-implicit-transaction-boundary.md).
|
||||
|
||||
A durability checkpoint inside an outer batch loop can be valid only when the same transaction persists a progress marker or state that makes retries strictly exclude completed work, the checkpoint follows a complete business unit, and errors propagate instead of being swallowed. Restart safety and bounded retrieval are separate requirements: a persisted watermark can make retries safe, but an outer `FindSet` over the full remaining tail with periodic commits still retrieves the complete set because [`FindSet` is not implemented as `TOP X`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/optimize-sql-al-database-methods-and-performance-on-server#get-find-findset-and-next).
|
||||
Restart safety concerns business effects, not whether a retry revisits a row. A durable checkpoint or processed state can exclude completed work, while demonstrably idempotent replay or durable deduplication can make revisiting it safe. For example, repeating a pure uppercase-name assignment wastes work but does not by itself demonstrate data corruption; repeating an increment can apply it twice.
|
||||
|
||||
## Best Practice
|
||||
|
||||
If the batch is large enough that a single transaction is untenable, use an ordered primary-key watermark and retrieve a bounded next-N key list. The sample uses a query capped by [`TopNumberOfRows`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/query/queryinstance-topnumberofrows-method) to fill a temporary key buffer, then takes update locks and modifies only those exact keys. It does not reconstruct an inclusive first-to-last range that concurrent inserts could expand. Persist the last selected key in the same transaction as the completed chunk, then commit after the bounded helper returns. Use a stable key and define how a later run handles records inserted at or below an already committed watermark. Let errors escape so failed work is not recorded as complete. A `Codeunit.Run` boundary can also own a chunk when its implicit commit and error behavior fit the caller — see `codeunit-run-as-atomic-sub-operation.md`.
|
||||
Choose the commit cadence separately from the retry strategy. Check the row count or elapsed time only after a complete business unit, and commit any final partial batch. A time threshold checked between units is not a fixed-duration guarantee: retrieval, locking, or a single slow unit can exceed it. Let errors propagate so uncommitted work rolls back.
|
||||
|
||||
When correctness depends on excluding completed work, persist its checkpoint or processed state in the same transaction as the corresponding business changes. Resume from that committed state, never from a key merely selected for future processing. Alternatively, establish that replay is idempotent or deduplicated for all effects, including external effects; a missing watermark alone is not a correctness finding.
|
||||
|
||||
Bounded retrieval is a separate performance requirement. Periodic commits do not cap a full-tail `FindSet`, because [`FindSet` is not implemented as `TOP X`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/optimize-sql-al-database-methods-and-performance-on-server#get-find-findset-and-next). When a bounded next-N batch is needed, a query capped by [`TopNumberOfRows`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/query/queryinstance-topnumberofrows-method) can fill a temporary key buffer; process only those keys, not an inclusive range that concurrent inserts could expand. Use a stable ordering key and define how to handle records inserted at or below a committed watermark. Do not require bounded retrieval solely because a loop commits.
|
||||
|
||||
The paired samples apply a one-time credit-limit increase with one worker over a stable customer set. Both select at most 500 exact keys and finish a chunk after processing them or reaching a one-minute elapsed-time threshold, whichever is observed first. The good sample commits the last processed key with the increases; the bad sample keeps that key only in memory, so a retry can increase already committed limits again. AL DateTime subtraction produces a [Duration in milliseconds](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/duration/duration-data-type).
|
||||
|
||||
See sample: [`avoid-commit-inside-loops.good.al`](avoid-commit-inside-loops.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Placing Commit inside `repeat ... until Next() = 0` without persisted progress is almost always a mistake: retries re-enter already committed work, while the cost of starting a transaction on every row dominates the operation. A progress variable held only in memory is not restart-safe. A full-tail `FindSet` with a commit every N rows is not bounded retrieval, even if a persisted watermark makes it restart-safe. A capped query that discovers only an upper key and then re-reads an inclusive key range is not exact batching either; concurrent inserts inside that range can enlarge the checkpoint.
|
||||
Committing an incomplete business unit, persisting a checkpoint ahead of its business changes, or replaying committed non-idempotent effects with only an in-memory progress variable and no deduplication. In the last case, identify the effect a retry duplicates rather than treating all repeated work as corruption. Committing every row without a reason can also waste transaction overhead; this is distinct from deliberate row-count or elapsed-time batching.
|
||||
|
||||
See sample: [`avoid-commit-inside-loops.bad.al`](avoid-commit-inside-loops.bad.al).
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue