[ https://issues.apache.org/jira/browse/DRILL-5015?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15656486#comment-15656486 ]
ASF GitHub Bot commented on DRILL-5015: --------------------------------------- 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. > As per documentation, when issuing a list of drillbits in the connection > string, we always attempt to connect only to the first one > ----------------------------------------------------------------------------------------------------------------------------------- > > Key: DRILL-5015 > URL: https://issues.apache.org/jira/browse/DRILL-5015 > Project: Apache Drill > Issue Type: Bug > Components: Client - JDBC > Affects Versions: 1.8.0, 1.9.0 > Reporter: Sorabh Hamirwasia > Assignee: Sudheesh Katkam > > When trying to connect to a Drill cluster by specifying more than 1 drillbits > to connect to, we always attempt to connect to only the first drillbit. > As an example, we tested against a pair of drillbits, but we always connect > to the first entry in the CSV list by querying for the 'current' drillbit. > The remaining entries are never attempted. > [root@pssc-60 agileSqlPerfTests]# /opt/mapr/drill/drill-1.8.0/bin/sqlline -u > "jdbc:drill:schema=dfs.tmp;drillbit=pssc-61:31010,pssc-62:31010" -f > whereAmI.q | grep -v logback > 1/1 select * from sys.drillbits where `current`; > +-----------------+------------+---------------+------------+----------+ > | hostname | user_port | control_port | data_port | current | > +-----------------+------------+---------------+------------+----------+ > | pssc-61.qa.lab | 31010 | 31011 | 31012 | true | > +-----------------+------------+---------------+------------+----------+ > 1 row selected (0.265 seconds) > Closing: org.apache.drill.jdbc.impl.DrillConnectionImpl > apache drill 1.8.0 > "a little sql for your nosql" > This property is meant for use by clients when not wanting to overload the ZK > for fetching a list of existing Drillbits, but the behaviour doesn't match > the documentation. > [Making a Direct Drillbit Connection | > https://drill.apache.org/docs/using-the-jdbc-driver/#using-the-jdbc-url-format-for-a-direct-drillbit-connection > ] > We need to randomly shuffle between this list and If an entry in the shuffled > list is unreachable, we need to try for the next entry in the list. -- This message was sent by Atlassian JIRA (v6.3.4#6332)