drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sohami <...@git.apache.org>
Subject [GitHub] drill pull request #648: DRILL-5015: Randomly select the drillbit from the l...
Date Fri, 11 Nov 2016 08:19:21 GMT
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/648#discussion_r87551806
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
    @@ -223,19 +224,94 @@ public void connect(Properties props) throws RpcException {
         connect(null, props);
       }
     
    +  /**
    +   * Function to Populate the endpointList with the list of drillbits
    +   * provided in the connection string by client.
    +   *
    +   * For direct connection we can get URL string having drillbit property as below:
    +   * drillbit=<ip>:<port> --- use the ip and port specified as the Foreman
ip and port
    +   * drillbit=<ip>        --- use the ip specified as the Foreman ip with default
port in config file
    +   * drillbit=<ip1>:<port1>,<ip2>:<port2>... --- Randomly select
the ip and port pair from the specified list as the
    +   *                                             Foreman ip and port.
    +   *
    +   * @param drillbits string with drillbit value provided in connection string
    +   * @param defaultUserPort string with default userport of drillbit specified in config
file
    +   * @return list of drillbitendpoints parsed from connection string
    +   * @throws InvalidConnectionInfoException if the connection string has invalid or no
drillbit information
    +   */
    +  static List<DrillbitEndpoint> parseAndVerifyEndpoints(String drillbits, String
defaultUserPort)
    +                                throws InvalidConnectionInfoException {
    +    // If no drillbits is provided then just return empty list.
    +    if (drillbits.trim().isEmpty()) {
    +      throw new InvalidConnectionInfoException("No drillbit information specified in
the connection string");
    +    }
    +
    +    ArrayList<DrillbitEndpoint> endpointList = new ArrayList<>();
    +    final String[] connectInfo = drillbits.split(",");
    +
    +    // Fetch ip address and port information for each drillbit and populate the list
    +    for (String drillbit : connectInfo) {
    +
    +      // Trim all the empty spaces and check if the entry is empty string.
    +      // Ignore the empty ones.
    +      drillbit = drillbit.trim();
    +
    +      if (!drillbit.isEmpty()) {
    +        // Verify if we have only ":" or only ":port" pattern
    +        if (drillbit.charAt(0) == ':') {
    +          // Invalid drillbit information
    +          throw new InvalidConnectionInfoException("Malformed connection string with
drillbit hostname or " +
    +                                                     "hostaddress missing for an entry:
" + drillbit);
    +        }
    +
    +        // We are now sure that each ip:port entry will have some both the entries.
    +        // Split each drillbit connection string to get ip address and port value
    +        final String[] drillbitInfo = drillbit.split(":");
    +
    +        // Check if we have more than one port
    +        if (drillbitInfo.length > 2) {
    +          throw new InvalidConnectionInfoException("Malformed connection string with
more than one port in a " +
    +                                                     "drillbit entry: " + drillbit);
    +        }
    +
    +        // At this point we are sure that drillbitInfo has atleast hostname or host address
    +        // trim all the empty spaces which might be present in front of hostname or
    +        // host address information
    +        final String ipAddress = drillbitInfo[0].trim();
    +        String port = defaultUserPort;
    +
    +        if (drillbitInfo.length == 2) {
    +          // We have a port value also given by user. trim all the empty spaces between
: and port value before
    +          // validating the correctness of value.
    +          port = drillbitInfo[1].trim();
    +        }
    +
    +        try {
    +          final DrillbitEndpoint endpoint = DrillbitEndpoint.newBuilder()
    +                                            .setAddress(ipAddress)
    +                                            .setUserPort(Integer.parseInt(port))
    +                                            .build();
    +
    +          endpointList.add(endpoint);
    +        } catch (NumberFormatException e) {
    +          throw new InvalidConnectionInfoException("Malformed port value in entry: "
+ ipAddress + ":" + port + " " +
    +                                                     "passed in connection string");
    +        }
    +      }
    +    }
    +    return endpointList;
    --- End diff --
    
    Nice Catch! Thanks! I forgot to trim the drillbits string and assign to itself, rather
I just checked in if condition. I have made the change. Still when the string contains only
"," it can lead to empty endpoint list. Added a check for that along with a test case.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message