-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LoomKafkaProducer|Consumer let the background thread finish itself #4013
LoomKafkaProducer|Consumer let the background thread finish itself #4013
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4013 +/- ##
=======================================
Coverage 48.25% 48.25%
=======================================
Files 246 246
Lines 14540 14540
=======================================
Hits 7016 7016
Misses 6817 6817
Partials 707 707 ☔ View full report in Codecov by Sentry. |
actually, we still need to interrupt the |
e360777
to
7eb9c2a
Compare
/test channel-integration-tests-ssl TestEventTransformationForSubscription looks like same error as in yesterdays nightly https://prow.knative.dev/view/gs/knative-prow/logs/nightly_eventing-kafka-broker_main_periodic/1817493539722891264 , assuming flake |
/test upgrade-tests |
/test reconciler-tests-namespaced-broker |
/test reconciler-tests-namespaced-broker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[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 |
Follows up to #3957 , but for LoomKafkaProducer, and unifies the approach
Instead of interrupting the
BlockingQueue::take
, we just usepoll
in the background thread, so we don't have to deal with interrupting the thread, so we now don't accidentally interrupt the actual processing of the queued tasks.Instead of waiting for the Thread to be marked as "finished", we just wait by
Thread::join
. The only way it would block forever, is if the processing of the task would wait forever.Proposed Changes