-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allow stop to interrupt the consumer thread and safely release the resource #13418
Conversation
@sajjad-moradi @mcvsubbu I don't find a good way to add a unit test for it. Can you please help verify the fix? |
...re/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java
Show resolved
Hide resolved
Sajjad and I will review this and get back to you. Please hold merging |
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.
The fix looks good to me.
Also I simulated the SEGV scenario by modifying an integration test, and verified that the fix actually does the intended behavior.
Subbu also wants to take a 2nd look. Could you please hold off merging till we have his review as well? |
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, thanks.
Solve #13398 and #10861, and maybe #13263
stop()
interrupt the consumer thread if it is not invoked by the same thread, and wait for it to joinstop()
is already invoked (_shouldStop
is set)stop()
is invoked, we can guarantee the consumer thread is stopped, and we may safely release the resources held by the consumer