The EntitySpaces Community

Share and learn about the EntitySpaces Architecture.
Welcome to The EntitySpaces Community Sign in | Join | Help
in
Home Forums Photos

Non Thread Safe Issue in FindByPrimaryKey

Last post 09-30-2007, 5:27 AM by Mike.Griffin. 3 replies.
Sort Posts: Previous Next
  •  09-29-2007, 6:02 AM 5378

    Non Thread Safe Issue in FindByPrimaryKey

    I am a developer from Enlighten Designs and have just recently released a fairly well trafficked site that uses EntitySpaces
    (to a degree, I might go into that in another post) for it's DAL. Since 90% of the database is only updated at most once a
    day at a scheduled time and the fact that site does get hit quite hard we cache alot of reference data from tables
    (such as a country list). While I have gotten the impression before from reading the forums that Mike seems to be against caching
    actual ES objects we tried it anyway. Basically we just call LoadAll on a few collections and dump them into the cache, grabbing it
    back out of the cache at a later dateand use FindByPrimaryKey. Unfortunately we came across a fun Thread Saftey issue.
     Code:
    1    protected esEntity FindByPrimaryKey(object primaryKeyValue)
    2 {
    3 if (base.Table != null)
    4 {
    5 this.AssignPrimaryKeys();
    6 DataRow row = base.Table.Rows.Find(primaryKeyValue);
    7 this.RemovePrimaryKeys();
    8 if (row != null)
    9 {
    10 return this.entities[row];
    11 }
    12 }
    13 return null;
    14 }
    If two threads are in the above method; it is possible for the following situatio to arise: thread #2 just finishes line 5 and is about to 
    execute line 6 while thread #1 is slightly ahead and completes line 7. Meaning thread #2 is attempting a Rows.Find on a table with
    no primary keys assigned. Normally this wouldn't be an issue but since we have multiple threads playing with the exact same collection
    object we start getting some interesting results. We have a temp fix which is as below, but as you can see it's certainly not optimal
    and are currently pondering dropping the esEntityCollection object in favor of a simple Dictionary<int, esEntity>
    (what I wouldn't give for a esEntityCollection.ToDictionary() method right now)
     Code:
    1    protected esEntity FindByPrimaryKey2(object primaryKeyValue)
    2 {
    3 lock(this) {
    4 return FindByPrimaryKey(primaryKeyValue);
    5 }
    6 }
     
  •  09-29-2007, 7:13 AM 5380 in reply to 5378

    Re: Non Thread Safe Issue in FindByPrimaryKey

    Great post by the way, I love it when on the very first post the problem at hand is clearly laid out so we don't spend the first few posts trying to get at the actual problem, so thanx for that.  Since AssignPrimaryKeys and RemovePrimaryKeys are non-trivial functions locking around the entire FindByPrimaryKey isn't optimal, I agree on that for sure.

    Here's what I'm thinking. Suppose we add a property called "Cache" or some such flag that you set to true. That basically calls AssignByPrimaryKeys() and from then on FindByPrimaryKey checks the flag and doesn't call those support routines. However, I would then also like to make Filter / Sort be able to work against the same collection, and make sure that enumeration is thread safe. I think this is a great idea, making our collections thread safe, is something we should do. However, it probably wont make this release.

    This might help you, we have a method in the collection that does this:

     

    Code:
            public static implicit operator List(EmployeeCollection coll)
    {
    List list = new List();

    foreach (Employee emp in coll)
    {
    list.Add(emp);
    }

    return list;
    }
     

    Here's a method that we can add in the next release for sure.

     

    Code:
            public static implicit operator Dictionary<int, Employee>(EmployeeCollection coll)
    {
    Dictionary<int, Employee> dictionary = new Dictionary<int, Employee>();

    foreach (Employee emp in coll)
    {
    dictionary[emp.EmployeeID.Value] = emp;
    }

    return dictionary;
    }
     

    But, in reality we couldn't make it as simple as above, the problem is composite primary keys, we need to be all things to all people ... What are you thoughts on all this?

    [Modified]

    To use the above code you would do this:

     

    Code:
    EmployeeCollection coll = new EmployeeCollection();
    coll.LoadAll();
    
    Dictionary<int, Employee> dictionary = coll;
     

     


    EntitySpaces | Twitter | BLOG | Please honor our Software License
  •  09-29-2007, 3:16 PM 5385 in reply to 5380

    Re: Non Thread Safe Issue in FindByPrimaryKey

    No problem, it was actually rather fun figuring the whole thing out.  Your suggested solution sounds good, not only should it fix the problem at hand but I'm guessing make ES slightly more efficient when used like this.  Obviously there is a reason you were removing the PK after the search so my new concern is what other functionality will be effected by this. Will this effectively be making the collection read only? Which obviously wouldn't be a problem in our case, I'm just trying to get a feeling for when I couldn't use the new functionality

     The new dictionary operator looks great, as for the composite key problem the only solution I can think of is adding a PrimaryKey struct when generating the entities

     Code:

    //In the MetaDataClass?
    public struct PrimaryKey { int employeeID, int cloneID };

    //The EsEntity would then need a new method to get the PK
    public static implicit operator PrimaryKey(Employee emp)
    {
    PrimaryKey pk = new PrimaryKey();
    pk.employeeID = emp.EmployeeID;
    pk.cloneID = emp.CloneID;
    return pk;
    }

    //DictionaryMethod
    public static implicit operator Dictionary<PrimaryKey, Employee>(EmployeeCollection coll)
    {
    Dictionary<PrimaryKey, Employee> dictionary = new Dictionary<PrimaryKey, Employee>
      foreach (Employee emp in coll)
    {
    dictionary[emp] = emp;
    }

    return dictionary
    }
    while I'm guessing this would work it might actually be fairly annoying to use in the case when you only have a single column primarykey, at which point you could instead generate your suggested dictionary method. Alternatively I guess you could add even more operators to convert between the new PrimaryKey type and whatever type the column is but surely this would be overkill for such a small function (unless you find another use for the primarykey class i guess).
  •  09-30-2007, 5:27 AM 5395 in reply to 5385

    Re: Non Thread Safe Issue in FindByPrimaryKey

    Nice, I like that. I was thinking of a string that was a concatenation of the primary keys ... something like this (pure speculation at this point):

     

    Code:
    EmployeeCollection coll = new EmployeeCollection();
    coll.LoadAll();

    // Creates the internal dictionary? coll.EnableCaching = true;

    // key is a concatenation of the primarykeys (calls static method) string key = Employee.CreatePrimaryKeyToken(employeeId, cloneID);

    // Works no matter how many keys are in the PK Employee emp = coll.GetCachedObject(key);
     

    Creating the dictionary would look like this:

     

    Code:
    public static implicit operator Dictionary<string, Employee>(EmployeeCollection coll)
    {
    Dictionary<string, Employee> dictionary = new Dictionary<string, Employee>

    foreach (Employee emp in coll)
    {
    string token = Employee.CreatePrimaryKeyToken(emp.EmployeeId, emp.CloneID);
    dictionary[token] = emp;
    }

    return dictionary
    }
     

    still thinking on it .... 


    EntitySpaces | Twitter | BLOG | Please honor our Software License
View as RSS news feed in XML