Hi Armin, Yes, we probably could get away with no finally block. My only concern is the case where you have someone who gets this exception and decides to swallow it. In this case the counts will remain out of whack and this piece of work will continue to process (some exception processing or collecting log information), but pull any possible hits out of the local cache first because the counts were not decremented to zero appropriately.
I guess it just doesn't feel right leaving this cache messed up when we don't know for a fact that the broker is going to 'for sure' be closed, committed, or aborted immediately. So I guess I would lean towards catching Throwable, calling localClear on the cache. One other thing I noticed while poking around. I think the InternalCache#clear() method should call localClear() instead of localCache.clear(). Thanks much. Paul -----Original Message----- From: Armin Waibel [mailto:[EMAIL PROTECTED] Sent: Thursday, September 30, 2004 2:46 PM To: OJB Users List Subject: Re: OJB Cache and materialization. Hi Paul, you are right, the finally-block isn't a good idea. It should be safe to remove these blocks, because InternalCache#localClear() was called (and reset the counter) on PB.abort/commitTransaction and PB.close(). This should guarantee that InternalCache is always in sync. What do you think? regards, Armin Nase, Paul R. wrote: > Thanks Armin, > > It looks good (works too), but I have one issue with the fixes you provided. > You put the disableMaterializationCache() in a 'finally' block which is kewl. > It ensures your counts don't get all messed up. BUT what happens if > you get an exception in the middle of your materialization? Seems to > me, you will get these half-baked invalid objects plopped into the > 'realCache' when your last > disableMaterializationCache() is hit and the count is decremented to zero. > > Maybe you could catch all exceptions and set that enabledReadCache > flag in the InternalCache to false in the catch block? Of course, > you'd have to provide a setter method other than > disableMaterializationCache() to turn off the enabledReadCache flag. > > Paul > > -----Original Message----- > From: Armin Waibel [mailto:[EMAIL PROTECTED] > Sent: Thursday, September 30, 2004 9:15 AM > To: OJB Users List > Subject: Re: OJB Cache and materialization. > > Hi Paul, > > it's fixed in CVS branch "OJB_1_0_RELEASE". > > >> We are currently using OJB RC5 and are not in a position to move > to >> 1.0.1 yet, > > Think you are using rc6, because the "materialization buffer" > (InternalCache class) was introduced after rc5 - AFAIK > > It shouldn't be a problem to adapt the patch for earlier releases > (after > rc5) of OJB. I only introduce an 'counter' which counts the number of invocations of > method InternalCache#enableMaterializationCache. > > Modified classes are: > InternalCache > PersistenceBrokerImpl > RsIterator > > Diff see below. > > regards, > Armin > > > Revision Changes Path > No revision > No revision > 1.63.2.3 +27 -21 > db-ojb/src/java/org/apache/ojb/broker/accesslayer/RsIterator.java > > Index: RsIterator.java > =================================================================== > RCS file: > /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/RsIterator.java,v > retrieving revision 1.63.2.2 > retrieving revision 1.63.2.3 > diff -u -r1.63.2.2 -r1.63.2.3 > --- RsIterator.java 9 Aug 2004 07:51:26 -0000 1.63.2.2 > +++ RsIterator.java 30 Sep 2004 14:02:03 -0000 1.63.2.3 > @@ -452,26 +452,32 @@ > synchronized (result) > { > getCache().enableMaterializationCache(); > - getCache().cache(oid, result); > - /* > - * arminw: move LoadedObjectsRegistry > to odmg-layer > - */ > - // LoadedObjectsRegistry.register(result); > - /** > - * MBAIRD if you have multiple classes > mapped to a > - * table, and you query on the base class > you could get > - * back NON base class objects, so we > shouldn't pass > - * m_cld, but rather the class descriptor > for the > - * actual class. > - */ > - // fill reference and collection attributes > - ClassDescriptor cld = > getBroker().getClassDescriptor(result.getClass()); > - // don't force loading of reference > - final boolean unforced = false; > - // Maps ReferenceDescriptors to HashSets of > owners > - > getBroker().getReferenceBroker().retrieveReferences(result, cld, unforced); > - > getBroker().getReferenceBroker().retrieveCollections(result, cld, unforced); > - getCache().disableMaterializationCache(); > + try > + { > + getCache().cache(oid, result); > + /* > + * arminw: move LoadedObjectsRegistry to > odmg-layer > + */ > + // LoadedObjectsRegistry.register(result); > + /** > + * MBAIRD if you have multiple classes > mapped to a > + * table, and you query on the base > class you could get > + * back NON base class objects, so we > shouldn't pass > + * m_cld, but rather the class > descriptor for the > + * actual class. > + */ > + // fill reference and collection attributes > + ClassDescriptor cld = > getBroker().getClassDescriptor(result.getClass()); > + // don't force loading of reference > + final boolean unforced = false; > + // Maps ReferenceDescriptors to HashSets > of owners > + > getBroker().getReferenceBroker().retrieveReferences(result, cld, unforced); > + > getBroker().getReferenceBroker().retrieveCollections(result, cld, unforced); > + } > + finally > + { > + getCache().disableMaterializationCache(); > + } > } > } > } > > > > No revision > No revision > 1.5.2.1 +18 -3 > db-ojb/src/java/org/apache/ojb/broker/cache/Attic/InternalCache.java > > Index: InternalCache.java > =================================================================== > RCS file: > /home/cvs/db-ojb/src/java/org/apache/ojb/broker/cache/Attic/InternalCache.java,v > retrieving revision 1.5 > retrieving revision 1.5.2.1 > diff -u -r1.5 -r1.5.2.1 > --- InternalCache.java 25 Jun 2004 16:09:50 -0000 1.5 > +++ InternalCache.java 30 Sep 2004 14:02:03 -0000 1.5.2.1 > @@ -40,6 +40,7 @@ > private ObjectCache realCache; > private HashMap localCache; > private boolean enabledReadCache; > + private int invokeCounter; > > public InternalCache(ObjectCache realCache) > { > @@ -50,13 +51,25 @@ > > public void enableMaterializationCache() > { > + ++invokeCounter; > enabledReadCache = true; > } > > public void disableMaterializationCache() > { > - enabledReadCache = false; > - pushToRealCache(); > + if(enabledReadCache) > + { > + --invokeCounter; > + /* > + if materialization of the requested object was > completed, the > + counter represents '0' > + */ > + if (invokeCounter == 0) > + { > + enabledReadCache = false; > + pushToRealCache(); > + } > + } > } > > private void pushToRealCache() > @@ -114,6 +127,8 @@ > " push to real ObjectCache"); > } > localCache.clear(); > + invokeCounter = 0; > + enabledReadCache = false; > } > > public void clear() > > > > No revision > No revision > 1.83.2.4 +40 -3 > db-ojb/src/java/org/apache/ojb/broker/core/PersistenceBrokerImpl.java > > Index: PersistenceBrokerImpl.java > =================================================================== > RCS file: > /home/cvs/db-ojb/src/java/org/apache/ojb/broker/core/PersistenceBrokerImpl.java,v > retrieving revision 1.83.2.3 > retrieving revision 1.83.2.4 > diff -u -r1.83.2.3 -r1.83.2.4 > --- PersistenceBrokerImpl.java 21 Sep 2004 00:29:01 -0000 1.83.2.3 > +++ PersistenceBrokerImpl.java 30 Sep 2004 14:02:03 -0000 1.83.2.4 > @@ -446,6 +446,21 @@ > { > logger.warn("No running tx found, please only delete objects in > context of an PB-transaction" + > ", to avoid side-effects - e.g. when rollback of complex > objects"); > + /* > + arminw: > + this could help user to find missing tx declaration > + */ > + if(logger.isEnabledFor(Logger.INFO)) > + { > + try > + { > + throw new Exception("** Try to delete object > without active PersistenceBroker transaction **"); > + } > + catch(Exception e) > + { > + e.printStackTrace(); > + } > + } > } > try > { > @@ -1336,8 +1351,15 @@ > public Object getObjectByIdentity(Identity id) throws > PersistenceBrokerException > { > objectCache.enableMaterializationCache(); > - Object result = doGetObjectByIdentity(id); > - objectCache.disableMaterializationCache(); > + Object result; > + try > + { > + result = doGetObjectByIdentity(id); > + } > + finally > + { > + objectCache.disableMaterializationCache(); > + } > return result; > } > > @@ -1527,6 +1549,21 @@ > { > logger.warn("No running tx found, please only store in context of an > PB-transaction" + > ", to avoid side-effects - e.g. when rollback of complex > objects"); > + /* > + arminw: > + this could help user to find missing tx declaration > + */ > + if(logger.isEnabledFor(Logger.INFO)) > + { > + try > + { > + throw new Exception("** Try to store object > without active PersistenceBroker transaction **"); > + } > + catch(Exception e) > + { > + e.printStackTrace(); > + } > + } > } > // Invoke events on PersistenceBrokerAware instances and listeners > if (insert) > > > > Armin Waibel wrote: > > >>Hi Paul, >> >>I will have a look at this. >>Stay tuned! >> >>regards, >>Armin >> >>Nase, Paul R. wrote: >> >> >>>Hi, >>> >>>We are encountering a problem with our cache materializing half-baked >>>objects when under a heavy multi-user load. >>> >>>We are currently using OJB RC5 and are not in a position to move to >>>1.0.1 yet, >>>although in the future we would like to. We also have a JCS cache >>>configured. >>> >>>Our problem occurs when you have an object that contains references >>>to >>>2 or more >>>collections of other objects. The code in question is in the >>>RsIterator.getObjectFromResultSet() method. A snippet of the code is >>>shown below. >>> synchronized (result) >>> { >>> getCache().enableMaterializationCache(); >>> getCache().cache(oid, result); >>> /* >>> * arminw: move LoadedObjectsRegistry to odmg-layer >>> */ >>> // LoadedObjectsRegistry.register(result); >>> /** >>> * MBAIRD if you have multiple classes mapped to a >>> * table, and you query on the base class you could get >>> * back NON base class objects, so we shouldn't pass >>> * m_cld, but rather the class descriptor for the >>> * actual class. >>> */ >>> // fill reference and collection attributes >>> ClassDescriptor cld = >>>getQueryObject().getClassDescriptor().getRepository().getDescriptorFo >>>r(result.getClass()); >>> >>> // don't force loading of reference >>> final boolean unforced = false; >>> // Maps ReferenceDescriptors to HashSets of owners >>> getBroker().getReferenceBroker().retrieveReferences(result, >>>cld, unforced); >>> getBroker().getReferenceBroker().retrieveCollections(result, >>>cld, unforced); >>> getCache().disableMaterializationCache(); >>> } >>>When we have our object being materialized, it has 2 collections of >>>persistent objects. When getCache().cache(oid, result); is called, >>>the object is placed into a 'local' cache. When >>>retrieveCollections() is called, it is quite possible that this same >>>method (getObjectFromResultSet) will be called during the collection >>>object's materialization. The materialization of the collection >>>object then calls getCache().disableMaterializationCache(), which at >>>this point the 'local' >>>cache contains objects from only one of the collections (the one >>>currently being >>>processed) plus the original object being materialized. The >>>disableMaterializationCache() causes any of the objects in the 'local' >>>cache to >>>be 'pushed' to the 'real' cache. But the problem comes because the >>>original object we are materializing has two collections, one >>>collection has been retrieved, we are about to retrieve the other, >>>but the original object and only one of its collections has been >>>pushed to the real cache. >>>Another thread >>>may come and steal this invalid data out of the real cache and do >>>nasty things. >>> >>>Is there a handy solution to this problem? >>> >>>I am very very sorry for the long post... >>> >>>Paul Nase >>> >>> >>>--------------------------------------------------------------------- >>>To unsubscribe, e-mail: [EMAIL PROTECTED] >>>For additional commands, e-mail: [EMAIL PROTECTED] >>> >>> >>> >> >>--------------------------------------------------------------------- >>To unsubscribe, e-mail: [EMAIL PROTECTED] >>For additional commands, e-mail: [EMAIL PROTECTED] >> >> >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
