SourceForge.net Logo
Main Overview Wiki Issues Forum Build Fisheye
Issue Details (XML | Word | Printable)

Key: CMP-913
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Shay Banon
Reporter: Patrick Twohig
Votes: 1
Watchers: 5
Operations

If you were logged in you would be able to see more operations.
Compass

Google App Engine: Improved transaction management

Created: 31/Dec/09 07:07 PM   Updated: 25/Feb/10 01:08 PM
Component/s: None
Affects Version/s: None
Fix Version/s: 2.3.0 beta1

File Attachments: 1. Text File Compass-GAE-1-1-2010.patch (42 kB)
2. Text File Compass-TestCase-GAE-1-1-2009.patch (0.7 kB)
3. Java Source File HelloWorld.java (2 kB)
4. Java Source File PMF.java (2 kB)
5. Java Source File TestDataObject.java (0.9 kB)



 Description  « Hide
I'm getting an error while trying to put new objects into the datastore using JDO. I think it's related to this issue which has been brought up on the forums
http://forum.compass-project.org/thread.jspa?threadID=216436&tstart=0. Apologies in advance if this is a duplicate of another issue. As I see it right now, the code in the GoogleAppEngineDirectory does not write to the index in its own transaction, and since Google App Engine restricts transactions to a single entity group the whole thing fails if you write to the datastore from within a transaction.

I'm putting together a patch for the org.compass.needle.gae.* classes. It basically puts the locks in their own entity group, and performs the locking/unlocking in their own transaction. Additionally, it isloates writes to the index in a transaction of their own. Off hand, I couldn't think of a better way to do it. I'll post the patch sometime in the next couple of days.



 All   Comments   Change History      Sort Order: Ascending order - Click to sort in descending order
Patrick Twohig added a comment - 01/Jan/10 02:16 AM
Maybe I'm Doing It Wrong, but here's a bit of code that reproduces the bug. From what I can read in the GoogleAppEngineDirectory class, it starts manipulating the data store within the context of a transaction, however the index isn't stored in the same entity group as the object being stored to the data store. In this case it's the instance of TestDataObject.

Patrick Twohig added a comment - 01/Jan/10 08:13 PM
Here's the patch! I ended up using the latest Google App Engine API. A few things have changed since the original directory store has been written, which appears to be the ability to run queries from within a transaction. I also had to add a few lines to some of the unit test code, but I ran through the tests a few times and they all appear to have passed.

Here's what this patch does and the rationale behind it all.

1) It puts all locks in their own entity group, separate from the entities which house the index. This way a separate transaction can be used to lock/unlock the search index while writing. According to the guys in the #appengine IRC channel and from what I could gather form the GAE documentation putting two entities in the same entity group isn't necessary unless you want to use them within the same transaction. In short it provides not significant performance improvements. In our case, with locks, we're doing exactly the opposite and never manipulate a lock outside of a transaction so there's no good reason to put it in the same entity group as the directory.
2) If a transaction is running when writing to the data store, it performs the writes in a new transaction the restores the current transaction to the previous state. This way, updating the search index does not interfere with any currently running transactions.
3) Obtaining and releasing locks is performed in a separate transaction, regardless as to what's going on.
4) Guarantees that everywhere the GoogleAppEngineDirectoryStore begins a transaction it will attempt to commt it, or roll it back if an exception occurs.
5) Added a configuration option to tune the amount of times it will attempt to repeat transactions if the application is experiencing heavy load.

Happy New Year,
Patrick.


Antony Trupe added a comment - 03/Jan/10 05:47 PM
Maybe I applied the patch wrong and/or built the jar wrong, or maybe this is a separate issue totally...
I get the following error when persisting objects to the datastore:
....
com.bitdual.client.rpc.RedirectException' threw an unexpected exception: org.compass.gps.device.jdo.JdoGpsDeviceException: {appengine}: Failed while updating [1:1]; nested exception is org.compass.core.engine.SearchEngineException: Transaction is set as read only
at com.google.gwt.user.server.rpc.RPC.encodeResponseForFailure(RPC.java:378)
at com.google.gwt.user.server.rpc.RPC.invokeAndEncodeResponse(RPC.java:581)
at com.google.gwt.user.server.rpc.RemoteServiceServlet.processCall(RemoteServiceServlet.java:188)
at com.google.gwt.user.server.rpc.RemoteServiceServlet.processPost(RemoteServiceServlet.java:224)
at com.google.gwt.user.server.rpc.AbstractRemoteServiceServlet.doPost(AbstractRemoteServiceServlet.java:62)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:713)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:806)
at org.mortbay.jetty.servlet.ServletHolder.handle(ServletHolder.java:487)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1093)
at com.google.appengine.api.blobstore.dev.ServeBlobFilter.doFilter(ServeBlobFilter.java:51)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1084)
at com.google.apphosting.utils.servlet.TransactionCleanupFilter.doFilter(TransactionCleanupFilter.java:43)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1084)
at com.google.appengine.tools.development.StaticFileFilter.doFilter(StaticFileFilter.java:121)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1084)
at org.mortbay.jetty.servlet.ServletHandler.handle(ServletHandler.java:360)
at org.mortbay.jetty.security.SecurityHandler.handle(SecurityHandler.java:216)
at org.mortbay.jetty.servlet.SessionHandler.handle(SessionHandler.java:181)
at org.mortbay.jetty.handler.ContextHandler.handle(ContextHandler.java:712)
at org.mortbay.jetty.webapp.WebAppContext.handle(WebAppContext.java:405)
at com.google.apphosting.utils.jetty.DevAppEngineWebAppContext.handle(DevAppEngineWebAppContext.java:70)
at org.mortbay.jetty.handler.HandlerWrapper.handle(HandlerWrapper.java:139)
at com.google.appengine.tools.development.JettyContainerService$ApiProxyHandler.handle(JettyContainerService.java:352)
at org.mortbay.jetty.handler.HandlerWrapper.handle(HandlerWrapper.java:139)
at org.mortbay.jetty.Server.handle(Server.java:313)
at org.mortbay.jetty.HttpConnection.handleRequest(HttpConnection.java:506)
at org.mortbay.jetty.HttpConnection$RequestHandler.content(HttpConnection.java:844)
at org.mortbay.jetty.HttpParser.parseNext(HttpParser.java:644)
at org.mortbay.jetty.HttpParser.parseAvailable(HttpParser.java:211)
at org.mortbay.jetty.HttpConnection.handle(HttpConnection.java:381)
at org.mortbay.io.nio.SelectChannelEndPoint.run(SelectChannelEndPoint.java:396)
at org.mortbay.thread.BoundedThreadPool$PoolThread.run(BoundedThreadPool.java:442)
Caused by: org.compass.gps.device.jdo.JdoGpsDeviceException: {appengine}: Failed while updating [1:1]; nested exception is org.compass.core.engine.SearchEngineException: Transaction is set as read only
at org.compass.gps.device.jdo.Jdo2GpsDevice$JdoGpsInstanceLifecycleListener.postStore(Jdo2GpsDevice.java:158)
at org.datanucleus.jdo.JDOCallbackHandler.postStore(JDOCallbackHandler.java:140)
at org.datanucleus.state.JDOStateManagerImpl.internalMakePersistent(JDOStateManagerImpl.java:3189)
at org.datanucleus.state.JDOStateManagerImpl.makePersistent(JDOStateManagerImpl.java:3161)
at org.datanucleus.ObjectManagerImpl.persistObjectInternal(ObjectManagerImpl.java:1298)
at org.datanucleus.ObjectManagerImpl.persistObject(ObjectManagerImpl.java:1175)
at org.datanucleus.jdo.JDOPersistenceManager.jdoMakePersistent(JDOPersistenceManager.java:669)
at org.datanucleus.jdo.JDOPersistenceManager.makePersistent(JDOPersistenceManager.java:694)
at com.bitdual.server.dao.QuestionDao.save(QuestionDao.java:271)
at com.bitdual.server.QuestionServiceImpl.save(QuestionServiceImpl.java:102)
at com.bitdual.server.QuestionServiceImpl.save(QuestionServiceImpl.java:557)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
at java.lang.reflect.Method.invoke(Unknown Source)
at com.google.appengine.tools.development.agent.runtime.Runtime.invoke(Runtime.java:100)
at com.google.gwt.user.server.rpc.RPC.invokeAndEncodeResponse(RPC.java:562)
... 30 more
Caused by: org.compass.core.engine.SearchEngineException: Transaction is set as read only
at org.compass.core.lucene.engine.LuceneSearchEngine.verifyNotReadOnly(LuceneSearchEngine.java:130)
at org.compass.core.lucene.engine.LuceneSearchEngine.createOrUpdate(LuceneSearchEngine.java:273)
at org.compass.core.lucene.engine.LuceneSearchEngine.save(LuceneSearchEngine.java:263)
at org.compass.core.impl.DefaultCompassSession.save(DefaultCompassSession.java:489)
at org.compass.core.impl.DefaultCompassSession.save(DefaultCompassSession.java:473)
at org.compass.core.impl.ExistingCompassSession.save(ExistingCompassSession.java:313)
at org.compass.core.impl.ExistingCompassSession.save(ExistingCompassSession.java:313)
at org.compass.gps.device.jdo.Jdo2GpsDevice$JdoGpsInstanceLifecycleListener$2.doInCompassWithoutResult(Jdo2GpsDevice.java:151)
at org.compass.core.CompassCallbackWithoutResult.doInCompass(CompassCallbackWithoutResult.java:29)
at org.compass.core.CompassTemplate.execute(CompassTemplate.java:133)
at org.compass.gps.impl.SingleCompassGps.executeForMirror(SingleCompassGps.java:151)
at org.compass.gps.device.jdo.Jdo2GpsDevice$JdoGpsInstanceLifecycleListener.postStore(Jdo2GpsDevice.java:149)
... 46 more

Patrick Twohig added a comment - 03/Jan/10 08:40 PM
Dumb question, but did this happen only after you applied the patch? The stack trace doesn't indicate that the org.compass.needle.gae.* classes directly caused the exception. If the patch is the cause of the problem, my best guess is that after GoogleAppEngineDirectory.doInTransaction() is called, it leaves the DatastoreService object in a screwed up state.

What I do know is that the call to PersistenceManager.makePersistent(Object) calls some Compass code which doesn't necessarily play well with how GAE deals with transactions. What happens in LuceneSearchEngine.createOrUpdate()?


Antony Trupe added a comment - 03/Jan/10 08:46 PM
It happened both before and after I applied the patch.
I'm working on a test app in hopes of isolating the issue, whether it be in your patch, compass, and/or my code(the latter two being more likely, and that the last most likely).

Antony Trupe added a comment - 03/Jan/10 10:08 PM
Some combination of your patch and the following change in my code resolved my issue.

CompassSearchSession search = PMF.getCompass().openSearchSession();
needed a matching
search.close();


Patrick Twohig added a comment - 04/Jan/10 12:49 AM
Ah, well, that's good. It's always best practice to close what you leave open...now that I think about it there may be places where I forget to do that in my code....anyhow. My best guess is that without calling close() you leave some write-lock in place which screws up the index the next time you try to access it.

pinecone added a comment - 06/Jan/10 09:15 AM
Hi Patrick and Shay,

First of all, nice work!

I wonder if the following approach works, which looks a little bit simpler to me:

1) Move the mirroring operations of a GpsDevice to the GAE background tasks and push these tasks to a task queue. So if the current write operation to datastore is in a transaction conext, this transaction will not be interfered since no index, which resides in a different entity group, is written yet.

2) When GAE runs the tasks (using a new thread), wrap the mirroring operations by a transaction to ensure isolation between index writes. This transaction includes only one sigle entity group that the index belongs to.

The drawback of this approach is that the order by which the index is written may differ from that by which the domain objects are saved, since the GAE queue may not execute tasks in FIFO order all the time (for example, when GAE system crashed and is recovering). But this may be fine to some applications (at least mine )

Any comment?


Shay Banon added a comment - 08/Jan/10 06:47 AM
Changing the title to reflect the changes required.

Shay Banon added a comment - 08/Jan/10 06:47 AM
Committed the changes, thanks, they look good. Hopefully this will solve the transaction problems people were having.

Patrick Twohig added a comment - 08/Jan/10 11:35 AM
Let's hope so. I think pinecone is on to something. It's probably a good idea to offload the indexing into a separate task. That being said, it's probably a good idea to file that under another issue if anybody makes any headway on it.

pinecone added a comment - 08/Jan/10 09:15 PM
Patrick,

Yes, I spent some time on my proposal, and the out-of-order problem can be solved by using versioning (i.e., @version in JPA/JDO).

However, later I realized that there is a limitation on GAE: all tasks in different queues together can have execution rate at most 20/sec.

I think currently, your patch is good enough .


Patrick Twohig added a comment - 13/Jan/10 02:46 PM
I think task queueing is the best solution because we may be over burdening the transaction every time. But I'd say wait until task queues for Java are no longer in the "experimental" stage.

Len Takeuchi added a comment - 24/Jan/10 02:51 PM
Hello,

I still seem to be experiencing index corruption. From my understanding, what these improvements have done is to isolate updates to index entities made from any pre-existing transactions and hence avoid the problem of updating entities in multiple entity groups in a transaction which is not allowed in App Engine. However, correct me if I'm wrong but changes made to index entities within an index session even for a single save operation may be done by multiple write operations which would each run in their own transactions. So does this mean that if I'm doing a save operation and there are some writes committed but eventually there is a write error, for example due to App Engines annoying 30 second limit being reached, then the index would be corrupted?

Len


Miroslav Genov added a comment - 25/Feb/10 02:54 AM
Hello,
I'm encountering similar problem with transactions. Please note that I'm not using automatic indexes and I'm doing indexing manually, i.e indexing is executed by task queue call page by page until all entities where indexed. When any of my entities is updated, new task is added in the task queue for that entity.

Here is the error which I'm encountering when multiple users are editing multiple documents:

org.compass.core.engine.SearchEngineException: Failed to prepare transaction for sub index [contractentity]; nested exception is java.lang.NullPointerException: null
java.lang.NullPointerException
at org.compass.core.lucene.engine.transaction.readcommitted.TransIndex.commit(TransIndex.java:130)
at org.compass.core.lucene.engine.transaction.readcommitted.TransIndexManager.commit(TransIndexManager.java:105)
at org.compass.core.lucene.engine.transaction.readcommitted.ReadCommittedTransactionProcessor$PrepareCallable.call(ReadCommittedTransactionProcessor.java:458)
at org.compass.core.lucene.engine.transaction.readcommitted.ReadCommittedTransactionProcessor.doPrepare(ReadCommittedTransactionProcessor.java:128)
at org.compass.core.lucene.engine.transaction.support.AbstractConcurrentTransactionProcessor.prepare(AbstractConcurrentTransactionProcessor.java:115)
at org.compass.core.lucene.engine.transaction.readcommitted.ReadCommittedTransactionProcessor.doCommit(ReadCommittedTransactionProcessor.java:144)
at org.compass.core.lucene.engine.transaction.support.AbstractConcurrentTransactionProcessor.commit(AbstractConcurrentTransactionProcessor.java:127)
at org.compass.core.lucene.engine.LuceneSearchEngine.commit(LuceneSearchEngine.java:163)
at org.compass.core.transaction.LocalTransaction.doCommit(LocalTransaction.java:91)
at org.compass.core.transaction.AbstractTransaction.commit(AbstractTransaction.java:46)
at org.compass.core.impl.DefaultCompassSession.close(DefaultCompassSession.java:746)
at org.compass.core.impl.DefaultCompassSession.commit(DefaultCompassSession.java:729)
at com.evo.adm.searchengine.CompassSearchEngine.indexEntity(CompassSearchEngine.java:163)


Patrick Twohig added a comment - 25/Feb/10 12:49 PM
I just started having a similar issue last night. None of the classes in your stack trace pertain to what's in the patch, maybe file under a different issue?