esahekmat commented on issue #705: HDDS-3249: renew ContainerCache.INSTANCE in order to test it in a fresh state URL: https://github.com/apache/hadoop-ozone/pull/705#issuecomment-603197739 Your description is very helpful to me. thanks a lot. I agree with you that testing isFull for ContainerCache is somehow wired as it related to the LRUMap which is out of scope for this test. we can simply remove this line (assertTrue(cache.isFull())) for this issue (in this PR) and open other Jira tasks to improve the structure of ContainerCache and BlockUtils. about your side notes: 1. I completely agree. composition would be a better idea than inheritance, we can hide the internals of ContainerCache better by composition. I will create a task regarding this in Jira to discuss more there. 2. you're right, we can replace the ReentrantLock with synchronized methods. I will create a task for it in Jira. 3. If ContainerCache is just used in BlockUtils, I think it would be better to make it as an inner class of BlockUtils or move it beside BlockUtils and make it package private. 4. I don't understand it well. I will be appreciated if you elaborate.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
