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 2022/04/25 03:54:52 UTC
[GitHub] [iotdb] cornmonster opened a new pull request, #5659: Optimize source handle error handling
cornmonster opened a new pull request, #5659:
URL: https://github.com/apache/iotdb/pull/5659
Add `SourceHandleListener#onFailure`. The method will be called when fatal exceptions happen during data transmission.
--
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
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #5659: Optimize source handle error handling
Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #5659:
URL: https://github.com/apache/iotdb/pull/5659#discussion_r857321047
##########
server/src/main/java/org/apache/iotdb/db/mpp/execution/QueryExecution.java:
##########
@@ -210,8 +208,7 @@ public TsBlock getBatchResult() {
return null;
}
return resultHandle.receive();
-
- } catch (ExecutionException | IOException | CancellationException e) {
+ } catch (ExecutionException e) {
Review Comment:
We can remove IOException but we need keep the `CancellationException` here. Because the blocked.get() may throw the CancellationException which is necessary to be caught here.
--
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
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #5659: Optimize source handle error handling
Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #5659:
URL: https://github.com/apache/iotdb/pull/5659#discussion_r857312418
##########
server/src/main/java/org/apache/iotdb/db/mpp/buffer/DataBlockManager.java:
##########
@@ -290,7 +312,7 @@ public ISinkHandle createSinkHandle(
executorService,
clientFactory.getDataBlockServiceClient(remoteEndpoint),
tsBlockSerdeFactory.get(),
- new SinkHandleListenerImpl(instanceContext));
+ new SinkHandleListenerImpl(instanceContext, instanceContext::failed));
Review Comment:
Now that we use `failureCallback`, we can decouple `FragmentInstanceContext` with this method. I mean, we can change the parameter from `FragmentInstanceContext` to `FailureCallback`
--
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
[GitHub] [iotdb] xingtanzjr merged pull request #5659: Optimize source handle error handling
Posted by GitBox <gi...@apache.org>.
xingtanzjr merged PR #5659:
URL: https://github.com/apache/iotdb/pull/5659
--
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
[GitHub] [iotdb] cornmonster commented on a diff in pull request #5659: Optimize source handle error handling
Posted by GitBox <gi...@apache.org>.
cornmonster commented on code in PR #5659:
URL: https://github.com/apache/iotdb/pull/5659#discussion_r857337227
##########
server/src/main/java/org/apache/iotdb/db/mpp/execution/QueryExecution.java:
##########
@@ -210,8 +208,7 @@ public TsBlock getBatchResult() {
return null;
}
return resultHandle.receive();
-
- } catch (ExecutionException | IOException | CancellationException e) {
+ } catch (ExecutionException e) {
Review Comment:
OK, will fix it.
--
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
[GitHub] [iotdb] cornmonster commented on a diff in pull request #5659: Optimize source handle error handling
Posted by GitBox <gi...@apache.org>.
cornmonster commented on code in PR #5659:
URL: https://github.com/apache/iotdb/pull/5659#discussion_r857336430
##########
server/src/main/java/org/apache/iotdb/db/mpp/buffer/DataBlockManager.java:
##########
@@ -290,7 +312,7 @@ public ISinkHandle createSinkHandle(
executorService,
clientFactory.getDataBlockServiceClient(remoteEndpoint),
tsBlockSerdeFactory.get(),
- new SinkHandleListenerImpl(instanceContext));
+ new SinkHandleListenerImpl(instanceContext, instanceContext::failed));
Review Comment:
You are right.
But to achieve that, we have to introduce a few more callbacks like onFinishedCallback and onClosedCallback. Do you think it is necessary to do it or leaving a TODO is good for now?
--
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
[GitHub] [iotdb] xingtanzjr commented on a diff in pull request #5659: Optimize source handle error handling
Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #5659:
URL: https://github.com/apache/iotdb/pull/5659#discussion_r857341419
##########
server/src/main/java/org/apache/iotdb/db/mpp/buffer/DataBlockManager.java:
##########
@@ -290,7 +312,7 @@ public ISinkHandle createSinkHandle(
executorService,
clientFactory.getDataBlockServiceClient(remoteEndpoint),
tsBlockSerdeFactory.get(),
- new SinkHandleListenerImpl(instanceContext));
+ new SinkHandleListenerImpl(instanceContext, instanceContext::failed));
Review Comment:
Sure we can leave a TODO now
--
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