Skip to content

HDDS-8020. File checksum helper leaking client#4307

Merged
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-8020
Feb 24, 2023
Merged

HDDS-8020. File checksum helper leaking client#4307
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-8020

Conversation

@adoroszlai
Copy link
Copy Markdown
Contributor

@adoroszlai adoroszlai commented Feb 23, 2023

What changes were proposed in this pull request?

Let file checksum helper release client in case of success to prevent memleak.

(It seems logic for releasing client was copied from BlockInputStream, which has been fixed in HDDS-7931 since then.)

https://issues.apache.org/jira/browse/HDDS-8020

How was this patch tested?

mvn clean test -am -pl :ozone-integration-test -Dtest='TestOzoneFileChecksum'

Verified no warning about ManagedChannelImpl was not shutdown properly appears in test log.

CI:
https://github.com/adoroszlai/hadoop-ozone/actions/runs/4257319915

@adoroszlai adoroszlai self-assigned this Feb 23, 2023
@adoroszlai adoroszlai marked this pull request as ready for review February 24, 2023 09:14
Copy link
Copy Markdown
Contributor

@sodonnel sodonnel left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@duongkame duongkame left a comment

Choose a reason for hiding this comment

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

+1, lgtm

@adoroszlai adoroszlai merged commit e3868fa into apache:master Feb 24, 2023
@adoroszlai adoroszlai deleted the HDDS-8020 branch February 24, 2023 17:57
@adoroszlai
Copy link
Copy Markdown
Contributor Author

Thanks @ayushtkn, @duongkame, @sodonnel for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants