pnowojski commented on a change in pull request #12706:
URL: https://github.com/apache/flink/pull/12706#discussion_r442262026
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/RemoteInputChannel.java
##########
@@ -169,9 +169,8 @@ void retriggerSubpartitionRequest(int subpartitionIndex)
throws IOException {
@Override
Optional<BufferAndAvailability> getNextBuffer() throws IOException {
- checkState(partitionRequestClient != null, "Queried for a
buffer before requesting a queue.");
-
checkError();
+ checkState(partitionRequestClient != null, "Queried for a
buffer before requesting a queue.");
Review comment:
I think it's mostly fine. If you want, the message could be updated to:
> Queried for a buffer before requesting a subpartition
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/RemoteInputChannel.java
##########
@@ -169,9 +169,8 @@ void retriggerSubpartitionRequest(int subpartitionIndex)
throws IOException {
@Override
Optional<BufferAndAvailability> getNextBuffer() throws IOException {
- checkState(partitionRequestClient != null, "Queried for a
buffer before requesting a queue.");
-
checkError();
+ checkState(partitionRequestClient != null, "Queried for a
buffer before requesting a queue.");
Review comment:
Have you thought about other places that we are doing
```
checkState(partitionRequestClient != null, ...);
```
? Frankly at the first glance it looks like everywhere we should be doing
`checkError` before. If that's correct, maybe we should introduce some helper
method
```
checkPartitionRequestQueueInitialized() {
checkError();
checkState(partitionRequestClient != null, ...);
}
```
?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
