> On April 14, 2017, 8:43 p.m., Jinmei Liao wrote: > > geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java > > Lines 65 (patched) > > <https://reviews.apache.org/r/58460/diff/1/?file=1692784#file1692784line65> > > > > We probably need to do some vetting on those methods to decide whether > > it's a good to put them up as interface methods. Some are definitely good, > > like getCqService, getRegion, getDistributionManager, but some are more > > like impmentation details, like registerInterestStarted() and > > registerInterestCompleted() (not exautive list). > > > > Of course this is an internal interface, we can definitely delete them > > if we don't need them, but I think it's better to start with minimum, and > > add, rather than start with all and delete afterwards. > > Kirk Lund wrote: > I actually updated that interface by refactoring everything to use > InternalCache instead of GemFireCacheImpl. I then copied the InternalCache > from that other branch for this checkin. I would hope that we can eventually > delete InternalCache altogether when we break down god classes like > GemFireCacheImpl. John created InternalCache to help with mocking and > refactoring.
It's either using this InternalCache or the GemfireCacheImple. I guess for your purpose of refactoring which is mostly helping with mocking/testing, and it's an internal class. I am ok with it for now. - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58460/#review172027 ----------------------------------------------------------- On April 14, 2017, 7:49 p.m., Kirk Lund wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58460/ > ----------------------------------------------------------- > > (Updated April 14, 2017, 7:49 p.m.) > > > Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Patrick > Rhomberg, and Udo Kohlmeyer. > > > Bugs: GEODE-2632 > https://issues.apache.org/jira/browse/GEODE-2632 > > > Repository: geode > > > Description > ------- > > Begin refactoring (baby steps) > > * add operations to InternalCache interface > * rename getter for InternalResourceManager getInternalResourceManager > * add @Override annotations > * fixup TODO comments > * delete superfluous comments and dead-code > > The end goals of multiple iterations of refactoring: 1) write high-level > benchmark for client put to server [done], 2) change internal dependencies > from impl class to interface (including GemFireCacheImpl to InternalCache), > 3) fix constructors and invert dependencies of internal classes (including > AcceptorImpl and client Command classes), 4) write low-level benchmark(s) for > client commands, 5) fix any performance issues involving SecurityService. > > I'd like to commit each iteration of refactoring as I go to keep the number > of merge conflicts and number of modified files lower. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/cache/partition/PartitionRegionHelper.java > 6b52e1ce49f133daf61a2f06343940890648ef61 > > geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java > 328ff35b940239bfbcf817d144d97f886506b33a > > geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRegion.java > 8cdc51705a63a60cfbaae1935b762903d0cf7668 > > geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java > 08f916b117a07ed303638366c64a2c9d5b807b91 > geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java > bcb381482a98a4c372613cba7aabd77f150780aa > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java > d0aacc2d95fa38b1bc9faf68de795af5ef3d9090 > > geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java > 258a82e04c7b9d2d09af124cd024d231a55a4d02 > > geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java > 0f5e31647bb899efb21f3b4101f6dba5c34116fd > > geode-core/src/main/java/org/apache/geode/internal/cache/control/RebalanceOperationImpl.java > 42622d534ae1f52a354bd0e71e3c23bd318425ce > > geode-core/src/main/java/org/apache/geode/internal/cache/control/ResourceAdvisor.java > f79a72902def061e6e22c8521b8fd3fb2eb33e0b > > geode-core/src/main/java/org/apache/geode/internal/cache/execute/MemberFunctionExecutor.java > 1952c2aba2f3dcb4107ae1f4da2188d3dab9b85a > > geode-core/src/main/java/org/apache/geode/internal/cache/execute/MultiRegionFunctionExecutor.java > 16baf46cafb2c6ddb8412f28ef54b771c5048fbe > > geode-core/src/main/java/org/apache/geode/internal/cache/execute/PartitionedRegionFunctionExecutor.java > 1f655695703c58609c1c76283ab025cacbf06fa6 > > geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java > 5e86ce87b99fc4e8793461e881853c141fdfec1f > > geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PartitionedRegionRebalanceOp.java > 51be10fca7e62e46aa2124959472eb6665677ef6 > > geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java > a0810d9ae2616903228966331dab329bfa699034 > > geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java > d7bf9d009164b5f9a5cad2ecef98d07e02e4c063 > > geode-core/src/test/java/org/apache/geode/cache/management/MemoryThresholdsDUnitTest.java > ed32be8f463b9012c6171403322800ba43faa3dc > > geode-core/src/test/java/org/apache/geode/cache/management/MemoryThresholdsOffHeapDUnitTest.java > d3b963cdbe3ac45a25ded1916491ba6370772a8f > > geode-core/src/test/java/org/apache/geode/cache/management/ResourceManagerDUnitTest.java > 076254770fa11d263b0d7f461d3680ea92fa98d6 > > geode-core/src/test/java/org/apache/geode/cache/query/dunit/ResourceManagerWithQueryMonitorDUnitTest.java > 8e3888f8c43211607737cdaf01556d7a19191e25 > > geode-core/src/test/java/org/apache/geode/internal/cache/EvictionTestBase.java > 8b1e87fe8993808e7a8a052cdb7a23b01d1fcad7 > > geode-core/src/test/java/org/apache/geode/internal/cache/OffHeapEvictionDUnitTest.java > 3770f4b9c5962f593a6438ee8d97044c5db18027 > > geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionEvictionDUnitTest.java > cc6616a610ff40592698ae2eeb192f82502f3ea1 > > geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionOffHeapEvictionDUnitTest.java > a6d86ec6fa1733233bfd6a22b525563d43bf59d7 > > geode-core/src/test/java/org/apache/geode/internal/cache/control/MemoryMonitorJUnitTest.java > 54145e13778de97c6ab3d217732606b62744d4dc > > geode-core/src/test/java/org/apache/geode/internal/cache/control/MemoryMonitorOffHeapJUnitTest.java > 13c10b8cb32a50248ef2ceb72d924faeba24e194 > > geode-core/src/test/java/org/apache/geode/internal/cache/control/RebalanceOperationDUnitTest.java > 5f1c7fe5aa90767c8009ef7a2596a4a0170f091b > > geode-core/src/test/java/org/apache/geode/management/bean/stats/MemberLevelStatsJUnitTest.java > afa238017419b792e4cb67b53ff5edb38b962e41 > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/EvictionDUnitTest.java > fc2be392122fef4288798d81ee3fac49158cb695 > > geode-rebalancer/src/main/java/org/apache/geode/cache/util/AutoBalancer.java > aa96914866636e6575340b5fd81688dff97ba5d0 > > geode-rebalancer/src/test/java/org/apache/geode/cache/util/AutoBalancerIntegrationJUnitTest.java > d3c48aa977e694c858d56a4a88d71fac11424aea > > > Diff: https://reviews.apache.org/r/58460/diff/1/ > > > Testing > ------- > > precheckin in progress > > > Thanks, > > Kirk Lund > >
