Skip to content

[fix][broker]Prevent StackOverFlowException in SHARED subscription - #16968

Merged
codelipenghui merged 2 commits into
apache:masterfrom
mattisonchao:fix_stack_over_flow
Aug 8, 2022
Merged

[fix][broker]Prevent StackOverFlowException in SHARED subscription#16968
codelipenghui merged 2 commits into
apache:masterfrom
mattisonchao:fix_stack_over_flow

Conversation

@mattisonchao

@mattisonchao mattisonchao commented Aug 7, 2022

Copy link
Copy Markdown
Member

Motivation

Similar issue: #10696

We found the StackOverFlow when redelivering large messages. the log here:

2022-08-01T15:30:08,243+0000 [pulsar-41-1] WARN  org.apache.bookkeeper.mledger.impl.EntryCacheImpl - failed to read entries for 467924-467924:46
java.lang.StackOverflowError: null
	at jdk.internal.org.objectweb.asm.ClassWriter.newStringishItem(ClassWriter.java:1189) ~[?:?]
	at jdk.internal.org.objectweb.asm.MethodWriter.visitTypeInsn(MethodWriter.java:866) ~[?:?]
	at jdk.internal.org.objectweb.asm.MethodVisitor.visitTypeInsn(MethodVisitor.java:430) ~[?:?]
	at java.lang.invoke.TypeConvertingMethodAdapter.cast(TypeConvertingMethodAdapter.java:188) ~[?:?]
	at java.lang.invoke.TypeConvertingMethodAdapter.convertType(TypeConvertingMethodAdapter.java:241) ~[?:?]
	at java.lang.invoke.InnerClassLambdaMetafactory$ForwardingMethodGenerator.convertArgumentTypes(InnerClassLambdaMetafactory.java:499) ~[?:?]
	at java.lang.invoke.InnerClassLambdaMetafactory$ForwardingMethodGenerator.generate(InnerClassLambdaMetafactory.java:472) ~[?:?]
	at java.lang.invoke.InnerClassLambdaMetafactory.spinInnerClass(InnerClassLambdaMetafactory.java:294) ~[?:?]
	at java.lang.invoke.InnerClassLambdaMetafactory.buildCallSite(InnerClassLambdaMetafactory.java:195) ~[?:?]
	at java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:329) ~[?:?]
	at java.lang.invoke.BootstrapMethodInvoker.invoke(BootstrapMethodInvoker.java:127) ~[?:?]
	at java.lang.invoke.CallSite.makeSite(CallSite.java:307) ~[?:?]
	at java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:258) ~[?:?]
	at java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:248) ~[?:?]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl$12.readEntryFailed(ManagedCursorImpl.java:1398) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.EntryCacheImpl.asyncReadEntry(EntryCacheImpl.java:195) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.asyncReadEntry(ManagedLedgerImpl.java:1982) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.asyncReadEntry(ManagedLedgerImpl.java:1897) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl.lambda$asyncReplayEntries$11(ManagedCursorImpl.java:1413) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177) ~[?:?]
	at java.util.TreeMap$KeySpliterator.forEachRemaining(TreeMap.java:2739) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497) ~[?:?]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl.asyncReplayEntries(ManagedCursorImpl.java:1407) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.asyncReplayEntriesInOrder(PersistentDispatcherMultipleConsumers.java:405) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
=>	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.readMoreEntries(PersistentDispatcherMultipleConsumers.java:255) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
=>	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.sendMessagesToConsumers(PersistentDispatcherMultipleConsumers.java:636) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.readEntriesComplete(PersistentDispatcherMultipleConsumers.java:513) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl$12.readEntryComplete(ManagedCursorImpl.java:1388) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.EntryCacheImpl.asyncReadEntry0(EntryCacheImpl.java:209) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.EntryCacheImpl.asyncReadEntry(EntryCacheImpl.java:188) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.asyncReadEntry(ManagedLedgerImpl.java:1982) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.asyncReadEntry(ManagedLedgerImpl.java:1897) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl.lambda$asyncReplayEntries$11(ManagedCursorImpl.java:1413) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177) ~[?:?]
	at java.util.TreeMap$KeySpliterator.forEachRemaining(TreeMap.java:2739) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497) ~[?:?]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl.asyncReplayEntries(ManagedCursorImpl.java:1407) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.asyncReplayEntriesInOrder(PersistentDispatcherMultipleConsumers.java:405) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
=>	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.readMoreEntries(PersistentDispatcherMultipleConsumers.java:255) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
=>	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.sendMessagesToConsumers(PersistentDispatcherMultipleConsumers.java:636) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.readEntriesComplete(PersistentDispatcherMultipleConsumers.java:513) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl$12.readEntryComplete(ManagedCursorImpl.java:1388) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.EntryCacheImpl.asyncReadEntry0(EntryCacheImpl.java:209) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.EntryCacheImpl.asyncReadEntry(EntryCacheImpl.java:188) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.asyncReadEntry(ManagedLedgerImpl.java:1982) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.asyncReadEntry(ManagedLedgerImpl.java:1897) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl.lambda$asyncReplayEntries$11(ManagedCursorImpl.java:1413) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177) ~[?:?]
	at java.util.TreeMap$KeySpliterator.forEachRemaining(TreeMap.java:2739) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497) ~[?:?]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl.asyncReplayEntries(ManagedCursorImpl.java:1407) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.asyncReplayEntriesInOrder(PersistentDispatcherMultipleConsumers.java:405) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
=>	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.readMoreEntries(PersistentDispatcherMultipleConsumers.java:255) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
=>	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.sendMessagesToConsumers(PersistentDispatcherMultipleConsumers.java:636) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.readEntriesComplete(PersistentDispatcherMultipleConsumers.java:513) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl$12.readEntryComplete(ManagedCursorImpl.java:1388) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.EntryCacheImpl.asyncReadEntry0(EntryCacheImpl.java:209) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.EntryCacheImpl.asyncReadEntry(EntryCacheImpl.java:188) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.asyncReadEntry(ManagedLedgerImpl.java:1982) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.asyncReadEntry(ManagedLedgerImpl.java:1897) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl.lambda$asyncReplayEntries$11(ManagedCursorImpl.java:1413) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177) ~[?:?]
	at java.util.TreeMap$KeySpliterator.forEachRemaining(TreeMap.java:2739) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497) ~[?:?]
	at org.apache.bookkeeper.mledger.impl.ManagedCursorImpl.asyncReplayEntries(ManagedCursorImpl.java:1407) ~[pulsar-managed-ledger-2.10.1.1.jar:2.10.1.1]
	at org.apache.pulsar.broker.service.persistent.PersistentDispatcherMultipleConsumers.asyncReplayEntriesInOrder(PersistentDispatcherMultipleConsumers.java:405) ~[pulsar-pulsar-broker-2.10.1.1.jar:2.10.1.1]

Modifications

  • Avoid recursive calls of readMoreEntries() on the same thread
  • Extract some same behavior readMoreEntries method to readMoreEntriesAsync method.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

  • doc-not-needed
    (Please explain why)

@mattisonchao mattisonchao self-assigned this Aug 7, 2022
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Aug 7, 2022
@mattisonchao

mattisonchao commented Aug 7, 2022

Copy link
Copy Markdown
Member Author

And I'm not sure if we use topic.getBrokerService().getTopicOrderedExecutor().chooseThread() run readMoreEntries method is better than use broker executor.

@mattisonchao mattisonchao added this to the 2.12.0 milestone Aug 7, 2022
Jason918
Jason918 approved these changes Aug 8, 2022
View reviewed changes

@Jason918 Jason918 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

gaoran10
gaoran10 approved these changes Aug 8, 2022
View reviewed changes

@gaoran10 gaoran10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! LGTM

@mattisonchao mattisonchao reopened this Aug 8, 2022
@Technoboy- Technoboy- modified the milestones: 2.12.0, 2.11.0 Aug 8, 2022
This was referenced Aug 10, 2022
Open
Merged
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Aug 16, 2022
@mattisonchao @nicoloboschi
[fix][broker]Prevent StackOverFlowException in SHARED subscription(a…
414578b 
…pache#16968)

(cherry picked from commit a88d952)
Technoboy- pushed a commit to merlimat/pulsar that referenced this pull request Aug 16, 2022
@mattisonchao @Technoboy-
[fix][broker]Prevent StackOverFlowException in SHARED subscription(a…
bdaa059 
…pache#16968)
Technoboy- pushed a commit to merlimat/pulsar that referenced this pull request Aug 16, 2022
@mattisonchao @Technoboy-
[cleanup][broker] Follow up on apache#16968 to restore some behavior …
89e5cb4 
…in PersistentDispatcherMultipleConsumers class (apache#17018)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Aug 16, 2022
@mattisonchao @nicoloboschi
[cleanup][broker] Follow up on apache#16968 to restore some behavior …
c2b346d 
…in PersistentDispatcherMultipleConsumers class (apache#17018)

(cherry picked from commit abff91f)
This was referenced Aug 17, 2022
Merged
Merged
@mattisonchao mattisonchao added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Aug 25, 2022
@dragonls dragonls mentioned this pull request Oct 18, 2022
Closed
nodece pushed a commit to nodece/pulsar that referenced this pull request Sep 10, 2024
@mattisonchao @nodece
[fix][broker]Prevent StackOverFlowException in SHARED subscription(a…
433b22b 
…pache#16968)

(cherry picked from commit a88d952)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
nodece pushed a commit to nodece/pulsar that referenced this pull request Sep 10, 2024
@mattisonchao @nodece
[cleanup][broker] Follow up on apache#16968 to restore some behavior …
6176720 
…in PersistentDispatcherMultipleConsumers class (apache#17018)

(cherry picked from commit abff91f)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lhotari lhotari lhotari left review comments
@michaeljmarshall michaeljmarshall michaeljmarshall left review comments
@Jason918 Jason918 Jason918 approved these changes
@Technoboy- Technoboy- Technoboy- approved these changes
@codelipenghui codelipenghui codelipenghui approved these changes
@gaoran10 gaoran10 gaoran10 approved these changes
@RobertIndie RobertIndie RobertIndie approved these changes
@poorbarcode poorbarcode poorbarcode approved these changes
@shibd shibd shibd approved these changes
@eolivelli eolivelli Awaiting requested review from eolivelli
@BewareMyPower BewareMyPower Awaiting requested review from BewareMyPower

Assignees

@mattisonchao mattisonchao

Labels

cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.8.5 release/2.9.4 release/2.10.2

Projects

None yet

Milestone

2.11.0

Development

Successfully merging this pull request may close these issues.

10 participants

@mattisonchao @lhotari @Jason918 @Technoboy- @codelipenghui @gaoran10 @RobertIndie @poorbarcode @shibd @michaeljmarshall