You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/10/23 15:02:13 UTC
[GitHub] [flink] kezhuw opened a new pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
kezhuw opened a new pull request #13776:
URL: https://github.com/apache/flink/pull/13776
## What is the purpose of the change
Fix spurious `InputStatus.END_OF_INPUT` from `SourceReaderBase.pollNext` caused by split reader exception.
## Brief change log
* Modify `SourceReaderBaseTest.testExceptionInSplitReader` to assert that `SourceReaderBase.pollNext` will not return `InputStatus.END_OF_INPUT` in case of split reader exception.
* Move `exceptionHandler` from `ThrowableCatchingRunnable` to `SplitFetcher.errorHandler`.
* Construct happens-before relation between `SplitFetcher.errorHandler` and `SplitFetcher.shutdownHook`.
* Check `SplitFetcherManager.uncaughtFetcherException` before returning `InputStatus.END_OF_INPUT`.
## Verifying this change
This change modifies tests and can be verified as follows:
- `SourceReaderBaseTest.testExceptionInSplitReader`
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: (no)
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (no)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715417467
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "100b67ca171766bec1ff561a66688471b24745be",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206",
"triggerID" : "100b67ca171766bec1ff561a66688471b24745be",
"triggerType" : "PUSH"
}, {
"hash" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9315",
"triggerID" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"triggerType" : "PUSH"
}, {
"hash" : "89580c411718035563019a64bbf178dcb6b739e8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "89580c411718035563019a64bbf178dcb6b739e8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 34137c2a23fa01e836a8e990c7094ab7359b37fd Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9315)
* 89580c411718035563019a64bbf178dcb6b739e8 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715417467
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "100b67ca171766bec1ff561a66688471b24745be",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206",
"triggerID" : "100b67ca171766bec1ff561a66688471b24745be",
"triggerType" : "PUSH"
}, {
"hash" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9315",
"triggerID" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"triggerType" : "PUSH"
}, {
"hash" : "89580c411718035563019a64bbf178dcb6b739e8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "89580c411718035563019a64bbf178dcb6b739e8",
"triggerType" : "PUSH"
}, {
"hash" : "c138c988f40fa5f328f5560750a5ccab744ce279",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9317",
"triggerID" : "c138c988f40fa5f328f5560750a5ccab744ce279",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 34137c2a23fa01e836a8e990c7094ab7359b37fd Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9315)
* 89580c411718035563019a64bbf178dcb6b739e8 UNKNOWN
* c138c988f40fa5f328f5560750a5ccab744ce279 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9317)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kezhuw commented on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
kezhuw commented on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-716670452
@flinkbot attention @StephanEwen
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kezhuw commented on a change in pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
kezhuw commented on a change in pull request #13776:
URL: https://github.com/apache/flink/pull/13776#discussion_r519434803
##########
File path: flink-connectors/flink-connector-base/src/main/java/org/apache/flink/connector/base/source/reader/fetcher/SplitFetcherManager.java
##########
@@ -112,7 +111,7 @@ public void accept(Throwable t) {
public abstract void addSplits(List<SplitT> splitsToAdd);
protected void startFetcher(SplitFetcher<E, SplitT> fetcher) {
- executors.submit(new ThrowableCatchingRunnable(errorHandler, fetcher));
+ executors.submit(fetcher);
Review comment:
We need error-setting(`errorHandler.accept(t)`) happens-before fetcher-removing(`shutdownHook.run()`) so that newly added error-checking wouldn't lost it. In order to achieve this, we need move `errorHandler` to `SplitFetcher` where `shutdownHook` locates in. After movement, `ThrowableCatchingRunnable` is useless.
Besides above semantic changes, I think it would be good to drop extra layer between two concurrent components `SourceReaderBase` and `SplitFetcher`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] becketqin commented on a change in pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
becketqin commented on a change in pull request #13776:
URL: https://github.com/apache/flink/pull/13776#discussion_r519524957
##########
File path: flink-connectors/flink-connector-base/src/main/java/org/apache/flink/connector/base/source/reader/fetcher/SplitFetcherManager.java
##########
@@ -112,7 +111,7 @@ public void accept(Throwable t) {
public abstract void addSplits(List<SplitT> splitsToAdd);
protected void startFetcher(SplitFetcher<E, SplitT> fetcher) {
- executors.submit(new ThrowableCatchingRunnable(errorHandler, fetcher));
+ executors.submit(fetcher);
Review comment:
@kezhuw Thanks for the explanation. Good point.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kezhuw commented on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
kezhuw commented on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-723619395
@becketqin Sorry for ping you. Seems that https://github.com/apache/flink/pull/13574 does not compile on https://github.com/apache/flink/pull/13955 due to missing `MockSplitEnumeratorContext.runInCoordinatorThread`. I have add an `UnsupportedOperationException` version for this pr, please ignore it in reviewing.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] becketqin commented on a change in pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
becketqin commented on a change in pull request #13776:
URL: https://github.com/apache/flink/pull/13776#discussion_r519419886
##########
File path: flink-connectors/flink-connector-base/src/main/java/org/apache/flink/connector/base/source/reader/fetcher/SplitFetcherManager.java
##########
@@ -112,7 +111,7 @@ public void accept(Throwable t) {
public abstract void addSplits(List<SplitT> splitsToAdd);
protected void startFetcher(SplitFetcher<E, SplitT> fetcher) {
- executors.submit(new ThrowableCatchingRunnable(errorHandler, fetcher));
+ executors.submit(fetcher);
Review comment:
I am curious. Is there a reason that we replace the `ThrowableCatchingRunnable` wrapper with the explicit error handler passed to the split fetcher 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715417467
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "100b67ca171766bec1ff561a66688471b24745be",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206",
"triggerID" : "100b67ca171766bec1ff561a66688471b24745be",
"triggerType" : "PUSH"
}, {
"hash" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9315",
"triggerID" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"triggerType" : "PUSH"
}, {
"hash" : "89580c411718035563019a64bbf178dcb6b739e8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "89580c411718035563019a64bbf178dcb6b739e8",
"triggerType" : "PUSH"
}, {
"hash" : "c138c988f40fa5f328f5560750a5ccab744ce279",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9317",
"triggerID" : "c138c988f40fa5f328f5560750a5ccab744ce279",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 89580c411718035563019a64bbf178dcb6b739e8 UNKNOWN
* c138c988f40fa5f328f5560750a5ccab744ce279 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9317)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715417467
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "100b67ca171766bec1ff561a66688471b24745be",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206",
"triggerID" : "100b67ca171766bec1ff561a66688471b24745be",
"triggerType" : "PUSH"
}, {
"hash" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9315",
"triggerID" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 34137c2a23fa01e836a8e990c7094ab7359b37fd Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9315)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] StephanEwen commented on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
StephanEwen commented on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-723593388
+1 to merge this from my side.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715417467
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "100b67ca171766bec1ff561a66688471b24745be",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206",
"triggerID" : "100b67ca171766bec1ff561a66688471b24745be",
"triggerType" : "PUSH"
}, {
"hash" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 100b67ca171766bec1ff561a66688471b24745be Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206)
* 34137c2a23fa01e836a8e990c7094ab7359b37fd UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715417467
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "100b67ca171766bec1ff561a66688471b24745be",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206",
"triggerID" : "100b67ca171766bec1ff561a66688471b24745be",
"triggerType" : "PUSH"
}, {
"hash" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9315",
"triggerID" : "34137c2a23fa01e836a8e990c7094ab7359b37fd",
"triggerType" : "PUSH"
}, {
"hash" : "89580c411718035563019a64bbf178dcb6b739e8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "89580c411718035563019a64bbf178dcb6b739e8",
"triggerType" : "PUSH"
}, {
"hash" : "c138c988f40fa5f328f5560750a5ccab744ce279",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "c138c988f40fa5f328f5560750a5ccab744ce279",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 34137c2a23fa01e836a8e990c7094ab7359b37fd Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=9315)
* 89580c411718035563019a64bbf178dcb6b739e8 UNKNOWN
* c138c988f40fa5f328f5560750a5ccab744ce279 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] StephanEwen commented on a change in pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
StephanEwen commented on a change in pull request #13776:
URL: https://github.com/apache/flink/pull/13776#discussion_r519436157
##########
File path: flink-connectors/flink-connector-base/src/main/java/org/apache/flink/connector/base/source/reader/fetcher/SplitFetcherManager.java
##########
@@ -112,7 +111,7 @@ public void accept(Throwable t) {
public abstract void addSplits(List<SplitT> splitsToAdd);
protected void startFetcher(SplitFetcher<E, SplitT> fetcher) {
- executors.submit(new ThrowableCatchingRunnable(errorHandler, fetcher));
+ executors.submit(fetcher);
Review comment:
But I think @kezhuw has a point - having this in the same class (rather than in a separat class `ThrowableCatchingRunnable`) is a good if there is a strict contract between the exception handling and closing. Otherwise the exception handling is factored out into a separate class (`ThrowableCatchingRunnable`) but with an implicit contract on how it must be used, to guarantee the happens-before relationship.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kezhuw commented on a change in pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
kezhuw commented on a change in pull request #13776:
URL: https://github.com/apache/flink/pull/13776#discussion_r519442839
##########
File path: flink-connectors/flink-connector-base/src/main/java/org/apache/flink/connector/base/source/reader/fetcher/SplitFetcherManager.java
##########
@@ -112,7 +111,7 @@ public void accept(Throwable t) {
public abstract void addSplits(List<SplitT> splitsToAdd);
protected void startFetcher(SplitFetcher<E, SplitT> fetcher) {
- executors.submit(new ThrowableCatchingRunnable(errorHandler, fetcher));
+ executors.submit(fetcher);
Review comment:
@becketqin I see new code in `SplitFetcher.run` throwing exception after `shutdownHook.run`. Is there any specific reason where run `splitReader.close` after shutdownHook ? Or, in another word, does exception from `splitReader.close` should fail job ? I will do rebase to solve conflict, please take a took after rebasing.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] StephanEwen commented on a change in pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
StephanEwen commented on a change in pull request #13776:
URL: https://github.com/apache/flink/pull/13776#discussion_r519434849
##########
File path: flink-connectors/flink-connector-base/src/main/java/org/apache/flink/connector/base/source/reader/fetcher/SplitFetcherManager.java
##########
@@ -112,7 +111,7 @@ public void accept(Throwable t) {
public abstract void addSplits(List<SplitT> splitsToAdd);
protected void startFetcher(SplitFetcher<E, SplitT> fetcher) {
- executors.submit(new ThrowableCatchingRunnable(errorHandler, fetcher));
+ executors.submit(fetcher);
Review comment:
I personally like the change - less wrapping tends to make code navigation easier. But this is a matter of taste here, either way looks totally 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715417467
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "100b67ca171766bec1ff561a66688471b24745be",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206",
"triggerID" : "100b67ca171766bec1ff561a66688471b24745be",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 100b67ca171766bec1ff561a66688471b24745be Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715399198
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 100b67ca171766bec1ff561a66688471b24745be (Fri Oct 23 15:05:38 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
* **This pull request references an unassigned [Jira ticket](https://issues.apache.org/jira/browse/FLINK-19717).** According to the [code contribution guide](https://flink.apache.org/contributing/contribute-code.html), tickets need to be assigned before starting with the implementation work.
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] StephanEwen commented on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
StephanEwen commented on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-720949824
@kezhuw I definitely have this on my list for this week.
It got a bit delayed to to general feature freeze tightness.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kezhuw commented on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
kezhuw commented on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-717968401
@StephanEwen @becketqin Anyone has time capacity to review this pr ? @dianfu @rmetzger may want this issue solved before first RC of 1.12.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] becketqin commented on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
becketqin commented on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-723723065
Merged to master:
2cce1aced0d6a311ff0803b773f1565e7f9d76fc
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715417467
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "100b67ca171766bec1ff561a66688471b24745be",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "100b67ca171766bec1ff561a66688471b24745be",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 100b67ca171766bec1ff561a66688471b24745be UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13776:
URL: https://github.com/apache/flink/pull/13776#issuecomment-715417467
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "100b67ca171766bec1ff561a66688471b24745be",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206",
"triggerID" : "100b67ca171766bec1ff561a66688471b24745be",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 100b67ca171766bec1ff561a66688471b24745be Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8206)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] becketqin merged pull request #13776: [FLINK-19717][connectors/common] Fix spurious InputStatus.END_OF_INPUT from SourceReaderBase.pollNext caused by split reader exception
Posted by GitBox <gi...@apache.org>.
becketqin merged pull request #13776:
URL: https://github.com/apache/flink/pull/13776
----------------------------------------------------------------
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:
users@infra.apache.org