added pluggable key creation strategy: support for compound keys and prefix #7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
The current version of the MemcachedState does not support compound keys. Also, if several parts of the topology save distinct states to memcached, the current implementation could lead to key overlap and thus destruction of state information. For example, a count grouping/saving by word running simultaneously to a count grouping/saving by first letter of the word could both yield key overlap for one-letter words like "a" in English or "y" in French.
I added a mechanism that delegates to a IStateSingleKeyBuilder to make the creation of memcached key more flexible. I also added a default ConcatKeyBuilder implementation of IStateSingleKeyBuilder, which just concatenates the toString() of the compound key to solve issue 1 and allows a optional prefix to be configured to solve issue 2.
The new usage is backward compatible with the old syntax. I illustrate the new features in the updated example in Test.java.
Unfortunately, I had to de-activate usage of CachedMap to have this working: data with compound keys put in the CachedMap by the example topology are never retrieved. I failed to understand why and failed to reproduce a bug with junit in CachedMap with compound keys. Simply commenting out CachedMap from MemcachedState solved the issue.