Asynchronously updating multiple row updates 1/4 of rows instantly and then waits

Viewed 662

I have a code to asynchronously update multiple rows in SQL Server's table. I tested it on updating 540 rows and 144 rows are updated in the table instanly, then it waits for about 5 minutes and then the rest is updated. At least this is how it looks when I check for updated rows with SELECT.. I'm wondering why is that.

The whole thing is triggered by button's click:

DialogResult res = MessageBox.Show($"Znaleziono {num} pasujących maszyn. Czy chcesz zaktualizować priorytet maszyny danymi z pliku?", "Potwierdź", MessageBoxButtons.YesNo, MessageBoxIcon.Question);
            if(res == DialogResult.Yes)
            {
                await UpdatePriority();
                MessageBox.Show("Updated!");

Here's UpdatePriority method that asynchronously call place.Edit() method for all places in the list of items:

public async Task<string> UpdatePriority()
        {
            List<Task<string>> UpdateTasks = new List<Task<string>>();

            try
            {
                foreach (Place p in Items.Where(i => i.IsUpdated==true))
                {
                    UpdateTasks.Add(Task.Run(()=> p.Edit()));
                }

                string response = "OK";

                IEnumerable<string> res = await Task.WhenAll<string>(UpdateTasks);

            }
            catch (Exception ex)
            {
                throw;
            }
            return "Nie udało się zaktualizować danych żadnego zasobu..";
        }

And here is Edit() method of place object. It basically updates place data in SQL server table:

public async Task<string> Edit()
        {
            string iSql = @"UPDATE JDE_Places
                            SET Priority=@Priority
                            WHERE PlaceId=@PlaceId";
            string msg = "OK";


            using (SqlCommand command = new SqlCommand(iSql, Settings.conn))
            {
                command.Parameters.AddWithValue("@PlaceId", PlaceId);
                command.Parameters.AddWithValue("@Priority", Priority);

                int result = -1;

                try
                {
                    result = await command.ExecuteNonQueryAsync();
                    IsUpdated = false;
                }
                catch (Exception ex)
                {
                    msg = $"Wystąpił błąd przy edycji zasobu {Name}. Opis błędu: {ex.Message}";
                }

            }
            return msg;
        }

And here's Settings conn property that serves as reusable connection object:

public static class Settings
    {
        private static SqlConnection _conn { get; set; }
        public static SqlConnection conn
        {
            get
            {
                if (_conn == null)
                {
                    _conn = new SqlConnection(Static.Secrets.ConnectionString);
                }
                if (_conn.State == System.Data.ConnectionState.Closed || _conn.State == System.Data.ConnectionState.Closed)
                {
                    try
                    {
                        _conn.Open();
                    }
                    catch (Exception ex)
                    {
                        MessageBox.Show("Nie udało się nawiązać połączenia z bazą danych.. " + ex.Message);
                    }

                }
                return _conn;
            }
        }
    }

I realize it's probably better to keep the connection within using statement (instead of reusing it), but when I added it to place.Edit() method it worked even slower (and unreliably).

UPDATE: I ran few tests more and the time they took to add 540 rows varied from 15 seconds to 400 seconds.. Then I just changed result = await command.ExecuteNonQueryAsync() to result = command.ExecuteNonQuery() in Edit() of place object, ran few tests more, and all finished under 10 seconds! I don't know why async version of ExecuteNonQuery() was so much worse than non-async one, though. Single Edit() method was taking around 0,1 sec with ExecuteNonQuery() and 1 - 400 seconds with ExecuteNonQueryAsync(). Here are logs: ExecuteNonQuery() ExecuteNonQueryAsync()

1 Answers

Your issue here is your Settings class. You're essentially trying to use the same SqlConnection object in multiple Sqlcommands. SqlConnection is not threadsafe when used like this. You end up with multiple commands because your code is non-blocking and async. That is what is causing your code the "wait" (or deadlock). This is why when you run it sync (without the ExecuteNonQueryAsync, etc.) it works correctly.

You don't need this object at all anyway. ADO.Net handles connection pooling for you, so there is no advantage in re-using the same SqlConnection. Just create a new one for each SqlCommand:

public async Task<string> Edit()
{


       using (SqlConnection conn = new SqlConnection(...))
       using (SqlCommand command = new SqlCommand(iSql, conn))
       {
       ...
       }
   }

and you should find that your "wait" goes away.

Related