Fix PRIMARY KEY violation in WorkloadDetails when interval fires multiple times#166
Conversation
…e and MERGE Instead of bulk-inserting directly into WorkloadDetails (which fails with a PRIMARY KEY violation when CloseInterval fires multiple times for the same interval_id, e.g. when events share identical timestamps), the new approach: 1. Bulk-copies aggregated rows into a temporary staging table (#WorkloadDetails_Staging) 2. Uses a MERGE statement to either INSERT new rows or UPDATE existing rows by properly combining aggregate statistics (sums add, min/max recalculate, counts add) Also moves rawData.Clear() inside the lock(rawData) block to fix a race condition where new events could be added to rawData between the write and the clear." Agent-Logs-Url: https://github.com/spaghettidba/WorkloadTools/sessions/df9cb5f7-e87d-43bd-aa92-5c81d6039a5c Co-authored-by: spaghettidba <7592822+spaghettidba@users.noreply.github.com>
Agent-Logs-Url: https://github.com/spaghettidba/WorkloadTools/sessions/df9cb5f7-e87d-43bd-aa92-5c81d6039a5c Co-authored-by: spaghettidba <7592822+spaghettidba@users.noreply.github.com>
|
@copilot I very much dislike this implementation. Why use a MERGE to patch a race condition rather than fixing it? |
…writes Root cause: when Interval=0 (default), CloseInterval's condition `duration.TotalMinutes >= Interval` is `0 >= 0` (always true). After the first write sets lastDump=lastEventTime, the next loop iteration fires immediately with the same lastEventTime -> same interval_id -> PRIMARY KEY violation on WorkloadDetails. Fix: track lastWrittenIntervalId (volatile for cross-thread visibility). CloseInterval computes the prospective interval_id before writing; if it matches lastWrittenIntervalId it skips the write and advances lastDump. WriteToServer records lastWrittenIntervalId after a successful commit. Additional changes: - Extract ComputeIntervalId() helper shared by CloseInterval and CreateInterval - Move rawData.Clear() inside lock(rawData) to close a race window Agent-Logs-Url: https://github.com/spaghettidba/WorkloadTools/sessions/5633e4ed-c992-449d-9b66-fe4df46a6cf1 Co-authored-by: spaghettidba <7592822+spaghettidba@users.noreply.github.com>
Replaced the MERGE approach with a proper fix in ac65562. The root cause: when The fix tracks |
| GC.WaitForPendingFinalizers(); | ||
| } | ||
|
|
||
| private static int ComputeIntervalId(DateTime intervalTime) |
There was a problem hiding this comment.
No good reason — the method doesn't use any instance state so static was technically accurate, but it's inconsistent with the rest of the private helpers in this class. Removed it in 8413cc8.
Agent-Logs-Url: https://github.com/spaghettidba/WorkloadTools/sessions/dfb1a930-55ad-4822-94c9-598e12bd4ff6 Co-authored-by: spaghettidba <7592822+spaghettidba@users.noreply.github.com>
CloseIntervalcondition0 >= 0is always true whenInterval=0(default); after the first write setslastDump=lastEventTime, the next loop iteration fires for the sameinterval_id→ PRIMARY KEY violationlastWrittenIntervalId(volatile).CloseIntervalskips the write if the prospectiveinterval_idmatches the last written one.WriteToServerrecordslastWrittenIntervalIdafter a successful commit.ComputeIntervalId()helper used by bothCloseIntervalandCreateIntervalstaticfromComputeIntervalIdfor consistency with class stylerawData.Clear()insidelock(rawData)to prevent a race between write and clear