[seek-dev] Questions about implementation of org.kepler.objectmanager.cache.DataCacheObject

Chad Berkley berkley at nceas.ucsb.edu
Wed Sep 7 09:55:12 PDT 2005


Hey Kevin,

There are definitely some weird things going on in that class.  I've 
been slowly cleaning it up as i modify it for the object manager.  The 
eml200datasource (and any other data sources) are going to need to be 
rewritten to use the object manager anyway, so i think these annoying 
(broken) data source specific methods can just go away or be 
re-architected into the new code.  I'm not sure who is slated to re-work 
this code.  i think it should probably be either you or jing but i'll 
let matt chime in on that.  the object manager is ready for use, so the 
re-write can start now.

chad

Kevin Ruland wrote:
> Hi all,
> 
> I have some questions about the o.k.objectmanager.cache.DataCacheObject
> implementation relating to the use of a spin lock in
> getDataItemFromEcoGrid().
> 
> The critical facts here are:
> 
> protected boolean needSynchronize = true;
> private static boolean lockEcoGrid = false;
> 
>   protected void getDataItemFromEcoGrid(String endPoint, String identifier)
>   {
> 
>     // create a ecogrid client object and get the full record from the
>     // client
>     dbg.print("Get " + identifier + " from " + endPoint, 2);
>     if(needSynchronize)
>     {
>       while(lockEcoGrid)
>       {
>         dbg.print("sleep " + identifier, 2);
>         try
>         {
>           Thread.sleep(500);
>         }
>         catch(Exception e)
>         {
> 
>         }
>       }
>       synchronized(this)
>       {
>         lockEcoGrid = true;
>       }
> 
>     }
> 
> //*********** Here work is done
> 
>     lockEcoGrid = false;
>     dbg.print("realse lock from " + identifier, 2);
> 
>   }
> 
> I suspect the intention of the lockEcoGrid flag it to prevent two
> threads from running the block of code which does actual work.  However,
> the code does not do this.  In fact, the spinlock and synchronization
> code does nothing at all.  Certainly the while loop will spin and wait,
> but the synchronized block does not prevent multiple threads from
> concurrently operating in the following block.  Suppose  we have thread
> 1 and thread 2 executing and lockEcoGrid is false.  Both threads can
> fall through to the synchronized statement.  Now thread 1 gets to
> execute the synchronized block, when it's finished, suppose it is
> suspended.  Then thread 2 gets to execute the synchronized block.  Now
> both threads can execute the code in "Here work is done".
> 
> The only class variable here is lockEcoGrid.  So I have to wonder,
> exactly what is being locked? If some remote resource (ecogrid server)
> is being locked then this locking should be done in the
> EcogridFactoryQueryClient class.  If this is code held over from a
> failed attempt to manage the remote resource, then it should be
> removed.  One final consideration is the remote resources can
> potentially be used by multiple Kepler instances and even two JVMs on
> the same host do not share synchronization semiphores.
> 
> Another question I have about this class is why it has a member called
> "getDataFromEcoGrid()"?  Does this really fit with the class' other
> methods which is about managing a file based cache system?  The method
> is used by both EcogridMetaDataCacheItem, and EcogridDataCacheItem. 
> Perhaps it is better for those two objects to share another ancestor. 
> And finally, EcogridQueryDataCacheItem has almost idential code in its
> doWork() method so it might benefit from this method as well.
> 
> Kevin
> _______________________________________________
> Seek-dev mailing list
> Seek-dev at ecoinformatics.org
> http://mercury.nceas.ucsb.edu/ecoinformatics/mailman/listinfo/seek-dev


More information about the Seek-dev mailing list