asked
Atomicity/thread-safety within a single UPDATE statement
Given the following table structure, for an RRD-style idea I've got floating in my head... CREATE TABLE rrd_control ( rrd_object INT, rrd_aggregation TINYINT, rrd_current BIGINT, rrd_max BIGINT, CONSTRAINT pk_rrd_control PRIMARY KEY CLUSTERED (rrd_object, rrd_aggregation) ) ...would the following be atomic and thread-safe (i.e. simultaneous calls would never get the same number), given that the assignment and the increment are in the same UPDATE statement... DECLARE @rowid BIGINT UPDATE rrd_control SET @rowid = rrd_current, rrd_current = CASE (rrd_current + 1) WHEN rrd_max THEN 0 ELSE rrd_current + 1 END WHERE rrd_object = @object_id AND rrd_aggregation = 0 ...or would I *need* some sort of explicit row lock and (serializable?) transaction? Is it possible to reasonably test/prove this without resorting to infinite monkeys with infinite typewriters? ---------- ## Edit: Testing process/scripts ## Based on the comments below, I did some testing using some mechanisms available to me in SQL Server Management Studio (as I did not have a C# environment to hand at the time). The tests involved opening 5 separate queries (each with their own SPID) and running the same SQL code in them simultaneously. I stored the numbers I retrieved in a separate global temporary table for each SPID (named `##rrd1`-`##rrd5`), each of which also contained an `IDENTITY` column and `DATETIME2` field (which I would populate with the `SYSDATETIME()` function), to capture the "fetch time" as accurately as I could. Then I set the `rrd_max` value for the `rrd_object`/`rrd_aggregation` combination I was interested in checking to 1000, so it was significantly higher than the perceived granularity of `SYSDATETIME()`, which was returning the same time value for anywhere between 30 and 70 iterations. I also used the `WAITFOR TIME` command, set to the same time value in each query, to have the iterations start at the same time (or at least close-enough). When the five separate queries finished, I used another query to aggregate and compare the results of both, using a `UNION` of `LEFT JOIN`s to find rows where the "fetch date" and "fetched number" were identical across each of the `##rrd1`-`##rrd5` tables. While running the five simultaneous queries, I occasionally spammed `sp_who2` and could see that, regardless of the test script, a running query would block a second query and all subsequent queries were blocked by the second one. In both cases, the aggregation query returned 0 rows. The two test scripts, run in each separate query (replacing `##rrd1` as appropriate), and a third query, used to aggregate and compare, were as follows: ### No lock/transaction ### IF OBJECT_ID('tempdb..##rrd1', 'U') IS NOT NULL DROP TABLE ##rrd1 CREATE TABLE ##rrd1 (i INT IDENTITY(1,1) PRIMARY KEY CLUSTERED, d DATETIME2, l INT, n BIGINT) GO DECLARE @loop INT = 10000, @object_id INT = OBJECT_ID('stats_rrd', 'U'), @rowid INT SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED WAITFOR TIME '23:30:00.000' WHILE @loop > 0 BEGIN UPDATE rrd_control SET @rowid = rrd_current, rrd_current = CASE (rrd_current + 1) WHEN rrd_max THEN 0 ELSE rrd_current + 1 END WHERE rrd_object = @object_id AND rrd_aggregation = 0 INSERT INTO ##rrd1 (d, l, n) VALUES (SYSDATETIME(), @loop, @rowid) SET @loop = @loop - 1 END GO ### With a lock/transaction ### IF OBJECT_ID('tempdb..##rrd1', 'U') IS NOT NULL DROP TABLE ##rrd1 CREATE TABLE ##rrd1 (i INT IDENTITY(1,1) PRIMARY KEY CLUSTERED, d DATETIME2, l INT, n BIGINT) GO DECLARE @loop INT = 10000, @object_id INT = OBJECT_ID('stats_rrd', 'U'), @rowid INT SET TRANSACTION ISOLATION LEVEL SERIALIZABLE WAITFOR TIME '23:45:00.000' WHILE @loop > 0 BEGIN BEGIN TRAN UPDATE rrd_control WITH (ROWLOCK, XLOCK) SET @rowid = rrd_current, rrd_current = CASE (rrd_current + 1) WHEN rrd_max THEN 0 ELSE rrd_current + 1 END WHERE rrd_object = @object_id AND rrd_aggregation = 0 COMMIT TRAN INSERT INTO ##rrd1 (d, l, n) VALUES (SYSDATETIME(), @loop, @rowid) SET @loop = @loop - 1 END GO ### Aggregation/comparison ### ;WITH cte AS ( SELECT r.i, r.d, r.l, r.n, xi = COALESCE(r2.i, r3.i, r4.i, r5.i), xd = COALESCE(r2.d, r3.d, r4.d, r5.d), xl = COALESCE(r2.l, r3.l, r4.l, r5.l), xn = COALESCE(r2.n, r3.n, r4.n, r5.n) FROM ##rrd1 r LEFT JOIN ##rrd2 r2 ON r.d = r2.d AND r.n = r2.n LEFT JOIN ##rrd3 r3 ON r.d = r3.d AND r.n = r3.n LEFT JOIN ##rrd4 r4 ON r.d = r4.d AND r.n = r4.n LEFT JOIN ##rrd5 r5 ON r.d = r5.d AND r.n = r5.n UNION ALL SELECT r.i, r.d, r.l, r.n, xi = COALESCE(r1.i, r3.i, r4.i, r5.i), xd = COALESCE(r1.d, r3.d, r4.d, r5.d), xl = COALESCE(r1.l, r3.l, r4.l, r5.l), xn = COALESCE(r1.n, r3.n, r4.n, r5.n) FROM ##rrd2 r LEFT JOIN ##rrd1 r1 ON r.d = r1.d AND r.n = r1.n LEFT JOIN ##rrd3 r3 ON r.d = r3.d AND r.n = r3.n LEFT JOIN ##rrd4 r4 ON r.d = r4.d AND r.n = r4.n LEFT JOIN ##rrd5 r5 ON r.d = r5.d AND r.n = r5.n UNION ALL SELECT r.i, r.d, r.l, r.n, xi = COALESCE(r1.i, r2.i, r4.i, r5.i), xd = COALESCE(r1.d, r2.d, r4.d, r5.d), xl = COALESCE(r1.l, r2.l, r4.l, r5.l), xn = COALESCE(r1.n, r2.n, r4.n, r5.n) FROM ##rrd3 r LEFT JOIN ##rrd1 r1 ON r.d = r1.d AND r.n = r1.n LEFT JOIN ##rrd2 r2 ON r.d = r2.d AND r.n = r2.n LEFT JOIN ##rrd4 r4 ON r.d = r4.d AND r.n = r4.n LEFT JOIN ##rrd5 r5 ON r.d = r5.d AND r.n = r5.n UNION ALL SELECT r.i, r.d, r.l, r.n, xi = COALESCE(r1.i, r2.i, r3.i, r5.i), xd = COALESCE(r1.d, r2.d, r3.d, r5.d), xl = COALESCE(r1.l, r2.l, r3.l, r5.l), xn = COALESCE(r1.n, r2.n, r3.n, r5.n) FROM ##rrd4 r LEFT JOIN ##rrd1 r1 ON r.d = r1.d AND r.n = r1.n LEFT JOIN ##rrd2 r2 ON r.d = r2.d AND r.n = r2.n LEFT JOIN ##rrd3 r3 ON r.d = r3.d AND r.n = r3.n LEFT JOIN ##rrd5 r5 ON r.d = r5.d AND r.n = r5.n UNION ALL SELECT r.i, r.d, r.l, r.n, xi = COALESCE(r1.i, r2.i, r3.i, r4.i), xd = COALESCE(r1.d, r2.d, r3.d, r4.d), xl = COALESCE(r1.l, r2.l, r3.l, r4.l), xn = COALESCE(r1.n, r2.n, r3.n, r4.n) FROM ##rrd5 r LEFT JOIN ##rrd1 r1 ON r.d = r1.d AND r.n = r1.n LEFT JOIN ##rrd2 r2 ON r.d = r2.d AND r.n = r2.n LEFT JOIN ##rrd3 r3 ON r.d = r3.d AND r.n = r3.n LEFT JOIN ##rrd4 r4 ON r.d = r4.d AND r.n = r4.n ) SELECT * FROM cte WHERE xi IS NOT NULL AND xd IS NOT NULL AND xl IS NOT NULL AND xn IS NOT NULL
I'm going to go the other way and say yes, but I typically do the update syntax slightly differently. DECLARE @rowid BIGINT UPDATE rrd_control SET @rowid = rrd_current = CASE (rrd_current + 1) WHEN rrd_max THEN 0 ELSE rrd_current + 1 END WHERE rrd_object = @object_id AND rrd_aggregation = 0 This would behave slightly differently, in that it would give you back the value after the increment, and not before it. I have used exactly that sort of syntax to achieve exactly what you want (the use case I had was issuing unique serial numbers for tickets). However - a word of caution - I do not know, and have not tested, what it would do under READ COMMITTED SNAPSHOT. It may be worth electing to go for SERIALIZABLE for the single transaction involved. Edit -> Here's some C# test code. From some (extremely) quick testing, it looks like READ COMMITTED SNAPSHOT is probably OK. But really, don't take my word for it - you need to do your own testing until you're happy. Dictionary all_longs = new Dictionary(); private object locker = new object(); private void button1_Click(object sender, EventArgs e) { for (int i = 0; i < 10; i++) { new Thread(new ThreadStart(tester)).Start(); } } void tester() { using (SqlConnection sqlConn = new SqlConnection("SERVER=.\\SQL2K8;Initial Catalog=tempTest;Integrated Security=true;")) { sqlConn.Open(); for (int i = 0; i < 10000; i++) { using (SqlCommand sqlCmd = new SqlCommand("test", sqlConn)) { using (SqlDataReader sdr = sqlCmd.ExecuteReader()) { sdr.Read(); lock (locker) { all_longs.Add(sdr.GetInt64(0), 0); } } } } } } Edit -> code mk2 Dictionary all_longs = new Dictionary(); private object locker = new object(); private object locker2 = new object(); int ixx = 10; private void button1_Click(object sender, EventArgs e) { for (int i = 0; i < 10; i++) { new Thread(new ThreadStart(threadProc)).Start(); } } void threadProc() { List longsReturned = new List(); using (SqlConnection sqlConn = new SqlConnection("SERVER=.\\SQL2K8;Initial Catalog=tempTest;Integrated Security=true;")) { sqlConn.Open(); for (int i = 0; i < 10000; i++) { using (SqlCommand sqlCmd = new SqlCommand("test", sqlConn)) { using (SqlDataReader sdr = sqlCmd.ExecuteReader()) { sdr.Read(); longsReturned.Add(sdr.GetInt64(0)); } } } } foreach (long l in longsReturned) { lock (locker) { all_longs.Add(l, 0); } } lock (locker2) { ixx--; if (ixx == 0) { this.Invoke(new MethodInvoker(delegate() { MessageBox.Show(ixx.ToString() + " " + all_longs.Count.ToString()); })); } } }
@jimbobmcgee - The only thing I would have done is simplified the results query. Something like: SELECT n FROM ( SELECT n FROM ##rrd1 UNION ALL SELECT n FROM ##rrd2 UNION ALL SELECT n FROM ##rrd3 UNION ALL SELECT n FROM ##rrd4 UNION ALL SELECT n FROM ##rrd5 ) all_values GROUP BY n HAVING COUNT(*) > 1
Hey Matt! I agree, in most circumstances you're going to be fine, but if you take the update lock out prior to the read you should be guaranteed of a consistent lock. I also agree, read committed snapshot changes the rules.
This is why I had to ask; two conflicting opinions and I can't make up my mind between them!! Given your C# code (which is similar to what I originally had) I feel there may be something lacking: doesn't the `lock (locker) { ... }` call implicitly change things? In all instances (bar, possibly, the very first call in each `tester()` loop) all `sqlCmd.ExecuteReader()` calls are blocked/queued by the list append, because they are all followed by a lock on the same locking object. I tried to look at the operation in the SQL Profiler, but I'm missing a step there, too; namely I can't seem to find an event for when a table is actually read/written -- I've got the statement start/end and I've got lock acquire/release but no table read/write, which is a shame.
@jimbobmcgee - I can see what you're saying, but the lock during the dictionary add isn't likely to be the majority of time spent - I think that would be very very lightweight. So, serial access to the dictionary wouldn't necessarily (and probably wouldn't) imply serial access to the database. If you are worried about it, then declare a `List` at the start of the thread proc, add to that list during the life of the thread, and then using a loop add stuff into the dictionary at the end (to ensure you have received no duplicates). In fact, I'll probably edit my answer with that code. I've just had 10 threads do 10000 each in about 10 seconds. So, probably some parallel access going on there...
Thanks for your time, both of you. While the definitive answer is probably Grant's, where an explicit lock would guarantee it, I expect that, for my needs, I would be able to get away without one, as per Matt's answer. If anyone ever does come up with a definitive proof, I'd love to see it; I can still only be half sure, given the testing approach -- there may be a magic number of simultaneous connections for which this logic falls over. That said, most RRD-based systems use a single, periodic data-collection process, so I expect that such a magic number would never be realistically reached.
Nope. You can't guarantee that you would never see a duplicate unless you lock the table prior to the read. You could put an update lock on it and then run your query, but you're going to see some pretty heavy blocking there. To test it shouldn't be too hard. Spawn a series of threads in C# or Powershell or something and run your query in a loop from three or four threads. You should be able to ascertain shortly whether or not you've an issue.
I have added the test process I used to the question ([meta] although I am happy to move it to an answer, if requested). My findings seem to support your answer, Matt, over yours, Grant. Both the lock and no-lock approaches seemed to cause blocks against the `rrd_control` table and neither appeared to result in duplication (although I did have a brief scare with both before I clocked that my cycle was shorter than the accuracy of the `SYSDATETIME()` function!!) However, is there anything that either of you feel I may have missed in my test process that might invalidate the findings?
'Twas gone midnight when I put that results script together; looking back on it I can see that yours would simplify greatly, with the addition of the insert date (since I'm looking for simultaneous retrieval of the same number), so: SELECT d, n FROM ( SELECT d, n FROM ##rrd1 UNION ALL SELECT d, n FROM ##rrd2 UNION ALL SELECT d, n FROM ##rrd3 UNION ALL SELECT d, n FROM ##rrd4 UNION ALL SELECT d, n FROM ##rrd5 ) all_values GROUP BY d, n HAVING COUNT(*) > 1 It's funny how cut-and-paste, with the odd block select/delete can replace good SQL practice at 1am...
