Stefan
b26c24ef98
- Improved user group deletion logic to accurately report success or failure. - Updated user group renaming checks to prevent unintended inserts or updates. - Added documentation on transaction blocking risks during full resets in stored procedures. - Removed unnecessary login scripts from database project.
66 lines
4.1 KiB
Markdown
66 lines
4.1 KiB
Markdown
---
|
|
status: Accepted
|
|
date: 2026-09-16
|
|
applyTo:
|
|
- "Fuchs_Database/dbo/Stored Procedures/mfr__updt__*.sql"
|
|
- "Fuchs_DataService/FdsMfrClient.cs"
|
|
supersededBy: ""
|
|
---
|
|
|
|
# 0022 — MFR sync transaction blocking during full resets is an accepted risk
|
|
|
|
## Context
|
|
Every `mfr__updt__*` stored procedure (25 of them, one per synced mfr entity table) wraps its
|
|
DELETE-stale-references + MERGE-upsert logic in a single explicit `BEGIN TRANSACTION` /
|
|
`COMMIT TRANSACTION`, with `SET XACT_ABORT ON` and a `TRY/CATCH` that rolls back and logs to
|
|
`fds__admin_dberrors` on failure. This gives all-or-nothing atomicity for one entity's sync
|
|
batch: either every stale side-table row is cleaned up and the target table is upserted, or
|
|
none of it is, so a mid-batch failure can never leave orphaned side-table rows alongside a
|
|
stale/missing target row.
|
|
|
|
`FdsMfrClient.cs`'s `CommandAfter` (which invokes the `mfr__updt__*` proc) and `CommandBefore`/
|
|
staging `SqlBulkCopy` both use a 600s command timeout (`MergeCommandTimeoutSeconds`), raised
|
|
from `SqlCommand`'s 30s default specifically so a **full reset** of a large entity table (not
|
|
the normal incremental sync, which only carries the rows that changed since the last run) has
|
|
room to complete. Before that timeout was raised, a slow full-reset batch would self-abort at
|
|
30s — the client cancellation would cancel the batch and roll back the open transaction —
|
|
capping how long the transaction (and the locks it holds on the target table and the affected
|
|
side tables) could stay open. With the timeout at 600s, a genuinely large full-reset batch can
|
|
now hold that same transaction, and therefore those same locks, for up to ten minutes, during
|
|
which other readers of the live `mfr__*` tables (e.g. intranet pages querying companies,
|
|
contacts, invoices, …) can block waiting for the write transaction to release its locks —
|
|
instead of blocking, at most, on one DELETE or one MERGE statement individually.
|
|
|
|
## Decision
|
|
Accept this as a known, documented risk rather than changing behavior. Full resets are rare
|
|
(a manual/administrative operation, not the routine incremental sync path) and are expected to
|
|
run outside of business hours. The atomicity the transaction provides — no orphaned side-table
|
|
rows left behind by a failed sync — is worth more than shortening the worst-case blocking
|
|
window for an operation this infrequent.
|
|
|
|
## Consequences
|
|
- A full reset of a large `mfr__*` entity table can block ordinary intranet reads of that
|
|
table (and the side tables `mfr__*PartnerSet`, `mfr__#customvalues`, `mfr__#locations`,
|
|
`mfr__comments`) for as long as that reset's DELETE+MERGE takes to execute, up to the 600s
|
|
command timeout.
|
|
- Anyone scheduling or manually triggering a full reset of a large entity table should do so
|
|
during low-traffic hours, same as any other maintenance operation with table-level locking.
|
|
- If full resets become frequent enough, or large enough, for this blocking window to matter in
|
|
practice, revisit this decision — options considered and rejected here (see below) remain
|
|
available.
|
|
|
|
## Alternatives considered
|
|
- **Drop the explicit transaction**, letting each DELETE/MERGE statement auto-commit
|
|
individually (the behavior before the transaction wrapping was added). Shortens the lock
|
|
window back to per-statement, but reintroduces the exact orphaned-data risk on mid-batch
|
|
failure that the transaction wrapping was added to prevent. Rejected — full-batch atomicity
|
|
was a deliberate improvement and giving it up trades a rare, low-traffic-hours blocking issue
|
|
for a correctness regression on every sync.
|
|
- **Enable Read Committed Snapshot Isolation (RCSI)** (`ALTER DATABASE ... SET
|
|
READ_COMMITTED_SNAPSHOT ON`) so readers use row versioning instead of blocking on the writer's
|
|
locks, keeping the transaction's atomicity intact while removing reader blocking entirely.
|
|
This is the architecturally cleanest fix, but it is a database-wide isolation behavior change
|
|
(affects every table, every query, tempdb version-store load) that needs its own testing and
|
|
sign-off rather than being folded into this change. Left as the preferred follow-up if the
|
|
accepted risk above turns out not to be acceptable in practice.
|