KAFKA-19874: Fix resource leak in TopicBasedRemoteLogMetadataManager. #20849
+11
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…initialization This commit fixes a resource leak where ProducerManager would not be properly closed if ConsumerManager initialization failed. The fix uses temporary variables to hold the manager instances and only assigns them to the instance variables after successful initialization. If any exception occurs during initialization, the temporary instances are properly closed using Utils.closeQuietly(). Changes: - Introduced temporary variables for ProducerManager and ConsumerManager - Only assign to instance variables after full successful initialization - Added cleanup of temporary resources in the catch block - Added descriptive comments explaining the initialization flow
github-actions
bot
added
triage
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
There is a resource leak in the TopicBasedRemoteLogMetadataManager initialization
process. If ProducerManager is created successfully but ConsumerManager creation
fails, the ProducerManager instance is not properly closed, leading to resource
leaks (e.g., unclosed Kafka producer, threads, network connections).
Current Code (Problematic)
lock.writeLock().lock();
try {
producerManager = new ProducerManager(rlmmConfig, partitioner);
consumerManager = new ConsumerManager(rlmmConfig, remotePartitionMetadataStore, partitioner, time);
consumerManager.startConsumerThread();
// If exception occurs here, producerManager is not closed
} catch (Exception e) {
log.error("Encountered error while initializing producer/consumer", e);
initializationFailed = true;
} finally {
lock.writeLock().unlock();
}## Solution
Use temporary variables to hold manager instances and only assign to instance
variables after successful initialization. Clean up temporary resources in the
catch block if any exception occurs.
Impact