Prevent StackOverFlowException in KEY_SHARED subscription #14121
Conversation
|
@massakam you did a similar fix, please take a look. |
Jason918
left a comment
Jason918
left a comment
There was a problem hiding this comment.
This fix LGTM.
However I wonder if it's better just make readMoreEntries run in async executors, like
public void readMoreEntries() {
topic.getBrokerService().executor().execute(this::doReadMoreEntries);
}
public synchronized void doReadMoreEntries() {
// previous codes in readMoreEntries
}
As there are other 5 places call readMoreEntries() directly in PersistentStickyKeyDispatcherMultipleConsumers#sendMessagesToConsumers and 2 places in PersistentDispatcherMultipleConsumers#sendMessagesToConsumers and 1 place in PersistentDispatcherMultipleConsumers#readEntriesComplete. They may (or may not) cause the same issue like this.
|
@Jason918 I fixed the only case I have seen. I would commit this patch as I found a way (in an integration test of a non OSS client application) to trigger the problem systematically, and I am sure that it must be fixed in this point. (But I am not able to contribute easily a reproducer) |
(cherry picked from commit f97a76e)
…request !68) Prevent StackOverFlowException in KEY_SHARED subscription (apache#14121)
(cherry picked from commit f97a76e)
5 participants
Fix a StackOverFlowException that we have seen on Pulsar in some non-OSS user application.
Problem
We are calling readMoreEntries() recursively.
Modifications
The fix is to schedule the read in another thread, like we did in #10696
Details
The stacktrace is taken from the DataStax fork of Pulsar version 2.8.0_1.1.16, but the problem still applies to master, 2.9 and 2.8 (I didn't check 2.7)