LoomKafkaConsumer::close let closing task finish before interrupting #3957
Conversation
knative-prow
bot
added
the
size/S
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3957 +/- ##
=======================================
Coverage 48.24% 48.24%
=======================================
Files 246 246
Lines 14500 14500
=======================================
Hits 6995 6995
Misses 6799 6799
Partials 706 706 ☔ View full report in Codecov by Sentry. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: maschmid, pierDipi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow
bot
added
the
approved
|
/test reconciler-tests-keda |
| logger.debug("Queue is empty"); | ||
|
|
||
| if (!isFinished.get()) { | ||
| logger.debug("Background thread not finished yet, waiting for it to complete"); | ||
| Thread.sleep(2000L); | ||
|
|
||
| if (!isFinished.get()) { | ||
| logger.debug("Background thread still not finished yet, interrupting background thread"); | ||
| taskRunnerThread.interrupt(); | ||
| } | ||
| } | ||
|
|
||
| taskRunnerThread.interrupt(); |
There was a problem hiding this comment.
We could also consider using "isFinished" in the while above?
There was a problem hiding this comment.
Right now I wanted to preserve the logic of "wait forever for all the tasks except the last one and then give 2s for the last task".
If we are OK to never interrupt, we could just wait until !isFinished.get()
|
@maschmid we might have the same issue here |
Seeing quite a lot of InterruptedExceptions when deleting triggers,
The LoomKafkaConsumer::close does not wait for the close task to finish before interrupting (it only waits for the queue to be empty, but it is empty while it's processing the last task)
Proposed Changes