You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/07/16 03:11:46 UTC
[GitHub] [iotdb] wangchao316 commented on a change in pull request #3580: Fix thrift out of sequence
wangchao316 commented on a change in pull request #3580:
URL: https://github.com/apache/iotdb/pull/3580#discussion_r670933759
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/last/ClusterLastQueryExecutor.java
##########
@@ -258,19 +259,30 @@ private ByteBuffer lastAsync(Node node, QueryContext context)
}
private ByteBuffer lastSync(Node node, QueryContext context) throws TException {
- try (SyncDataClient syncDataClient =
- metaGroupMember
- .getClientProvider()
- .getSyncDataClient(node, RaftServer.getReadOperationTimeoutMS())) {
-
- return syncDataClient.last(
+ SyncDataClient client = null;
+ try {
+ client =
+ metaGroupMember
+ .getClientProvider()
+ .getSyncDataClient(node, RaftServer.getReadOperationTimeoutMS());
+ return client.last(
new LastQueryRequest(
PartialPath.toStringList(seriesPaths),
dataTypeOrdinals,
context.getQueryId(),
queryPlan.getDeviceToMeasurements(),
group.getHeader(),
- syncDataClient.getNode()));
+ client.getNode()));
+ } catch (IOException e) {
+ return null;
+ } catch (TException e) {
+ // the connection may be broken, close it to avoid it being reused
+ client.getInputProtocol().getTransport().close();
+ throw e;
+ } finally {
+ if (client != null) {
Review comment:
we should use try() --> close() --> putBack()
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/last/ClusterLastQueryExecutor.java
##########
@@ -258,19 +259,30 @@ private ByteBuffer lastAsync(Node node, QueryContext context)
}
private ByteBuffer lastSync(Node node, QueryContext context) throws TException {
- try (SyncDataClient syncDataClient =
- metaGroupMember
- .getClientProvider()
- .getSyncDataClient(node, RaftServer.getReadOperationTimeoutMS())) {
-
- return syncDataClient.last(
+ SyncDataClient client = null;
+ try {
+ client =
+ metaGroupMember
+ .getClientProvider()
+ .getSyncDataClient(node, RaftServer.getReadOperationTimeoutMS());
+ return client.last(
new LastQueryRequest(
PartialPath.toStringList(seriesPaths),
dataTypeOrdinals,
context.getQueryId(),
queryPlan.getDeviceToMeasurements(),
group.getHeader(),
- syncDataClient.getNode()));
+ client.getNode()));
+ } catch (IOException e) {
+ return null;
Review comment:
1. if have a exception, we need print a log.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/coordinator/Coordinator.java
##########
@@ -735,7 +730,7 @@ public Node getThisNode() {
* @param node the node to be connected
* @param timeout timeout threshold of connection
*/
- public SyncDataClient getSyncDataClient(Node node, int timeout) throws TException {
+ public SyncDataClient getSyncDataClient(Node node, int timeout) throws IOException {
Review comment:
Good Job.
I have a question. why will TException replace to IOException ?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -498,23 +498,22 @@ private void refreshClientOnce() {
}
private void refreshClientOnceSync(Node receiver) {
- RaftService.Client client;
+ RaftService.Client client = null;
try {
client =
getClientProvider()
.getSyncDataClientForRefresh(receiver, RaftServer.getWriteOperationTimeoutMS());
- } catch (TException e) {
- return;
- }
- try {
RefreshReuqest req = new RefreshReuqest();
client.refreshConnection(req);
+ } catch (IOException ignored) {
} catch (TException e) {
- logger.warn("encounter refreshing client timeout, throw broken connection", e);
+ logger.info("encounter refreshing client timeout, throw broken connection", e);
Review comment:
why use info replace warn? generate, exception deal bettwen warn and error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org