#319 Need flag to support AWS Static and Dynamic modes. - #324
#319 Need flag to support AWS Static and Dynamic modes.#324ashishsodhi1 wants to merge 1 commit into
Conversation
|
Thanks for the PR! Can you tell what's the motivation, which problem is solved and how to use it? |
|
For me questions arise with the title already, as I don't know what's static / dynamic mode. Links to documentation would of course be useful as well. |
|
Hi Dynamic mode -> http://docs.aws.amazon.com/AmazonElastiCache/latest/UserGuide/AutoDiscovery.Using.html Static mode->http://docs.aws.amazon.com/AmazonElastiCache/latest/UserGuide/AutoDiscovery.Manual.html Our specific issue was that we were packaging Amazon ElastiCache Cluster Client with our application for application caching so did not have the option of switching between Elastic Client and spymemcached client but still had the requirement to configure session backup with either manually or with dynamic discovery to support testing in local, uat, stage, and aws production environments differently.. |
|
Hi Margo, Thanks |
|
@sbellary Not yet, sorry, I'm having a look now. |
| import net.spy.memcached.DefaultConnectionFactory; | ||
|
|
||
| /** | ||
| * Created by asodhi on 11/29/2016. |
There was a problem hiding this comment.
If you're adding class documentation other content would be more useful :-)
There was a problem hiding this comment.
Will drop the class documentation looks like pretty much self explanatory.
| return maxReconnectDelay; | ||
| } | ||
| }; | ||
| } catch (final Exception e) { |
There was a problem hiding this comment.
try/catch should not be needed here.
| return maxReconnectDelay; | ||
| } | ||
| }; | ||
| } catch (final Exception e) { |
| import java.lang.reflect.*; | ||
|
|
||
| /** | ||
| * Created by asodhi on 11/29/2016. |
There was a problem hiding this comment.
More useful documentation, or drop it?
| /** | ||
| * Created by asodhi on 11/29/2016. | ||
| */ | ||
| public class MemcachedElasticConnectionFactory implements StorageClientFactory.MemcachedNodeURLBasedConnectionFactory { |
There was a problem hiding this comment.
Rename to MemcachedElastiCacheConnectionFactory?
| long maxReconnectDelay, Statistics statistics ); | ||
| } | ||
|
|
||
| static interface MemcachedNodeURLBasedConnectionFactory { |
There was a problem hiding this comment.
Rename to MemcachedConnectionFactory? The implementation class MemcachedConnectionFactory could then be renamed to e.g. StandardMemcachedConnectionFactory.
|
|
||
| static interface MemcachedNodeURLBasedConnectionFactory { | ||
| ConnectionFactory createBinaryConnectionFactory(long operationTimeout, | ||
| long maxReconnectDelay, boolean isClientDynamicMode); |
There was a problem hiding this comment.
The isClientDynamicMode should not be part of this API, because its specific to the ElastiCache specific implementation.
There was a problem hiding this comment.
Normally the client mode is determined by the node name in aws client. If the node name contains ".cfg." it would be a dynamic mode. Either we pass the Boolean or we pass the node name?
How about
ConnectionFactory createBinaryConnectionFactory(long operationTimeout,
long maxReconnectDelay, String node);
or
I could add a parameter to the constructor of MemcachedElastiCacheConnectionFactory?
|
|
||
| protected static MemcachedNodeURLBasedConnectionFactory createMemcachedNodeURLBasedConnectionFactory() { | ||
| try { | ||
| Class.forName("net.spy.memcached.ClientMode"); |
There was a problem hiding this comment.
This could be extracted into a method like isElastiCacheClientLib (which also hides exception handling), so that here would be a simple if/else.
| boolean isClientDynamicMode = false; | ||
| if (memcachedNodesManager.getMemcachedNodes().contains(".cfg.")) | ||
| { | ||
| isClientDynamicMode=true; |
There was a problem hiding this comment.
Why is the clientMode set to "Dynamic" statically? And why is this coupled to if (memcachedNodesManager.getMemcachedNodes().contains(".cfg."))?
I'd expect that the environment variable / system property client.mode is evaluated...
And as already said, determining the clientMode should be done inside the MemcachedElastiCacheConnectionFactory.
There was a problem hiding this comment.
Normally the client mode is determined by the node name in aws client. If the node name contains ".cfg." it would be a dynamic mode.
Check the implementation in
aws-elasticache-cluster-client-memcachedfor-java -->
/src/main/java/net/spy/memcached/MemcachedClient.java line
...
if(determineClientMode){
if(addrs.size() == 1){
if(addrs.get(0) == null){
throw new NullPointerException("Socket address is null");
}
String hostName = addrs.get(0).getHostName();
//All config endpoints has ".cfg." subdomain in the DNS name.
if(hostName != null && hostName.contains(".cfg.")){
cf = new DefaultConnectionFactory(ClientMode.Dynamic);
}
}
//Fallback to static mode
if(cf == null){
cf = new DefaultConnectionFactory(ClientMode.Static);
}
...
| } | ||
| } | ||
|
|
||
| protected static MemcachedNodeURLBasedConnectionFactory createMemcachedNodeURLBasedConnectionFactory() { |
There was a problem hiding this comment.
Rename to createMemcachedConnectionFactory?
|
Is this being worked on? I have the exact same need. |
|
@eschulma I think it's stalled. Maybe you want to pick up what's already there and continue with this? |
|
My environment is working, and I don't remember much about this now. But it looked like @ashishsodhi was close to being done. |
|
@eschulma ok :-) |
5 participants
No description provided.