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

Make memcache use default namespace. #43

Merged
merged 1 commit into from
Aug 3, 2015
Merged

Make memcache use default namespace. #43

merged 1 commit into from
Aug 3, 2015

Conversation

jongillham
Copy link
Member

This fixes bug #42 by ensuring all memcached entities use the default namespace despite the namespace of their respective keys.

@luna-duclos
Copy link

Wouldn't this cause weird issues if a two entities in different namespaces have the same key ? I believe the namespace chosen for memcache should always be the one from the key

This fixes bug #42 by ensuring all
memcached entities use the default namespace despite the namespace of
their respective keys.
@jongillham
Copy link
Member Author

@PSG-Luna that was my original though. Unfortunately it is not scalable to have memcach.Items use the same namespace as the datastore.Keys associated with them. This is because memcache.*Multi will not allow putting multiple entities into different namespaces in one call, unlike datastore.*Multi.

The way around it is to put all memcached entities into one namespace (currently the default) and do 'namespacing' ourselves. If you look at createMemcacheKey(key *datastore.Key) you will see it uses datastore.Key.Encode() which encodes the namespace as well. This means that there will be no key collisions between identical keys in different namespaces.

What do you think?

@luna-duclos
Copy link

Seems reasonable enough, thanks :)

jongillham added a commit that referenced this pull request Aug 3, 2015
Make memcache use default namespace.
@jongillham jongillham merged commit bdefbfb into qedus:master Aug 3, 2015
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

Successfully merging this pull request may close these issues.

2 participants