Skip to content

Make GetMulti cache results #2

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

Open
mjohnson9 opened this issue Mar 9, 2013 · 5 comments
Open

Make GetMulti cache results #2

mjohnson9 opened this issue Mar 9, 2013 · 5 comments

Comments

@mjohnson9
Copy link
Contributor

ds.GetMulti currently does not cache results, citing complexity due to mixed entity groups. I feel that it's appropriate to add this as an issue because it's a missing core feature.

@kylefinley
Copy link
Member

I agree dsGetMulti should use cache results. I can't recall exactly why this was skipped; I think I thought that it might end up being slower then just getting the entities from the datastore. In any case, it seems like it should be fairly strait-forward to implement:

  1. Check the dst type. If it's not a list create one.
  2. Iterate through the key's and dst's using ds.Get to populating a new list with the results.

Could goroutine be used here to speed things up?
Would you like to implement this?

@mjohnson9
Copy link
Contributor Author

@kylefinley,
I feel that implementing it in this manner would be a bad idea. I will explain when I get to my computer.

@mjohnson9
Copy link
Contributor Author

@kylefinley,
I'll explain now:

The reason that I believe this to be a bad idea is due to the fact that a normal GetMulti is a single procedure call to retrieve (potentially) hundreds of models. Therefore, I would assume that a single GetMulti for 2,000 records would be significantly faster than 2,000 Gets. I propose the following solution as a draft:

  1. Create a map[string][]*datastore.Key where the string corresponds to the entity groups of the keys contained in the slice. (we'll call it groupMap)
  2. For each input key, groupMap[entityGroup] = append(groupMap[entityGroup], key)
  3. We create a slice for all keys to be retrieved from each: memory, memcache, datastore and error (appengine.MultiError).
  4. We iterate over the groupMap, getting the storage settings for each entity group. After getting the settings, we add all keys to their appropriate slice.
  5. We attempt to retrieve the keys from memory. Any keys that fail get added to the memcache list.
  6. We attempt to retrieve the keys from memcache. Any keys that fail get added to the datastore list.
  7. We attempt to retrieve the keys from the datastore. Any keys that fail get an ErrNoSuchEntity error added to their respective slot in the MultiError.

What do you think?

@kylefinley
Copy link
Member

Ah, yes, you're absolutely right, calling GetMulti should be preferred.

Your draft sounds excellent, you definitely have a better grasp of this then I do. If you have the time please free to implement this; just be sure to add a couple of tests for the MultiError order and multiple dst types.

@mjohnson9
Copy link
Contributor Author

I'll most likely not have the time to implement such a thing for a few weeks, unfortunately. If you would like to do so, please feel free. If you haven't taken this on by the time that I have some free time then I'll probably implement it.

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

No branches or pull requests

2 participants