akalash commented on a change in pull request #17663:
URL: https://github.com/apache/flink/pull/17663#discussion_r743613444
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/LocalInputChannel.java
##########
@@ -338,10 +338,13 @@ void releaseAllResources() throws IOException {
@Override
void announceBufferSize(int newBufferSize) {
- checkState(!isReleased, "Channel released.");
-
- ResultSubpartitionView subpartitionView =
checkNotNull(this.subpartitionView);
- subpartitionView.notifyNewBufferSize(newBufferSize);
+ ResultSubpartitionView view = this.subpartitionView;
+ // if releaseAllResources would be called from the mailbox thread it
is possible that
Review comment:
> Please confirm that this may happen only when cancelling or failing a
task externally,
I confirm that in my opinion, it is the only place where it can happen.
In general, I agree that it is the same situation as with other similar
methods (like `RemoteInputChannel#resumeConsumption`) and I agree that it
should not matter much because it means that job is already failing. So
perhaps, my last changes should work fine.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/LocalInputChannel.java
##########
@@ -338,10 +338,13 @@ void releaseAllResources() throws IOException {
@Override
void announceBufferSize(int newBufferSize) {
- checkState(!isReleased, "Channel released.");
-
- ResultSubpartitionView subpartitionView =
checkNotNull(this.subpartitionView);
- subpartitionView.notifyNewBufferSize(newBufferSize);
+ ResultSubpartitionView view = this.subpartitionView;
+ // if releaseAllResources would be called from the mailbox thread it
is possible that
Review comment:
> Please confirm that this may happen only when cancelling or failing a
task externally,
I confirm that in my opinion, it is the only place where it can happen.
In general, I agree that it is the same situation as with other similar
methods (like `RemoteInputChannel#resumeConsumption`) and I agree that it
should not matter much because it means that job is already failing. So
perhaps, my last changes should work fine.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
