Queue and performing inserts into a database on a background thread

Question

I have a asp.net application in a web farm (3 servers). Alongside that application I have a module that logs every request made of the website to a database. Currently the inserts are sychronous. I would like to change it so the inserts are sent to a queue which will insert them as it is able.

Would it be better to

1. Attempt to insert each request on a background thread (would too many background threads get used up if the database hiccups?)

2. Start an in-process queue on a single background thread that just reads from queue and performs insert.

3. Create an out-of-process queue on the database server that each server sends page request logs to. Would the built in MSMQ be the thing to use for something like this? - or would that be overkill?

Answer 1

Option 2 sounds the best. Option 1 would definitely create way too many background threads, and option 3 sounds more complex than it needs to be.

You might try something like this.

Given this class:

class LogEntry
{
    public string IpAddress { get; set; }
    public string UserAgent { get; set; }
    public DateTime TimeStamp { get; set; }
    public string Url { get; set; }
    //whatever else you need
}

Use this class to perform the logging:

class SiteLogger
{
    private static object loggerLock = new object();
    private static List<LogEntry> _pendingEntries = new List<LogEntry>();
    private static Thread savingThread;

    public static void AddEntry(LogEntry entry)
    {
        // lock when accessing the list to avoid threading issues
        lock (loggerLock)
        {
            _pendingEntries.Add(entry);
        }

        if (savingThread == null)
        {
            // this should only happen with the first entry
            savingThread = new Thread(SaveEntries);
            savingThread.Start();
        }
    }

    private static void SaveEntries()
    {
        while (true)
        {
            while (_pendingEntries.Count > 0)
            {
                // lock around each individual save, not the whole loop
                // so we don't force one web request to wait for
                // all pending entries to be saved.
                lock (loggerLock)
                {
                    // save an entry to the database, however the app does that
                    MyDatabase.SaveLogEntry(_pendingEntries[0]);
                    _pendingEntries.RemoveAt(0);
                }
            }

            Thread.Sleep(TimeSpan.FromSeconds(2));
            // 2 seconds is a bit of an arbitrary value.  Depending on traffic levels,
            // it might need to go up or down.
        }
    }
}

I ran this with a simple command line test app without any database involvement (simulated a database call by sleeping for 10 ms) and it seemed to work great, but it should obviously be tested more before going into a production environment. Also, problems will of course arise if the requests come faster than you can save them to the database (which is unlikely but should be considered).

Update, February 2018: Looking at this now, I realize that you could end up with two savingThread instances if you get unlucky with thread timing (which you should assume you will). And new Thread() is kind of an old way to do this kind of thing in C# now. I'll leave modern, more thread-safe implementations of this as an exercise to the reader.

Answer 2

A more modern, TPL approach (as suggested in Feb '18 in the accepted answer) would probably look something like:

class SiteLogger
{
    private readonly Lazy<BlockingCollection<LogEntry>> _messageQueue = new Lazy<BlockingCollection<LogEntry>>(() =>
    {
        var collection = new BlockingCollection<LogEntry>();
        Task.Factory.StartNew(processMessages, TaskCreationOptions.LongRunning);
        return collection;

        void processMessages()
        {
            foreach (var entry in collection.GetConsumingEnumerable())
            {
                //Do whatever you need to do with the entry
            }
        }
    }, LazyThreadSafetyMode.ExecutionAndPublication);


    public void AddEntry(LogEntry logEntry) => _messageQueue.Value.TryAdd(logEntry);
}

Other best practices, like dependency injection, IoC, etc to ensure this is a singleton and properly tested are recommended but best left to a tutorial on any one of those topics.