Skip to content
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

Flaky ReactivePulsarListenerTombstoneTests$SingleComplexPayload causing random CI PR failures #561

Open
onobc opened this issue Feb 1, 2024 · 10 comments
Labels
flaky-test theme: testing Issues related to testing

Comments

@onobc
Copy link
Collaborator

onobc commented Feb 1, 2024

The ReactivePulsarListenerTombstoneTests$SingleComplexPayload fails randomly from time/time.

Example: https://github.com/spring-projects/spring-pulsar/actions/runs/7735972858/job/21092530057#step:4:7079

This needs to be fixed.

@onobc onobc added theme: testing Issues related to testing flaky-test labels Feb 1, 2024
@KartikShrivastava
Copy link
Contributor

Hi @onobc, can i work on this?

@onobc
Copy link
Collaborator Author

onobc commented Feb 18, 2024

Hi @KartikShrivastava ,

That would be great. The trick will be to reproduce it. I have not had luck doing so locally. Let me know if you have any questions.

@KartikShrivastava
Copy link
Contributor

Hi @onobc,
I too couldn't reproduce it locally. I have two hypothesis tho:

  • Messages are received in order different then the test expects
  • assertMessagesReceivedWithHeaders is called before latchWithHeaders is finished executing?

wdyt?

P.S.:
Linking the CI PRs failures I found associated with this flakiness [1] [2]

@onobc
Copy link
Collaborator Author

onobc commented Feb 25, 2024

Hi @KartikShrivastava ,
Thanks for digging into this!

Those are very plausible theories. I am not opposed to adding temporary code in tests or main (logging etc..) that may help prove the points. Do you see anything (and anywhere) we can instrument to see if we can get some more info during the CI run?

@KartikShrivastava
Copy link
Contributor

I will need to spend more time with this repo to comment on instrumenting more helpful info. But like you mentioned it's not a problem to add temporary code, then I can create a PR with some extra logging statements which we can use to confirm the value in receivedMessagesWithHeaders variable right before we assert it

@onobc
Copy link
Collaborator Author

onobc commented Feb 28, 2024

Hi @KartikShrivastava ,
If you do not want to continue looking into this issue I totally understand. The issues currently in the backlog are rather coarse grained and if you are looking something small to work on there is not a lot to choose from. We have been meaning to put some smaller issues in and mark them accordingly. I will try to get to that this week but in the meantime if you want me to find you something else just let me know. And there is no obligation to do anything - I just wanted to offer this in case.

Thanks

@KartikShrivastava
Copy link
Contributor

Hi @onobc, apologies for not raising that small PR yet, I was planning to do it in coming weekend. My weekdays availability is most of the time very less.

P.S: Please feel free to assign any other small and low priority task you have in mind, I find this project quite interesting to explore and learn from

@onobc
Copy link
Collaborator Author

onobc commented Feb 28, 2024

Absolutely no worries @KartikShrivastava . No need to apologize. I just thought about the issue and that it is not the most fun thing to work on and thought you might enjoy something else. I will ping you when we have added some low priority issues.

Thanks 👍🏻

@KartikShrivastava
Copy link
Contributor

Hi @onobc, I've created a PR adding one info log, please review as per your availability. Thanks!

onobc pushed a commit that referenced this issue Mar 4, 2024
@KartikShrivastava
Copy link
Contributor

Looks like this flaky test failed in a recent build, here's the stack trace (it failed after executing logger.info(message.toString()); statement once):

ReactivePulsarListenerTombstoneTests > SingleComplexPayload > shouldReceiveMessagesWithTombstone() STANDARD_OUT
    17:24:20.410 [Test worker] INFO  org.springframework.pulsar.reactive.listener.ReactivePulsarListenerTombstoneTests$SingleComplexPayload - ReceivedMessage[payload=Foo[value=foo], keyHeader=key:foo]

ReactivePulsarListenerTombstoneTests > SingleComplexPayload > shouldReceiveMessagesWithTombstone() FAILED
    java.util.ConcurrentModificationException
        at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013)
        at java.base/java.util.ArrayList$Itr.next(ArrayList.java:967)
        at org.springframework.pulsar.reactive.listener.ReactivePulsarListenerTombstoneTests$SingleComplexPayload.shouldReceiveMessagesWithTombstone(ReactivePulsarListenerTombstoneTests.java:249)

What could be the reason of ConcurrentModificationException?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test theme: testing Issues related to testing
Projects
None yet
Development

No branches or pull requests

2 participants