Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Currently CachingSpaceUsageSource only caches usedSpace, since that piece of information is most expensive to collect when using du.

This patch changes the class to also cache capacity and available.

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

How was this patch tested?

Updated unit test.

CI:
https://github.com/adoroszlai/ozone/actions/runs/8294068655

@adoroszlai adoroszlai self-assigned this Mar 15, 2024
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.

This is good also (see #6381)

@adoroszlai adoroszlai marked this pull request as ready for review May 14, 2024 09:52
@adoroszlai adoroszlai requested a review from xichen01 May 15, 2024 06:09
Copy link
Contributor

@xichen01 xichen01 left a comment

Choose a reason for hiding this comment

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

@adoroszlai Thanks for your work on this!

Cache the capacity and available does reduce the system call when writing file. especially in small file scenario. but for some scenario such as shared disks with other service. it may result the Container can not be recognized as a full Container.
So, this a trade-off between performance and correctness.

@xichen01
Copy link
Contributor

LGTM+1.

@adoroszlai adoroszlai requested a review from jojochuang May 20, 2024 09:01
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.

lgtm. Thanks @adoroszlai

@adoroszlai
Copy link
Contributor Author

Thanks @jojochuang, @smengcl, @xichen01 for the review.

swamirishi pushed a commit to swamirishi/ozone that referenced this pull request Dec 3, 2025
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.

4 participants