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

PartitionOffsetManager.Close deadlock without autocommit #2772

Open
maciej opened this issue Jan 24, 2024 · 2 comments
Open

PartitionOffsetManager.Close deadlock without autocommit #2772

maciej opened this issue Jan 24, 2024 · 2 comments
Labels
bug needs-investigation Issues that require followup from maintainers stale Issues and pull requests without any recent activity

Comments

@maciej
Copy link

maciej commented Jan 24, 2024

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

@dnwe dnwe added bug needs-investigation Issues that require followup from maintainers labels Jan 24, 2024
@dnwe
Copy link
Collaborator

dnwe commented Jan 24, 2024

Thanks for reporting this issue!

Copy link

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 github-actions bot added the stale Issues and pull requests without any recent activity label Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs-investigation Issues that require followup from maintainers stale Issues and pull requests without any recent activity
Projects
None yet
Development

No branches or pull requests

2 participants