Skip to content

KAFKA-7660: fix streams and Metrics memory leaks#5982

Merged
mjsax merged 2 commits intoapache:1.0from
vvcephei:1.0-memory-leaks
Dec 6, 2018
Merged

KAFKA-7660: fix streams and Metrics memory leaks#5982
mjsax merged 2 commits intoapache:1.0from
vvcephei:1.0-memory-leaks

Conversation

@vvcephei
Copy link
Copy Markdown
Contributor

Backport two memory-leak fixes (#5974 and #5953) (see also 2.1: #5979, 2.0: #5980, 1.1: #5981 )

It looks like we already had the parentSensors fix in 1.0 and lost it in 2.0. The change in
this PR just tidies it up a little for consistency with later branches.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

vvcephei and others added 2 commits November 30, 2018 17:04
A heap dump provided by Patrik Kleindl in https://issues.apache.org/jira/browse/KAFKA-7660 identifies the childrenSensors map in Metrics as keeping references to sensors alive after they have been removed.

This PR fixes it and adds a test to be sure.

Reviewers: Jason Gustafson <jason@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
@vvcephei
Copy link
Copy Markdown
Contributor Author

@ijuma , sorry to spam you, but I think it might be easier to deal with this in a batch...
The 1.0 branch builder also seems to be misconfigured; the failures for JDK7 and JDK10 are the same as #5981 (comment) (see also #5980 (comment))

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Dec 1, 2018

Retest this please

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 2, 2018

retest this please

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 3, 2018

This branch doesn't work fully with Java 10 either, just Java 9. The 10 failures with Java 10 are expected.

@vvcephei
Copy link
Copy Markdown
Contributor Author

vvcephei commented Dec 3, 2018

Just to clarify about the expected failure. It seems to be an incompatibility between java 10 and easymock:

org.apache.kafka.common.record.FileRecordsTest.testTruncateNotCalledIfSizeIsBiggerThanTargetSize

Error Message
java.lang.IllegalArgumentException
Stacktrace
java.lang.IllegalArgumentException
	at org.easymock.asm.ClassReader.<init>(Unknown Source)
	at org.easymock.asm.ClassReader.<init>(Unknown Source)
	at org.easymock.asm.ClassReader.<init>(Unknown Source)
	at org.easymock.cglib.proxy.BridgeMethodResolver.resolveAll(BridgeMethodResolver.java:63)
	at org.easymock.cglib.proxy.Enhancer.emitMethods(Enhancer.java:1132)
	at org.easymock.cglib.proxy.Enhancer.generateClass(Enhancer.java:630)
	at org.easymock.cglib.core.DefaultGeneratorStrategy.generate(DefaultGeneratorStrategy.java:25)
	at org.easymock.cglib.core.AbstractClassGenerator.generate(AbstractClassGenerator.java:329)
	at org.easymock.cglib.proxy.Enhancer.generate(Enhancer.java:492)
	at org.easymock.cglib.core.AbstractClassGenerator$ClassLoaderData$3.apply(AbstractClassGenerator.java:93)
	at org.easymock.cglib.core.AbstractClassGenerator$ClassLoaderData$3.apply(AbstractClassGenerator.java:91)
	at org.easymock.cglib.core.internal.LoadingCache$2.call(LoadingCache.java:54)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at org.easymock.cglib.core.internal.LoadingCache.createEntry(LoadingCache.java:61)
	at org.easymock.cglib.core.internal.LoadingCache.get(LoadingCache.java:34)
	at org.easymock.cglib.core.AbstractClassGenerator$ClassLoaderData.get(AbstractClassGenerator.java:116)
	at org.easymock.cglib.core.AbstractClassGenerator.create(AbstractClassGenerator.java:291)
	at org.easymock.cglib.proxy.Enhancer.createHelper(Enhancer.java:480)
	at org.easymock.cglib.proxy.Enhancer.createClass(Enhancer.java:337)
	at org.easymock.internal.ClassProxyFactory.createProxy(ClassProxyFactory.java:173)
	at org.easymock.internal.MocksControl.createMock(MocksControl.java:116)
	at org.easymock.internal.MocksControl.createMock(MocksControl.java:95)
	at org.easymock.EasyMock.mock(EasyMock.java:69)
	at org.easymock.EasyMock.createMock(EasyMock.java:311)
	at org.apache.kafka.common.record.FileRecordsTest.testTruncateNotCalledIfSizeIsBiggerThanTargetSize(FileRecordsTest.java:225)

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 3, 2018

Yeah, the ASM version used by EasyMock in this branch is too old.

@mjsax mjsax merged commit 9303455 into apache:1.0 Dec 6, 2018
@vvcephei vvcephei deleted the 1.0-memory-leaks branch December 7, 2018 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants