geode-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (GEODE-3406) Enable new flow for protocol on locators
Date Thu, 24 Aug 2017 18:11:00 GMT

    [ https://issues.apache.org/jira/browse/GEODE-3406?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16140434#comment-16140434
] 

ASF GitHub Bot commented on GEODE-3406:
---------------------------------------

Github user pivotal-amurmann commented on a diff in the pull request:

    https://github.com/apache/geode/pull/716#discussion_r135089666
  
    --- Diff: geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java
---
    @@ -50,51 +37,23 @@
       @Override
       public Result<ServerAPI.GetAvailableServersResponse> process(
           SerializationService serializationService, ServerAPI.GetAvailableServersRequest
request,
    -      Cache cache) {
    -
    -    InternalDistributedSystem distributedSystem =
    -        (InternalDistributedSystem) cache.getDistributedSystem();
    -    Properties properties = distributedSystem.getProperties();
    -    String locatorsString = properties.getProperty(ConfigurationProperties.LOCATORS);
    +      MessageExecutionContext executionContext) throws InvalidExecutionContextException
{
     
    -    HashSet<DistributionLocatorId> locators = new HashSet();
    -    StringTokenizer stringTokenizer = new StringTokenizer(locatorsString, ",");
    -    while (stringTokenizer.hasMoreTokens()) {
    -      String locator = stringTokenizer.nextToken();
    -      if (StringUtils.isNotEmpty(locator)) {
    -        locators.add(new DistributionLocatorId(locator));
    -      }
    +    InternalLocator locator = executionContext.getLocator();
    +    ArrayList serversFromSnapshot =
    --- End diff --
    
    I very much agree with Galen that this should be refactored. This is a big demeter violation
which is pointing at some worse code in ServerLocator which currently know how to answered
requests and how to get the information to answer them. If that was split out into one class
that can talk whatever protocol it's talking and another class that can get information from
the locator this could get cleaned up quite a bit and also make unit tests much easier. Since
we are talking about switching to Netty that might be wasted effort at this time. On the other
hand we should extract the business logic from the transport logic anyways when moving to
Netty and doing this beforehand might make that move easier.


> Enable new flow for protocol on locators
> ----------------------------------------
>
>                 Key: GEODE-3406
>                 URL: https://issues.apache.org/jira/browse/GEODE-3406
>             Project: Geode
>          Issue Type: Sub-task
>          Components: client/server
>            Reporter: Brian Baynes
>            Assignee: Hitesh Khamesra
>
> Enable magic byte and new flow for new protocol on locators.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message