Skip to content

Conversation

@duongkame
Copy link
Contributor

@duongkame duongkame commented Feb 2, 2024

What changes were proposed in this pull request?

Ozone client allocates large chunks of heap memory when writing keys (BlockOutputStream, BufferPool).

Extract client-related buffer improvement from #5497.

What is the link to the Apache JIRA

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

How was this patch tested?

CI: https://github.com/duongkame/ozone/actions/runs/7759444701

@jojochuang
Copy link
Contributor

will there be a similar effort to improve input stream memory consumption?

@duongkame
Copy link
Contributor Author

will there be a similar effort to improve input stream memory consumption?

Yes. https://issues.apache.org/jira/browse/HDDS-10283
Also, this PR is still a WIP, the non-incremental buffer needs changing as well.

@duongkame
Copy link
Contributor Author

This one is blocked by https://issues.apache.org/jira/browse/HDDS-10288.

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

Looks good. It would be nice to enable the leak detection for some of the tests that use BlockOutputStream (e.g. TestBlockOutputStream)

@duongkame
Copy link
Contributor Author

Looks good. It would be nice to enable the leak detection for some of the tests that use BlockOutputStream (e.g. TestBlockOutputStream)

Thanks for the review @jojochuang. All the tests with MiniCluster have LeakDetection enabled by default already.
https://github.com/apache/ozone/blob/master/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java#L120

@duongkame duongkame marked this pull request as ready for review February 8, 2024 01:46
@smengcl
Copy link
Contributor

smengcl commented Feb 8, 2024

hmm. leak detected in tests?

Error:  org.apache.hadoop.hdds.scm.storage.TestCommitWatcher.testReleaseBuffers  Time elapsed: 18.44 s  <<< FAILURE!
java.lang.AssertionError: Found 2 leaked objects, check logs
	at org.apache.hadoop.hdds.utils.db.CodecBuffer.assertNoLeaks(CodecBuffer.java:232)
	at org.apache.hadoop.hdds.utils.db.CodecTestUtil.gc(CodecTestUtil.java:54)
Error:  org.apache.hadoop.hdds.scm.storage.TestCommitWatcher.testReleaseBuffersOnException  Time elapsed: 35.839 s  <<< FAILURE!
java.lang.AssertionError: Found 4 leaked objects, check logs
	at org.apache.hadoop.hdds.utils.db.CodecBuffer.assertNoLeaks(CodecBuffer.java:232)

https://github.com/apache/ozone/actions/runs/7823740701/job/21345534308#step:6:178

Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

Thanks @duongkame . lgtm

btw do we have data from (manual) testing that actually shows the lower client JVM heap usage?

@adoroszlai
Copy link
Contributor

Thanks @duongkame for the improvement.

Since it is extracted from #5497, I think we should add the following when merging this PR:

Co-authored-by: Tsz-Wo Nicholas Sze <[email protected]>

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

@duongkame , thanks a lot for working on this!

+1 the change looks good.

@szetszwo szetszwo merged commit 75df6c1 into apache:master Feb 9, 2024
@szetszwo
Copy link
Contributor

szetszwo commented Feb 9, 2024

@jojochuang , @smengcl and @adoroszlai , thanks a lot for reviewing this!

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Feb 28, 2024
Co-authored-by: Tsz-Wo Nicholas Sze <[email protected]>
(cherry picked from commit 75df6c1)
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Feb 28, 2024
Co-authored-by: Tsz-Wo Nicholas Sze <[email protected]>
(cherry picked from commit 75df6c1)
smengcl added a commit to smengcl/hadoop-ozone that referenced this pull request Mar 4, 2024
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Mar 5, 2024
Co-authored-by: Tsz-Wo Nicholas Sze <[email protected]>
(cherry picked from commit 75df6c1)
Change-Id: I2f7fd360ef43267ac382e66a1dceb546e06a03c3
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Mar 5, 2024
Co-authored-by: Tsz-Wo Nicholas Sze <[email protected]>
(cherry picked from commit 75df6c1)
Change-Id: Ieaa18864dd4316305df31b05688a5cafa039927c
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Mar 5, 2024
Co-authored-by: Tsz-Wo Nicholas Sze <[email protected]>
(cherry picked from commit 75df6c1)
Change-Id: Ieaa18864dd4316305df31b05688a5cafa039927c
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Mar 5, 2024
Co-authored-by: Tsz-Wo Nicholas Sze <[email protected]>
(cherry picked from commit 75df6c1)
Change-Id: I0cdce3fc27f9a65bcdfd1225dac7bd93ba5ede4f
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Mar 5, 2024
Co-authored-by: Tsz-Wo Nicholas Sze <[email protected]>
(cherry picked from commit 75df6c1)
Change-Id: I180c759a4bedb03bf3b83e231dfa1ad17af6425e
@duongkame duongkame deleted the HDDS-9843 branch April 12, 2025 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants