dawidwys commented on a change in pull request #13423:
URL: https://github.com/apache/flink/pull/13423#discussion_r492618028
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/AbstractInput.java
##########
@@ -68,4 +68,13 @@ public void processLatencyMarker(LatencyMarker
latencyMarker) throws Exception {
public void setKeyContextElement(StreamRecord record) throws Exception {
owner.internalSetKeyContextElement(record, stateKeySelector);
}
+
+ @Override
+ public void endInput() throws Exception {
+ if (owner instanceof BoundedOneInput && inputId == 1) {
Review comment:
It shouldn't. Nevertheless I think it is not safe to just assume so.
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/StreamSource.java
##########
@@ -107,7 +106,11 @@ public void run(final Object lockingObject,
// in theory, the subclasses of StreamSource
may implement the BoundedOneInput interface,
// so we still need the following call to end
the input
synchronized (lockingObject) {
- operatorChain.endMainOperatorInput(1);
+ if (this instanceof BoundedOneInput) {
+ ((BoundedOneInput)
this).endInput();
+ } else if (this instanceof
BoundedMultiInput) {
+ ((BoundedMultiInput)
this).endInput(1);
Review comment:
Not sure if I understand the comment.
Does a StreamSource even have inputs? Of course I might be missing something
here, but isn't the input id, the id of an input that has finished? Honestly, I
find this part of code quite shady, but I think the logic pre and post this
change is the same. Again I might be wrong.
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/StreamSource.java
##########
@@ -107,7 +106,11 @@ public void run(final Object lockingObject,
// in theory, the subclasses of StreamSource
may implement the BoundedOneInput interface,
// so we still need the following call to end
the input
synchronized (lockingObject) {
- operatorChain.endMainOperatorInput(1);
+ if (this instanceof BoundedOneInput) {
+ ((BoundedOneInput)
this).endInput();
+ } else if (this instanceof
BoundedMultiInput) {
+ ((BoundedMultiInput)
this).endInput(1);
Review comment:
Sorry, I am not sure if I understand the comment.
Does a StreamSource even have inputs? Of course I might be missing something
here, but isn't the input id, the id of an input that has finished? Honestly, I
find this part of code quite shady, but I think the logic pre and post this
change is the same. Again I might be wrong.
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/StreamSource.java
##########
@@ -107,7 +106,11 @@ public void run(final Object lockingObject,
// in theory, the subclasses of StreamSource
may implement the BoundedOneInput interface,
// so we still need the following call to end
the input
synchronized (lockingObject) {
- operatorChain.endMainOperatorInput(1);
+ if (this instanceof BoundedOneInput) {
+ ((BoundedOneInput)
this).endInput();
+ } else if (this instanceof
BoundedMultiInput) {
+ ((BoundedMultiInput)
this).endInput(1);
Review comment:
Sorry, I am not sure if I understand the comment.
Does a StreamSource even have inputs? Of course I might be missing something
here, but isn't the input id, the id of an input that has finished? Honestly, I
find this part of code quite shady (as there are actually no inputs that can
finish, imo), but I think the logic pre and post this change is the same. Again
I might be wrong.
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/StreamSource.java
##########
@@ -107,7 +106,11 @@ public void run(final Object lockingObject,
// in theory, the subclasses of StreamSource
may implement the BoundedOneInput interface,
// so we still need the following call to end
the input
synchronized (lockingObject) {
- operatorChain.endMainOperatorInput(1);
+ if (this instanceof BoundedOneInput) {
+ ((BoundedOneInput)
this).endInput();
+ } else if (this instanceof
BoundedMultiInput) {
+ ((BoundedMultiInput)
this).endInput(1);
Review comment:
Sorry, I am not sure if I understand the comment.
Does a StreamSource even have inputs? Of course I might be missing something
here, but isn't the input id, the id of an input that has finished? Honestly, I
find this part of orginal code quite shady (as there are actually no inputs
that can finish, imo), but I think the logic pre and post this change is the
same. Again I might be wrong.
----------------------------------------------------------------
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]
