2011aad commented on a change in pull request #18822:
URL: https://github.com/apache/flink/pull/18822#discussion_r813544404
##########
File path:
flink-connectors/flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/internal/JdbcOutputFormat.java
##########
@@ -154,7 +154,9 @@ public void open(int taskNumber, int numTasks) throws
IOException {
try {
flush();
} catch (Exception e) {
- flushException = e;
+ if (flushException == null) {
Review comment:
@tsreaper I agree with you.
You are discussing about whether multiple exceptions should be stored to
show. We can consider two scenerios.
First, the exceptions are triggered by the same one row. In this case, the
exceptions are almost the same one (except the extreme case like a temporary
connection exception followed by a sql exception from the database, considering
it occurs rarely, I think it's ok to just store the first one to trigger a job
restart, and after restart, user will see the next exception if it still
there), so only one exception need to be stored.
Second, the exceptions are triggered by different rows. Consider a case that
row1 causes exception A followed by row2 causes exception B. In such case, if
exception B do not occur, row2 will be processed and emit to the downstream. So
I think if row1 triggers an exception, no rows after row1 should be processed
and therefore no further exceptions should be triggered.
So, I think we do not need to catch multiple exceptions to show to the user.
--
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]
