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.

Sign up for GitHub

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

Jump to bottom

fix: prevent AsyncProducer retryBatch from leaking #2208

Merged
merged 1 commit into from
Apr 13, 2022
Merged

fix: prevent AsyncProducer retryBatch from leaking #2208

merged 1 commit into from
Apr 13, 2022

Conversation

k-wall
Copy link
Contributor

@k-wall k-wall commented Apr 11, 2022 •
edited
Loading

retryBatch caused the brokerRefs count to be incremented (getBrokerProducer), but it is never decremented again, so the
goroutines related to the brokerProducer are leaked.

This is the final goroutine leak discovered by #2202.

./tests -test.run ^TestAsyncProducerIdempotentRetryCheckBatch$
PASS
goleak: Errors on successful test run: found unexpected goroutines:
[Goroutine 54 in state select, with github.com/Shopify/sarama.(*brokerProducer).run on top of the stack:
goroutine 54 [select]:
github.com/Shopify/sarama.(*brokerProducer).run(0xc0002883c0)
	/Users/kwall/src/sarama/async_producer.go:807 +0x19a
github.com/Shopify/sarama.withRecover(0x0)
	/Users/kwall/src/sarama/utils.go:43 +0x3e
created by github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer
	/Users/kwall/src/sarama/async_producer.go:692 +0x25b

 Goroutine 55 in state chan receive, with github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func1 on top of the stack:
goroutine 55 [chan receive]:
github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func1()
	/Users/kwall/src/sarama/async_producer.go:699 +0x65
github.com/Shopify/sarama.withRecover(0x0)
	/Users/kwall/src/sarama/utils.go:43 +0x3e
created by github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer
	/Users/kwall/src/sarama/async_producer.go:695 +0x32f

 Goroutine 56 in state chan receive, with github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func2 on top of the stack:
goroutine 56 [chan receive]:
github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func2()
	/Users/kwall/src/sarama/async_producer.go:745 +0xcc
github.com/Shopify/sarama.withRecover(0x0)
	/Users/kwall/src/sarama/utils.go:43 +0x3e
created by github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer
	/Users/kwall/src/sarama/async_producer.go:741 +0x3c7

 Goroutine 62 in state select, with github.com/Shopify/sarama.(*brokerProducer).run on top of the stack:
goroutine 62 [select]:
github.com/Shopify/sarama.(*brokerProducer).run(0xc0002889c0)
	/Users/kwall/src/sarama/async_producer.go:807 +0x19a
github.com/Shopify/sarama.withRecover(0x0)
	/Users/kwall/src/sarama/utils.go:43 +0x3e
created by github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer
	/Users/kwall/src/sarama/async_producer.go:692 +0x25b

 Goroutine 63 in state chan receive, with github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func1 on top of the stack:
goroutine 63 [chan receive]:
github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func1()
	/Users/kwall/src/sarama/async_producer.go:699 +0x65
github.com/Shopify/sarama.withRecover(0x0)
	/Users/kwall/src/sarama/utils.go:43 +0x3e
created by github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer
	/Users/kwall/src/sarama/async_producer.go:695 +0x32f

 Goroutine 64 in state chan receive, with github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func2 on top of the stack:
goroutine 64 [chan receive]:
github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func2()
	/Users/kwall/src/sarama/async_producer.go:745 +0xcc
github.com/Shopify/sarama.withRecover(0x0)
	/Users/kwall/src/sarama/utils.go:43 +0x3e
created by github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer
	/Users/kwall/src/sarama/async_producer.go:741 +0x3c7
]

@k-wall k-wall changed the title bugfix: AsyncProducer retryBatch causes goroutines to be leaked AsyncProducer retryBatch causes goroutines to be leaked Apr 11, 2022
@k-wall k-wall changed the title AsyncProducer retryBatch causes goroutines to be leaked fix: AsyncProducer retryBatch causes goroutines to be leaked Apr 11, 2022
@k-wall k-wall changed the title fix: AsyncProducer retryBatch causes goroutines to be leaked fix: AsyncProducer retryBatch allows goroutines to be leaked Apr 11, 2022
@k-wall k-wall mentioned this pull request Apr 11, 2022
Closed
@k-wall
fix: prevent AsyncProducer retryBatch from leaking
c34e274 
retryBatch caused the brokerRefs count to be incremented
(getBrokerProducer), but it is never decremented again, so the
goroutines related to the brokerProducer are leaked.
@dnwe dnwe force-pushed the retry-batch-leak branch from de2038a to c34e274 Compare April 13, 2022 21:24
@dnwe dnwe added the fix label Apr 13, 2022
@dnwe dnwe changed the title fix: AsyncProducer retryBatch allows goroutines to be leaked fix: prevent AsyncProducer retryBatch from leaking Apr 13, 2022
dnwe
dnwe approved these changes Apr 13, 2022
View reviewed changes
Copy link
Collaborator

@dnwe dnwe left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@dnwe dnwe merged commit 602d831 into IBM:main Apr 13, 2022
This was referenced May 11, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dnwe dnwe dnwe approved these changes

Assignees
No one assigned
Labels
fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@k-wall @dnwe