You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@curator.apache.org by "Aleksander Melnichnikov (JIRA)" <ji...@apache.org> on 2018/03/01 09:46:00 UTC
[jira] [Updated] (CURATOR-457) Configuring service discovery paths
[ https://issues.apache.org/jira/browse/CURATOR-457?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Aleksander Melnichnikov updated CURATOR-457:
--------------------------------------------
Description:
Right now paths for services in service discovery hardcoded to pattern: */\{basePath} /\{serviceName}/\{instanceId}.* And that's impossible to configure paths this way: */\{basePathPart1} /\{serviceName}/\{basePathPart2}/\{instanceId}*
For example:
If basePath = */base1/base2/base3*, serviceName = *serviceName*, instanceId - *serviceId*
then path will be */**base1**/**base2**/**base3**/**serviceName**/**serviceId*, that can't be configured like this: */**base1**/**base2/serviceName**/**base3**/**serviceId.*
Because of strong encapsulation in ServiceDiscoveryImpl class (package private method modifiers)
{code:java}
@VisibleForTesting
String pathForName(String name)
{
return ZKPaths.makePath(basePath, name);
}
@VisibleForTesting
String pathForInstance(String name, String id)
{
return ZKPaths.makePath(pathForName(name), id);
}
{code}
So, can we rethink visibility modifiers or add some extendable entity which would be responsible for constructing paths?
For example:
Create interface
{code:java}
/**
* A constructor which constructs path to services for service discovering
*/
public interface DiscoveryPathConstructor
{
/**
* Return the path where all service names registered
* @return basePath
*/
String getBasePath();
/**
* Return the path where all instances of service registered
* @param name service name
* @return path to service instances
*/
String getPathForInstances(String name);
/**
* Return the path where concrete instance registered
* @param name service name
* @param id concrete instance id
* @return path to concrete instance
*/
String getPathForInstance(String name, String id);
}
{code}
And make a constructor and field in ServiceDiscoveryImpl:
{code:java}
private final DiscoveryPathConstructor pathConstructor;
/**
* @param client the client
* @param pathConstructor constructor for instances path
* @param serializer serializer for instances (e.g. {@link JsonInstanceSerializer})
* @param thisInstance instance that represents the service that is running. The instance will get auto-registered
* @param watchInstances if true, watches for changes to locally registered instances
*/
public ServiceDiscoveryImpl(CuratorFramework client, DiscoveryPathConstructor pathConstructor, InstanceSerializer<T> serializer,
ServiceInstance<T> thisInstance, boolean watchInstances)
{
this.watchInstances = watchInstances;
this.client = Preconditions.checkNotNull(client, "client cannot be null");
this.serializer = Preconditions.checkNotNull(serializer, "serializer cannot be null");
this.pathConstructor = Preconditions.checkNotNull(pathConstructor, "pathConstructor cannot be null");
if ( thisInstance != null )
{
Entry<T> entry = new Entry<T>(thisInstance);
entry.cache = makeNodeCache(thisInstance);
services.put(thisInstance.getId(), entry);
}
}
{code}
You can view commit in my fork of framework, if that's ok - I can make a pull request.
https://github.com/Me1kaa/curator/commit/6e6a34db7d71a84e3a53d284fc94e1fe2e3c10d8
was:
Right now paths for services in service discovery hardcoded to pattern: */\{basePath} /\{serviceName}/\{instanceId}.* And that's impossible to configure paths this way: */\{basePathPart1} /\{serviceName}/{basePathPart2}/\{instanceId}**
For example:
If basePath = */base1/base2/base3*, serviceName = *serviceName*, instanceId - *serviceId*
then path will be */**base1**/**base2**/**base3**/**serviceName**/**serviceId*, that can't be configured like this: */**base1**/**base2/serviceName**/**base3**/**serviceId.*
Because of strong encapsulation in ServiceDiscoveryImpl class (package private method modifiers)
{code:java}
@VisibleForTesting
String pathForName(String name)
{
return ZKPaths.makePath(basePath, name);
}
@VisibleForTesting
String pathForInstance(String name, String id)
{
return ZKPaths.makePath(pathForName(name), id);
}
{code}
So, can we rethink visibility modifiers or add some extendable entity which would be responsible for constructing paths?
For example:
Create interface
{code:java}
/**
* A constructor which constructs path to services for service discovering
*/
public interface DiscoveryPathConstructor
{
/**
* Return the path where all service names registered
* @return basePath
*/
String getBasePath();
/**
* Return the path where all instances of service registered
* @param name service name
* @return path to service instances
*/
String getPathForInstances(String name);
/**
* Return the path where concrete instance registered
* @param name service name
* @param id concrete instance id
* @return path to concrete instance
*/
String getPathForInstance(String name, String id);
}
{code}
And make a constructor and field in ServiceDiscoveryImpl:
{code:java}
private final DiscoveryPathConstructor pathConstructor;
/**
* @param client the client
* @param pathConstructor constructor for instances path
* @param serializer serializer for instances (e.g. {@link JsonInstanceSerializer})
* @param thisInstance instance that represents the service that is running. The instance will get auto-registered
* @param watchInstances if true, watches for changes to locally registered instances
*/
public ServiceDiscoveryImpl(CuratorFramework client, DiscoveryPathConstructor pathConstructor, InstanceSerializer<T> serializer,
ServiceInstance<T> thisInstance, boolean watchInstances)
{
this.watchInstances = watchInstances;
this.client = Preconditions.checkNotNull(client, "client cannot be null");
this.serializer = Preconditions.checkNotNull(serializer, "serializer cannot be null");
this.pathConstructor = Preconditions.checkNotNull(pathConstructor, "pathConstructor cannot be null");
if ( thisInstance != null )
{
Entry<T> entry = new Entry<T>(thisInstance);
entry.cache = makeNodeCache(thisInstance);
services.put(thisInstance.getId(), entry);
}
}
{code}
You can view commit in my fork of framework, if that's ok - I can make a pull request.
https://github.com/Me1kaa/curator/commit/6e6a34db7d71a84e3a53d284fc94e1fe2e3c10d8
> Configuring service discovery paths
> -----------------------------------
>
> Key: CURATOR-457
> URL: https://issues.apache.org/jira/browse/CURATOR-457
> Project: Apache Curator
> Issue Type: Improvement
> Reporter: Aleksander Melnichnikov
> Assignee: Jordan Zimmerman
> Priority: Minor
>
> Right now paths for services in service discovery hardcoded to pattern: */\{basePath} /\{serviceName}/\{instanceId}.* And that's impossible to configure paths this way: */\{basePathPart1} /\{serviceName}/\{basePathPart2}/\{instanceId}*
> For example:
> If basePath = */base1/base2/base3*, serviceName = *serviceName*, instanceId - *serviceId*
> then path will be */**base1**/**base2**/**base3**/**serviceName**/**serviceId*, that can't be configured like this: */**base1**/**base2/serviceName**/**base3**/**serviceId.*
> Because of strong encapsulation in ServiceDiscoveryImpl class (package private method modifiers)
> {code:java}
> @VisibleForTesting
> String pathForName(String name)
> {
> return ZKPaths.makePath(basePath, name);
> }
> @VisibleForTesting
> String pathForInstance(String name, String id)
> {
> return ZKPaths.makePath(pathForName(name), id);
> }
> {code}
> So, can we rethink visibility modifiers or add some extendable entity which would be responsible for constructing paths?
> For example:
> Create interface
> {code:java}
> /**
> * A constructor which constructs path to services for service discovering
> */
> public interface DiscoveryPathConstructor
> {
> /**
> * Return the path where all service names registered
> * @return basePath
> */
> String getBasePath();
> /**
> * Return the path where all instances of service registered
> * @param name service name
> * @return path to service instances
> */
> String getPathForInstances(String name);
> /**
> * Return the path where concrete instance registered
> * @param name service name
> * @param id concrete instance id
> * @return path to concrete instance
> */
> String getPathForInstance(String name, String id);
> }
> {code}
> And make a constructor and field in ServiceDiscoveryImpl:
> {code:java}
> private final DiscoveryPathConstructor pathConstructor;
> /**
> * @param client the client
> * @param pathConstructor constructor for instances path
> * @param serializer serializer for instances (e.g. {@link JsonInstanceSerializer})
> * @param thisInstance instance that represents the service that is running. The instance will get auto-registered
> * @param watchInstances if true, watches for changes to locally registered instances
> */
> public ServiceDiscoveryImpl(CuratorFramework client, DiscoveryPathConstructor pathConstructor, InstanceSerializer<T> serializer,
> ServiceInstance<T> thisInstance, boolean watchInstances)
> {
> this.watchInstances = watchInstances;
> this.client = Preconditions.checkNotNull(client, "client cannot be null");
> this.serializer = Preconditions.checkNotNull(serializer, "serializer cannot be null");
> this.pathConstructor = Preconditions.checkNotNull(pathConstructor, "pathConstructor cannot be null");
> if ( thisInstance != null )
> {
> Entry<T> entry = new Entry<T>(thisInstance);
> entry.cache = makeNodeCache(thisInstance);
> services.put(thisInstance.getId(), entry);
> }
> }
> {code}
> You can view commit in my fork of framework, if that's ok - I can make a pull request.
> https://github.com/Me1kaa/curator/commit/6e6a34db7d71a84e3a53d284fc94e1fe2e3c10d8
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)