Consistency issue using aetest
zboralski opened this issue · 10 comments
If I try to create an entity in a unit test :
u := &User{Name: "Joe", CName: "joe"}
k, err := n.Put(u)
if err != nil {
t.Fatal(err)
}
Parentless queries will fail because of a consistency issue (parentless puts may not be available right away.)
The problem can be avoided by sleeping for 1 second but the test becomes dependent on the machine load, etc.
// Sleep a bit to wait for the HRD emulation to get out of our way
time.Sleep(1000 * time.Millisecond)
According to this post the proper way to avoid this issue inside a test is to get the entity using its known key, right after.
My subsequent queries work perfectly if I perform a datastore.Get(c,k,u) first.
However they fail miserably if I use n.Get(u) even though the key is known and set:
Sure you are not modifying the local cache? See my misunderstanding here. If you're not, please issue a pull request with a failing test.
See issue #27
No I am not touching the local cache and in goon_test.go you can see :
// Sleep a bit to wait for the HRD emulation to get out of our way
time.Sleep(1000 * time.Millisecond)
I’m hosting an event today so I’ll send the pull request tomorrow. Cheers!
On 15 Aug 2014, at 23:31, mzimmerman notifications@github.com wrote:
Sure you are not modifying the local cache? See my misunderstanding here. If you're not, please issue a pull request with a failing test.
See issue #27 #27
—
Reply to this email directly or view it on GitHub #41 (comment).
The TestCaches test seems to test exactly what you're stating is the issue and it passes for me.
goapp test -run Caches
The only time we're doing time.Sleep in the goon tests is so that we can run queries successfully. As the thread you linked to states, the datastore needs time to build the index so your queries are successful. It's not that we're waiting to fetch the data by key.
What version of the SDK are you using?
What I was trying to say is that the proper way to do the test would be to fetch the entity created right away using get with the key returned by the put. Performing this get ensures that the entity is written to the datastore and indexed. Once you do that queries will work without having to rely on a sleep timer.
Using a datastore get after a put works, using a goon get, the test fails. I'll post the PR tomorrow.
On 16 Aug 2014, at 13:28, mzimmerman notifications@github.com wrote:
The TestCaches test seems to test exactly what you're stating is the issue and it passes for me.
goapp test -run CachesThe only time we're doing time.Sleep in the goon tests is so that we can run queries successfully. As the thread you linked to states, the datastore needs time to build the index so your queries are successful. It's not that we're waiting to fetch the data by key.
What version of the SDK are you using?
—
Reply to this email directly or view it on GitHub.
Has anyone actually had the goon tests fail because of the sleep? I have never seen it. It's important to note that the goon tests will never produce a false positive due to this sleep. That said, I guess it wouldn't be a problem to swap the sleep out for a get, if there's more evidence that a get will actually commit the indexes.
However, @zboralski, it seems to me from your initial post that your problem isn't with the goon tests but with your own tests.
The thing with goon's Put is that it populates the local cache. The reason why just doing a Get afterwards doesn't achieve your goal has to do with the fact that the Get will just retrieve the entity from the local cache and no underlying datastore.Get is ever made. You would need to clear the local cache with FlushLocalCache before doing the Get.
Go figure. As I understood it, doing a Get by key would cause writing all the pending writes to the datastore, but I didn't think it would ensure that indexes are generated but it does indeed seem to do that (at least on the dev SDK). I've issued a pull request for that.
I have gotten queries to fail due to the sleep (not goon's but my own).
@xStrom's got it right. I was misreading what the error was, but now seeing his explanation I get it. Your own tests are failing since your goon.Get call doesn't actually go to the datastore since it pulls from local cache, so it doesn't ensure your indexes are being generated in time. Either flushing goon local cache or doing a datastore.Get (like you've done) is a good solution.
@xStrom flushing the local cache works. I should have thought of that. Thank you.
@mzimmerman Thanks, that's perfect!
@mzimmerman gets executed in a request following a write in the previous request fail quite often... that's in an app not a test.
Also @xStrom flushing the local cache before every get really defeats the purpose of having a local cache...
It's important that we remember the scope of the conversation - which is unit tests. I suggested using FlushLocalCache precisely because the local cache was getting in your way. However if you have a different goal, where you want to use the local cache, then of course flushing it works against your goal.
Yes, this is not a goon issue. What you're essentially now asking for is the goon.Get() function to go back to the datastore when the key is already in the cache. If we did that, goon would really be failing in it's mission.
In your tests, when you need to query results of an entity, do the following.
datastore.Get(c,g.Key(data),data)Then continue on in your tests as normal. This does not destroy the local cache. Note that if you're having this issue in your tests, you'll also have it in production. Datastore queries are eventual consistency, not immediate -- this is regardless of using goon.