[
https://issues.apache.org/jira/browse/HDDS-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17118635#comment-17118635
]
Istvan Fajth commented on HDDS-3267:
------------------------------------
As discussed in the PR, the problem to solve with this cache is covered with
the following 4 requirement:
- we want to cache db connections and keep them live
- we want to keep the size restriction on the cache
- we want to close the db connection in case an unused cached connection is
evicted from the cache
- we want to close the db connection in case it is used and got evicted from
the cache only at the end of the current use, or - god forbid us - uses on
multiple threads.
As we realised it is not easily solved by LoadingCache.
Also during the discussion we realised the following flaw with the current
caching implementation in ContainerCache:
The ContainerCache code uses this.put() to put the value to the LRUMap, but in
LRUMap put is not overridden, it provides addMapping() which checks properly
for boundaries, and since the ContainerCache code does not check for
boundaries, basically in the current implementation we have an unlimited cache
which lies about itself and says it is limited by maxSize.
At the end of the discussion, we realised that with HDDS-3630 probably we won't
need this whole cache anymore, so if [~esa.hekmat] agrees with [~elek] and me,
we can close the PR and this JIRA also.
> Replace ContainerCache in BlockUtils by LoadingCache
> ----------------------------------------------------
>
> Key: HDDS-3267
> URL: https://issues.apache.org/jira/browse/HDDS-3267
> Project: Hadoop Distributed Data Store
> Issue Type: Improvement
> Reporter: Isa Hekmatizadeh
> Assignee: Isa Hekmatizadeh
> Priority: Minor
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> As discussed in [here|https://github.com/apache/hadoop-ozone/pull/705]
> current version of ContainerCache is just used by BlockUtils and has several
> architectural issues. for example:
> * It uses a ReentrantLock which could be replaced by synchronized methods
> * It should maintain a referenceCount for each DBHandler
> * It extends LRUMap while it would be better to hide it by the composition
> and not expose LRUMap related methods.
> As [~pifta] suggests, we could replace all ContainerCache functionality by
> using Guava LoadingCache.
> This new LoadingCache could be configured to evict by size, by this
> configuration the functionality would be slightly different as it may evict
> DBHandlers while they are in use (referenceCount>0) but we can configure it
> to use reference base eviction based on CacheBuilder.weakValues()
> I want to open this discussion here instead of Github so I created this
> ticket.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
