C# - Sql transactie vanuit threadpool

[SideShow]

Hallo

Ik heb een logger klasse, die logt naar SQL server. Elke Log() aanroep zet gewoon 1 record in een logtabel.

Dit is de code:

C#:

        public void Log(...args...)
        {
            _sqlCommand.Parameters[0].Value = Guid.NewGuid();
            ...

            for (int i = 0; i < 100; i++)
                ThreadPool.QueueUserWorkItem(ThreadPoolCallback);
        }

        private void ThreadPoolCallback(Object threadContext)
        {
            using (var sqlConnection = new SqlConnection(_connectionString))
            {
                try
                {
                    _sqlCommand.Connection = sqlConnection;
                    sqlConnection.Open();
                    _sqlCommand.ExecuteNonQuery();
                }
                finally
                {
                    sqlConnection.Close();
                }
            }
        }

Om het systeem performant te houden, en niet-UI-gerelateerde zaken asynchroon te doen, wil ik van de eerste keer ook testen hoe performant dit is, door bij wijze van test 100 taken toe te voegen aan de threadpool.

_sqlCommand is een private class field en er wordt steeds een nieuwe connectie gemaakt: gezien de connectie met sql server toch standaard via een connectionpool gebeurt, kan dit geen probleem opleveren, dacht ik.

Het probleem is dat dit meestal enkele keren lukt, er komen dus enkele (tientallen) records bij in mijn logtabel, maar dan vliegt hij eruit. Ofwel gaat hij plots zeggen dat de connectie niet kan geopend worden omdat ze nooit gesloten werd, ofwel gaat hij zeggen dat de huidige state = connecting is, of een nullreference zou hij ook durven doen tijdens de ExecuteNonQuery.

Iemand een idee?

Btw, ik heb geen catch omdat unhandeld exeptions normaal gezien gewoon in de eventlog komen van de server (ze moeten zeker niet opgegooid worden naar UI gerelateerde code), maar ik heb wel een finally blok nodig, dacht ik.

Links naar code op externe sites hebben we hier liever niet

[ Voor 2% gewijzigd door Woy op 05-10-2011 14:57 ]

[Woy]

Aangezien je SqlCommand geshared word over meerdere threads is deze code sowieso niet goed. Waarom maak je niet gewoon telkens een nieuw SqlCommand aan?

[SideShow]

Misschien is dat inderdaad de oplossing.
Ik had het zo gedaan omdat het natuurlijk aanvoelde om in de Log() method de parameters op te vullen van het commando.
Indien ik het doe zoals jij zegt, moet ik dan wellicht al die parameters in veldjes steken van de logger klasse, en deze dan gebruiken om een SqlCommand object te maken in de callback method (?)

edit: of wellicht beter van een SqlParameter array te maken als veld in de Logger classe

[ Voor 11% gewijzigd door SideShow op 05-10-2011 15:09 ]

[Woy]

Als je multithreaded werkt wil je het liefst zo min mogelijk resources delen met meerdere threads, want dan zal je synchronisatie toe moeten passen. Het is dus verstandig om zo min mogelijk context voor je Log call als velden in je Log class op te nemen, daardoor kom je met multi-threading altijd in de problemen.

Ik zou het dan ongeveer als volgt doen

C#:

public void Log(string foo, object bar)
{
    vart item = new MyLogItem(foo, bar);
    for (int i = 0; i < 100; i++) 
        ThreadPool.QueueUserWorkItem(ThreadPoolCallback, item); 
} 

private void ThreadPoolCallback(Object context)
{
    MyLogItem item = context as MyLogItem;
    if (item != null)
    {
        using (var sqlConnection = new SqlConnection(_connectionString))
        {
            sqlConnection.Open();
            using (var command = sqlConnection.CreateCommand())
            {
                command.CommandText = "INSERT INTO Table( foo, bar ) VALUES( @foo, @bar )";
                command.Parameters.AddWithValue("@foo", item.Foo);
                command.Parameters.AddWithValue("@bar", item.Bar);
                command.ExecuteNonQuery();
            }
        }
    }
}

De instance van MyLogItem word alleen read-only gebruik binnen de andere threads dus dat levert geen problemen op.

[ Voor 11% gewijzigd door Woy op 05-10-2011 15:27 ]

[gorgi_19]

Waarom gooi je de verwerking van de gehele pool niet in een background thread ipv het per actie individueel te doen?

[Woy]

Dat is natuurlijk een nog betere oplossing. Je kunt gewoon een (ProducerConsumer)Queue bijhouden met alle logmessages, en een background thread die telkens laten verwerken.

[SideShow]

Zeker een interessante kijk!

[Woy]

Je zult er dan natuurlijk nog steeds voor moeten zorgen dat het inserten in de Queue thread-safe gebeurt.