sarama icon indicating copy to clipboard operation
sarama copied to clipboard

PartitionOffsetManager.Close deadlock without autocommit

Open maciej opened this issue 1 year ago • 2 comments

Description

PartitionOffsetManager.Close() function deadlocks when

config.Consumer.Offsets.AutoCommit.Enable = false

The Close() function relies on pom.errors channel being closed.

// offset_manager.go, line 605 to 617 in github.com/IBM/sarama v1.41.2
func (pom *partitionOffsetManager) Close() error {
	pom.AsyncClose()

	var errors ConsumerErrors
	for err := range pom.errors {  // <-- happens here
		errors = append(errors, err)
	}

	if len(errors) > 0 {
		return errors
	}
	return nil
}

It never is as the offset manager mainLoop is never initialized if auto-commit is disabled.

// offset_manager.go, line 79-82 in github.com/IBM/sarama v1.41.2
	if conf.Consumer.Offsets.AutoCommit.Enable {
		om.ticker = time.NewTicker(conf.Consumer.Offsets.AutoCommit.Interval)
		go withRecover(om.mainLoop)
	}

I believe this behaviour is not acceptable. As a Sarama user I'd expect the called functions to return an error (or panic) if used inappropriately, but never deadlock.

Gist with code reproducing the issue: https://gist.github.com/maciej/d5be479a3576b01a1f5f145aaa803ca1

Versions

(listing Go and Kafka versions, but these are irrelevant to this bug)

Sarama Kafka Go
v1.42.1 2.5.0 1.21.6
Configuration

Already specified.

Logs

N/A

Additional Context

N/A

maciej avatar Jan 24 '24 16:01 maciej

Thanks for reporting this issue!

dnwe avatar Jan 24 '24 18:01 dnwe

Thank you for taking the time to raise this issue. However, it has not had any activity on it in the past 90 days and will be closed in 30 days if no updates occur. Please check if the main branch has already resolved the issue since it was raised. If you believe the issue is still valid and you would like input from the maintainers then please comment to ask for it to be reviewed.

github-actions[bot] avatar Apr 23 '24 20:04 github-actions[bot]

I can still reproduce the issue in Sarama v1.43.2: https://gist.github.com/maciej/c6d0f65d8b1155b39beaba3eada92cdb

Any chance this will get fixed?

maciej avatar May 24 '24 21:05 maciej