MemberJunction / MemberJunction/MJ

Cross-process lost updates from write-all-columns spUpdate — deferred by #4251, referenced to a closed issue

Open
#4,339 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

## Summary

#4251 fixes lost updates on credential rows by serializing writes through a per-credential promise chain. It is explicit that this closes the race **within one process only**, and defers the cross-process case:

> Cross-process writers can still clobber each other's columns — that is a property of the write-all-columns `spUpdate` semantics affecting every entity, tracked by the sparse-update work in #2552

**#2552 does not track this.** It is CLOSED, and its subject is retiring `_Clear` companions via universal JSON-arg CRUD sprocs — a PostgreSQL 100-argument limit problem. Searching its body for `sparse`, `concurren`, `clobber`, `lost update`, `race` returns **zero** matches. I also could not find any open issue covering write-all-columns clobbering.

So once #4251 merges, a known and deliberately-scoped-out defect has a dangling reference and no owner. This issue is that owner.

## The defect

`BaseEntity.Save()` generates an UPDATE over every `IsSPParameter` field, reading `field.Value` at SQL-build time — it does not restrict to dirty fields. Two processes that load the same row, each change a different column, and each save will both write **all** columns. The second commit silently reverts the first's change.

#4251 makes this concrete rather than theoretical: against live SQL Server, an `updateCredential` following any `getCredential` lost its value **25–50% of the time**, and even "passing" runs had the mirror failure — the update's full-row save clobbered the in-flight `LastUsedAt` write. One of the two writes always lost.

Serializing within a process removes the intra-process window. Two MJAPI instances behind a load balancer still have it.

## Why it is worth solving generally

It affects every entity, not credentials. Anything with a background "touch" column — last-used, last-run, progress — is a standing writer that can revert a legitimate concurrent edit to any other column on the same row.

## Possible directions

Not a proposal, just the shape of the option space:

- **Sparse UPDATE** — emit only dirty columns. Interacts with the `_Clear` companion mechanism that #2552 was about, which is presumably why they were conflated.
- **Optimistic concurrency** — a row version checked on update, failing loudly instead of silently losing a write.
- **Neither, documented** — decide the platform's write semantics are last-writer-wins per column-set and say so, so callers know to serialize themselves as #4251 does.

The third is a legitimate answer, but it should be a decision rather than the current state of nobody having noticed.

Follow-up to #4251. Superseding the incorrect reference to #2552.

Contributor guide

Open the contributing guide

Research direction

Start with BaseEntity.Save() and the write-all-columns spUpdate behavior described in the issue, then read #4251 for the single-process mitigation and verify the status of #2552. The work is not ready for a straightforward patch: first decide between sparse updates, optimistic concurrency, or documented semantics, then define how cross-process concurrent writes will be validated.

Written by the indexing model from the issue text.

Assessment

Tech stack
sql
Domain
database
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.