google-code-export / objectify-appengine

Automatically exported from code.google.com/p/objectify-appengine
MIT License
1 stars 0 forks source link

An entity with a Cached annotation is not flushed when put in a transaction #30

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I created an entity that had an @Parent(so as to be part of a specific entity 
group) 
and was annotated with @Cached. This entity has a simple List member.
My test created the entity with O1 (O for Objectify).

The entity was read with O2 (O2 was a transactional Objectify) and an item was 
added to 
the List. Item was put and committed using O2.

The entity was read with O3 and the size of the List was printed. It remained 
zero.

I re-did the above using a non-trasnactional O2 and the size of the List when 
printed 
was 1.

Original issue reported on code.google.com by btoc...@gmail.com on 6 Apr 2010 at 1:43

GoogleCodeExporter commented 9 years ago
I've added a test that does exactly what you describe and it passes (at least, 
on the local datastore):

http://code.google.com/p/objectify-
appengine/source/browse/trunk/src/com/googlecode/objectify/test/TransactionTests
.java

Can you create a test that demonstrates the behavior you describe?

Original comment by lhori...@gmail.com on 6 Apr 2010 at 6:14

GoogleCodeExporter commented 9 years ago
Took me a while to reproduce in a unit test. I had to use a parent and String 
Id 
values. I have attached the test case.

Original comment by btoc...@gmail.com on 6 Apr 2010 at 10:17

Attachments:

GoogleCodeExporter commented 9 years ago
This is turning out to be one of the most insidious bugs I've ever tried to 
hunt down.  The bug doesn't manifest if 
you use a different name for parent and child keys, or if you clear the 
memcache, or if you create a fresh Entity 
instead of loading it out of the datastore before changing.

Bizarre.  Will continue in the morning.

Original comment by lhori...@gmail.com on 7 Apr 2010 at 8:55

GoogleCodeExporter commented 9 years ago
It also doesn't manifest if you do not perform the change in a transaction. It 
took me 
hours to replicate the bug in a unit test. Also if one changes the id on 
SimpleParent 
and SimpleEntity to long it works fine.

Original comment by btoc...@gmail.com on 7 Apr 2010 at 2:04

GoogleCodeExporter commented 9 years ago
btw, when reviewing the code for CachingDatastoreService I think I spotted a 
number of 
problems.
When an entity is put as part of a transaction, deferCachePut is used to stored 
the 
entity in a Map until the commit occurs. At this point a check is made to make 
sure the 
entity is Cacheable. When the commit is done the entity is put directly into 
Memcached.
I would recommend you user putInCache() to put the entity into Memcached so 
expiration 
annotations can be obeyed. Also putInCache() does the check to make sure the 
entity is 
Cacheable.

Original comment by btoc...@gmail.com on 7 Apr 2010 at 5:45

GoogleCodeExporter commented 9 years ago
This is unexpected!

@Test
    public void testEntityKeys() throws Exception {
        // Need to register it so the entity kind becomes cacheable
        DatastoreService ds = DatastoreServiceFactory.getDatastoreService();

        com.google.appengine.api.datastore.Key parentKeyA = 
KeyFactory.createKey("SimpleParent", "same");
        com.google.appengine.api.datastore.Key childKeyA = 
KeyFactory.createKey(parentKeyA, "SimpleEntity", "different");

        Entity entA1 = new Entity(childKeyA);
        ds.put(entA1);

        Entity entA2 = ds.get(childKeyA);

        assert new String(MemcacheSerialization.makePbKey(entA1.getKey())).equals(new 
String(MemcacheSerialization.makePbKey(childKeyA)));
        assert new String(MemcacheSerialization.makePbKey(entA2.getKey())).equals(new 
String(MemcacheSerialization.makePbKey(childKeyA)));

        com.google.appengine.api.datastore.Key parentKeyB = 
KeyFactory.createKey("SimpleParent", "same");
        com.google.appengine.api.datastore.Key childKeyB = 
KeyFactory.createKey(parentKeyB, "SimpleEntity", "same");

        Entity entB1 = new Entity(childKeyB);
        ds.put(entB1);

        Entity entB2 = ds.get(childKeyB);

        assert new String(MemcacheSerialization.makePbKey(entB1.getKey())).equals(new 
String(MemcacheSerialization.makePbKey(childKeyB)));
        assert new String(MemcacheSerialization.makePbKey(entB2.getKey())).equals(new 
String(MemcacheSerialization.makePbKey(childKeyB)));

    }

Original comment by btoc...@gmail.com on 7 Apr 2010 at 6:18

GoogleCodeExporter commented 9 years ago
The change to using putInCache() (and deleteFromCache()) in commit() was made 
yesterday - it wasn't the issue, 
but it was an issue.  I can tell you're paying attention :-)

Funny, you're following almost the exact same path that Scott and I have been 
following!  This bug should win 
some sort of Evil award.

BTW we have managed to work around the problem in Objectify by using the string 
version of Key as the 
memcache key.  The change is in svn trunk.

Original comment by lhori...@gmail.com on 7 Apr 2010 at 6:25

GoogleCodeExporter commented 9 years ago
I've added this test (and the related tests) to EvilMemcacheBugTests.java.

Original comment by lhori...@gmail.com on 7 Apr 2010 at 6:35

GoogleCodeExporter commented 9 years ago
Great, thanks for your work. Great library btw!

Original comment by btoc...@gmail.com on 7 Apr 2010 at 6:36

GoogleCodeExporter commented 9 years ago
btoc007, do you want to report this as an appengine issue or shall we?

Original comment by lhori...@gmail.com on 7 Apr 2010 at 8:12

GoogleCodeExporter commented 9 years ago
I'll let you guys do it

Original comment by btoc...@gmail.com on 7 Apr 2010 at 8:13

GoogleCodeExporter commented 9 years ago
Here's the Appengine issue:

http://code.google.com/p/googleappengine/issues/detail?id=3060

I'm closing this as Fixed in Objectify since we have a workaround.  Huge thanks 
for hunting it down!  This was a 
really, really evil problem.

Original comment by lhori...@gmail.com on 8 Apr 2010 at 2:26