Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistency issue using aetest #41

Closed
zboralski opened this issue Aug 15, 2014 · 10 comments
Closed

Consistency issue using aetest #41

zboralski opened this issue Aug 15, 2014 · 10 comments

Comments

@zboralski
Copy link

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:

@mzimmerman
Copy link
Contributor

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

@zboralski
Copy link
Author

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).

@mzimmerman
Copy link
Contributor

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?

@zboralski
Copy link
Author

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 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?


Reply to this email directly or view it on GitHub.

@xStrom
Copy link
Collaborator

xStrom commented Aug 17, 2014

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.

@mzimmerman
Copy link
Contributor

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.

@zboralski
Copy link
Author

@xStrom flushing the local cache works. I should have thought of that. Thank you.

@mzimmerman Thanks, that's perfect!

@zboralski
Copy link
Author

@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...

@xStrom
Copy link
Collaborator

xStrom commented Feb 4, 2015

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.

@mzimmerman
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants