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 HiveMinioDataLake leak when start fails #14763

Merged
merged 1 commit into from
Oct 27, 2022
Merged

Fix HiveMinioDataLake leak when start fails #14763

merged 1 commit into from
Oct 27, 2022

Conversation

findepi
Copy link
Member

@findepi findepi commented Oct 26, 2022

HiveMinioDataLake.start starts more than one container. If second
fails to start, first will be left running. That's why the class has
separate start method. The caller needs to hold on to an instance
before invoking the start method, and ensure close() is called
regardless of start success.

@findepi
Fix HiveMinioDataLake leak when start fails
8d662d5 
`HiveMinioDataLake.start` starts more than one container. If second
fails to start, first will be left running. That's why the class has
separate `start` method. The caller needs to hold on to an instance
before invoking the `start` method, and ensure `close()` is called
regardless of `start` success.
@findepi findepi added test no-release-notes This pull request does not require release notes entry labels Oct 26, 2022
@cla-bot cla-bot bot added the cla-signed label Oct 26, 2022
skrzypo987
skrzypo987 approved these changes Oct 26, 2022
View reviewed changes
Copy link
Member

@skrzypo987 skrzypo987 left a comment

Choose a reason for hiding this comment

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

I wonder what we can do to prevent that mistake from happening again.

@findepi findepi merged commit adc14ab into trinodb:master Oct 27, 2022
@findepi findepi deleted the findepi/fix-hiveminiodatalake-leak-when-start-fails-121c7a branch October 27, 2022 16:30
@github-actions github-actions bot added this to the 402 milestone Oct 27, 2022
@colebow colebow mentioned this pull request Nov 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skrzypo987 skrzypo987 skrzypo987 approved these changes

@Praveen2112 Praveen2112 Praveen2112 approved these changes

@kokosing kokosing kokosing approved these changes

@Dith3r Dith3r Dith3r approved these changes

Assignees
No one assigned
Labels
cla-signed no-release-notes This pull request does not require release notes entry test
Milestone
402
Development

Successfully merging this pull request may close these issues.
5 participants
@findepi @Dith3r @kokosing @Praveen2112 @skrzypo987