Is this locking Okay?

Question

I've a table which contains sequence number.

Table structure

SequenceGenerator
    Year int
    Month int
    NextNumber int

Year + Month make primary key. Sequence is reset every month.

I'm using Subsonic to generate DAL. To get next sequence number I've written a class which returns next number to requestors:

private static readonly object _lock = new Object();
private static readonly string FormatString = "{0}{1}{2}{3}";
private static readonly string NumberFormat = "000000";

public static object GetNextNumber(string prefix)
{
    lock (_lock)
    {
        int yr = DateTime.Now.Year;
        int month = DateTime.Now.Month;

        SequenceGeneratorCollection col = new SequenceGeneratorCollection()
           .Where(SequenceGenerator.Columns.Year, Comparison.Equals, yr)
          .Where(SequenceGenerator.Columns.Month, Comparison.Equals, month)
          .Load();

        if (col==null || col.Count == 0)
        {
            SequenceGenerator tr = new SequenceGenerator();
            tr.Year = yr;
            tr.Month = month;
            tr. NextNumber = 1;
            tr.Save();
            return string.Format(FormatString, prefix, yr, 
                      month,tr.NextNumber.ToString(NumberFormat));
        }

        SequenceGenerator  t = col[0];
        t.NextNumber += 1;
        t.Save();

        return string.Format(FormatString, prefix, yr, month, 
                 t.NextNumber.ToString(NumberFormat));
    }
}

Answer 1

This locking is really risky, you should use database level transaction if you want to ensure the data remains coherent.

The lock(_lock) does not protect you against having two app domains talking to the DB at the same time.

Answer 2

This lock won't when you have more than one client locking different _lock objects. You should use the database locking mechanisms for this.

Answer 3

Not recommended. This should be done in DB with auto number fields. Also even if you do not do this in DB and still choose to pursue this way, make sure you lock for least amount of code, dont wrap the whole method in lock.