Fix race condition in ViewCount with atomic SQL updates#193
Fix race condition in ViewCount with atomic SQL updates#193
Conversation
Co-authored-by: hishamco <3237266+hishamco@users.noreply.github.com>
…syntax Co-authored-by: hishamco <3237266+hishamco@users.noreply.github.com>
|
@hishamco This PR is even worse. Which AI model was used to generate it? Because this is not a good PR. The PR directly updates the Document record by scanning the JSON document, which is slow, doesn’t solve the underlying problem, and ends up updating every version of the document on every view. The only approach I see working efficiently here is to introduce a dedicated table for tracking views. Every time a user clicks on a content item, insert a new record into this table (e.g., ContentItemViewTracker with ContentItemId, UserId, DateTime). Then, add a background service that runs every 10 minutes to aggregate the data and update a stats table (e.g., ContentItemViewTrackerStats with ContentItemId, Total). This way, view tracking is non-blocking since it’s just an insert, and the aggregation happens asynchronously in the background, avoiding any impact on production traffic. Yes, this means the counter won’t update immediately, but it’s a safe and scalable approach for a production application. If an immediate update is required, an alternative is to skip the raw tracking table and upsert directly into ContentItemViewTrackerStats: insert a new record when a ContentItemId is viewed for the first time, or increment the Total column if it already exists. |
Don't care, I thought he would give me a clue :)
This is HUGE!!
This makes sense, but again, how does the OC avoid a race condition while editing a content item, which is a common use case while editing by multiple authors, @sebastienros? I think you talked about this a long time back, right? |
|
What is the goal? Prevent multiple editors (humans) from editing the same content item concurrently? |
|
Yes, in my case, avoid multiple users accessing the same content item, which modifies the |
|
So I think like @MikeAlhayek said this should use a dedicated table (or document, though not easier at all). This would represent actual locks, who owns it, and when it was acquired (so it can be timed out). Then the UI would just show that the editor is acquired in exclusive mode, or for others that it can't be updated for now, ideally with polling (or server-side events) so it can detect when it's free again. Or when the lock has timed out. The same way and actual editor should ping the server to keep the session alive, to show it's still in edit mode. If not the user loses its lock (if there is someone else requesting it, no need if there is not other editor waiting). |
|
Thanks both for your input, I will check what I do in my case because of introducing a table, there's no need for the |
|
Closing this and updating the other PR |
The ViewCount module's read-modify-write pattern causes lost updates when multiple users view the same content simultaneously. An editor modifying content while others view it can also lose changes.
Changes
Modified
ViewCountService:ISessionto access database connectionIncrementViewCountAtomicallyAsync()using database-specific JSON functionsJSON_MODIFY), SQLite (json_set), MySQL (JSON_SET), and PostgreSQL (jsonb_set)Dependencies:
Dapper.StrongNamefor parameterized SQL executionImplementation
The increment now executes atomically within YesSql's transaction context, eliminating concurrent update conflicts.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.