Skip to content

Comments

HDDS-5791. Use Guava Cache to replace current ResourceLimitCache for stateMachineDataCache#2694

Closed
ChenSammi wants to merge 4 commits intoapache:masterfrom
ChenSammi:HDDS-5791
Closed

HDDS-5791. Use Guava Cache to replace current ResourceLimitCache for stateMachineDataCache#2694
ChenSammi wants to merge 4 commits intoapache:masterfrom
ChenSammi:HDDS-5791

Conversation

@ChenSammi
Copy link
Contributor

Copy link
Contributor

@lokeshj1703 lokeshj1703 left a comment

Choose a reason for hiding this comment

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

@ChenSammi Thanks for working on this! Please find my comments below.

  1. Guava cache may not provide us the consistency guarantees we need for stateMachineData cache. For instance we must make sure that cache entry is evicted only after majority servers have written the data. But by limiting the number of entries in the guava cache any future addition of entries would lead to eviction of older entries. Ref: https://issues.apache.org/jira/browse/HDDS-2542 - We had seen a race condition earlier between read and write stateMachineData
  2. The expiry time of 15mins after access might lead to retention of chunks for a longer time. This could increase the memory footprint of datanode.

@bshashikant
Copy link
Contributor

@ChenSammi , i think we need much finer control over sizing as well eviction strategy here. Simple guava cache has not worked earlier as well. Eviction has to be controlled based on how the threshold of difference between majority and min index for datanode look like.

Let's hold on this patch for now and let's discuss other alternatives.

@bshashikant
Copy link
Contributor

opened #2704 for resorting to commit index based eviction policy.

@ChenSammi
Copy link
Contributor Author

ChenSammi commented Oct 8, 2021

@ChenSammi Thanks for working on this! Please find my comments below.

1. Guava cache may not provide us the consistency guarantees we need for stateMachineData cache. For instance we must make sure that cache entry is evicted only after majority servers have written the data. But by limiting the number of entries in the guava cache any future addition of entries would lead to eviction of older entries. Ref: https://issues.apache.org/jira/browse/HDDS-2542 - We had seen a race condition earlier between read and write stateMachineData

2. The expiry time of 15mins after access might lead to retention of chunks for a longer time. This could increase the memory footprint of datanode.

@lokeshj1703 , thanks for the feedback. Didn't notice that GuavaCache is used before HDDS-2542.
With read/writeStateMachineData race condition resolved by HDDS-5619, it's a good time to rethink if GuavaCache is safe to use now.
Guava cache with expire time will increae the memory footprint, this is a real cost when using Guava cache, with the benefit of keep the data in cache as long as possbile, that's the key point to reduce the slow follower happen.

@bshashikant
Copy link
Contributor

Closing this as alternate solutions have been proposed to address this.

@bshashikant bshashikant closed this Dec 6, 2021
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.

3 participants