DBZ-6213 Adjust connector restart #4705
Conversation
debezium-core/src/main/java/io/debezium/connector/common/BaseSourceTask.java
Outdated
Show resolved
Hide resolved
|
Hi @vjuranek, thanks for your contribution. Please prefix the commit message(s) with the DBZ-xxx JIRA issue key. |
debezium-core/src/main/java/io/debezium/connector/common/BaseSourceTask.java
Outdated
Show resolved
Hide resolved
| @@ -312,7 +301,6 @@ private void stop(boolean restart) { | |||
| setTaskState(State.RESTARTING); | |||
| if (restartDelay == null) { | |||
| restartDelay = ElapsedTimeStrategy.constant(Clock.system(), retriableRestartWait.toMillis()); | |||
| restartDelay.hasElapsed(); | |||
There was a problem hiding this comment.
IMHO by removing it you ae removing the pause.
There was a problem hiding this comment.
Right, but only the first one pause:-) This method says, if the delay interval has already elapsed or not. As a side effect it recomputes next timestamp for waiting. This method is very confusing (it's description, side effects and initialization) and IMHO should be also refactored.
However, you are right that removing it is wrong - due to its side effect if this call is removed, the elapsed interval won't be initialized and actually won't do any wait when the strategy is used for the first time. So basically this has to be called every time right after ElapsedTimeStrategy is created.
There was a problem hiding this comment.
@vjuranek I agree on the refactoring
debezium-core/src/main/java/io/debezium/connector/common/BaseSourceTask.java
Outdated
Show resolved
Hide resolved
debezium-core/src/main/java/io/debezium/connector/common/BaseSourceTask.java
Outdated
Show resolved
Hide resolved
debezium-core/src/main/java/io/debezium/connector/common/BaseSourceTask.java
Outdated
Show resolved
Hide resolved
| @@ -312,7 +301,6 @@ private void stop(boolean restart) { | |||
| setTaskState(State.RESTARTING); | |||
| if (restartDelay == null) { | |||
| restartDelay = ElapsedTimeStrategy.constant(Clock.system(), retriableRestartWait.toMillis()); | |||
| restartDelay.hasElapsed(); | |||
There was a problem hiding this comment.
Right, but only the first one pause:-) This method says, if the delay interval has already elapsed or not. As a side effect it recomputes next timestamp for waiting. This method is very confusing (it's description, side effects and initialization) and IMHO should be also refactored.
However, you are right that removing it is wrong - due to its side effect if this call is removed, the elapsed interval won't be initialized and actually won't do any wait when the strategy is used for the first time. So basically this has to be called every time right after ElapsedTimeStrategy is created.
* Try to start the connector in BaseSourceTask::start(). This is the root cause of DBZ-6213 as EmbeddedEngine calls start() and assumes it really tries to start the connector. * Simplify the code by removing unnecessary checks.
|
@vjuranek Applied, thanks a lot |
https://issues.redhat.com/browse/DBZ-6213