[IOTDB-1399]Add a session interface to connect multiple nodes #3434
Conversation
|
|
||
| private boolean reconnect() { | ||
| boolean flag = false; | ||
| for (int i = 1; i <= Config.RETRY_NUM; i++) { |
There was a problem hiding this comment.
Always try the first RETRY_NUM clients, it's not good. Maybe you can record a nextHostIndex to try next hostId.
There was a problem hiding this comment.
in new commit,Instead of trying the first node in the list every time, try the current node again, and then try the next node in the list
There was a problem hiding this comment.
I think we should connect nodes randomly, so that we can reduce the pressure on one node.
There was a problem hiding this comment.
ok, in new commit,connect nodes randomly
| this.enableCacheLeader = enableCacheLeader; | ||
| } | ||
|
|
||
| public synchronized void clusterOpen() throws IoTDBConnectionException { |
There was a problem hiding this comment.
I think it's no need to create a new open function, you could put the logic which just create SessionConnection who use the nodeUrl in the contructSessionConn.
There was a problem hiding this comment.
ok,i modify in new commit,using the old open method
| this.enableCacheLeader = enableCacheLeader; | ||
| } | ||
|
|
||
| public Session(List<String> nodeUrls, String username, String password) { |
There was a problem hiding this comment.
It's better using java doc to specify the URL format clearly
| throws StatementExecutionException, IoTDBConnectionException { | ||
| try { | ||
| logger.info("{} execute sql {}", defaultSessionConnection.getEndPoint(), sql); | ||
| logger.info("{} execute sql {}", defaultEndPoint, sql); |
There was a problem hiding this comment.
defaultSessionConnection.getEndPoint() may not equal defaultEndPoint as long as we enable redirection
| defaultSessionConnection.getEndPoint(), | ||
| sql, | ||
| e.getEndPoint()); | ||
| logger.debug("{} redirect query {} to {}", defaultEndPoint, sql, e.getEndPoint()); |
| EndPoint endPoint = parseNodeUrl(nodeUrl); | ||
| endPointsList.add(endPoint); |
There was a problem hiding this comment.
parseNodeUrl method may return null, we can not add one null endpoint to the endPointsList
| } | ||
| String ip = split[0]; | ||
| try { | ||
| int rpcPort = Integer.parseInt(split[1]); | ||
| return endPoint.setIp(ip).setPort(rpcPort); | ||
| } catch (NumberFormatException e) { | ||
| logger.warn("parse url fail {}", nodeUrl); | ||
| } | ||
| return endPoint; | ||
| } |
There was a problem hiding this comment.
If the user given one wrong URL format, in the code implement, the NumberFormatException may occur, you will return one empty endPoint.
In my opinion, if the seed URL format error, It's better rethrow the exception to the user.
| if (endPoint == null) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Its better not allow null endPoint added in the endPointList in the previous parseSeedNodeUrls method. In other words, abnormal judgment should be made as soon as possible.
| flag = true; | ||
| if (transport != null) { | ||
| transport.close(); | ||
| int currHostIndex = endPointList.indexOf(session.defaultEndPoint); |
There was a problem hiding this comment.
currHostIndex maybe -1, so you should handle it.
| } catch (InterruptedException e1) { | ||
| logger.error("reconnect is interrupted.", e1); | ||
| Thread.currentThread().interrupt(); | ||
| int tag = 0; |
| } | ||
|
|
||
| private boolean reconnect() { | ||
| boolean flag = false; |
5 participants
While the cluster is running, the coordinator node may go down, which may cause the session to be unable to read or write properly.i add a session interface to connect multiple nodes.Users can now define a list that contains multiple servers that can be connected. When one of the servers goes down, try again. If it fails, connect to the next one. If all the nodes in the list fail, an exception will be thrown.