Skip to content

Conversation

@nicoloboschi
Copy link
Contributor

Motivation

I noticed an increased memory usage in long-running process which run the unit broker tests. After investigating I found out the introduction of mockito-inline is the culprit.

In the documentation is specified that it this behaviour is possible and there's a way to fix it

Modifications

  • Added the inline mocks cleanup between tests along with other mockito cleanups
  • no-need-doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 9, 2022
nicoloboschi added a commit to datastax/pulsar that referenced this pull request May 9, 2022
@nicoloboschi
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@nicoloboschi nicoloboschi requested a review from lhotari May 10, 2022 08:50
@nicoloboschi
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Great catch

@nicoloboschi
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

1 similar comment
@nicoloboschi
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@lhotari
Copy link
Member

lhotari commented May 13, 2022

@nicoloboschi Are the tests more flaky after this change or are there real failures? I wonder if the classloading works correctly for Mockito if reflection isn't used in a TestNG listener?

@nicoloboschi nicoloboschi force-pushed the tests-mockito-inline-leak branch from f6b14c7 to 43731ec Compare May 13, 2022 14:24
@nicoloboschi
Copy link
Contributor Author

nicoloboschi commented May 17, 2022

are the tests more flaky after this change or are there real failures?

I believe they are flaky as always. I addressed the PersistentDispatcherFailoverConsumerTest in #15638

let me re-run the tests

@nicoloboschi
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@nicoloboschi
Copy link
Contributor Author

it is better to merge #15638 before this one. then rebase and re-run the checks to see everything is green

@lhotari
Copy link
Member

lhotari commented Jun 2, 2022

This fix is important for stabilizing our CI. My assumption is that it would fix the strange issue #15689 where Mocks from another test are leaked into that test.

@nicoloboschi
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@nicoloboschi nicoloboschi merged commit 14b95ec into apache:master Jun 3, 2022
@nicoloboschi nicoloboschi deleted the tests-mockito-inline-leak branch June 3, 2022 08:25
nicoloboschi added a commit that referenced this pull request Jun 3, 2022
codelipenghui added a commit to codelipenghui/incubator-pulsar that referenced this pull request Jun 10, 2022
@nodece nodece mentioned this pull request Jul 27, 2022
nodece added a commit to nodece/pulsar that referenced this pull request Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants