> On Jan. 23, 2017, 7:29 p.m., Neeru Gupta wrote: > > common/src/main/java/org/apache/atlas/utils/LruCache.java, line 127 > > <https://reviews.apache.org/r/55443/diff/3/?file=1610867#file1610867line127> > > > > Do we need to reap the values for Get operation? Reaping needs to > > acquire a write lock. If lock gets acquired by another thread while > > isReapNeeded check is performd, then getting value from cache may get > > blocked. > > I think we should not reap values while reading from map to avoid delay > > in providing the cached value. If we do, then write lock should be held > > until reaping is done.
I don't think reaping should be avoided during get() -- if the user wants to have reap enabled, then we need to do the reaping when it is required. If the get() does not trigger the reaping, something else will, and things will block until it is done. The time it takes to do the reap is proportional to the number of things that need to be reaped. Once it is finished, the reaping won't happen until the reap time has elapsed again. The reap time should be set pretty high so that this is not a frequent operation. We know there is overhead associated with reaping, and that is why it is disabled by default. Also, the isReapNeeded check will never block. However, reapIfNeeded() will block if isReapNeeded() returns true and another thread has the write lock. That's a good point about the write lock, I'll fix it. - Jeff ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55443/#review162676 ----------------------------------------------------------- On Jan. 23, 2017, 9:41 p.m., Jeff Hagelberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55443/ > ----------------------------------------------------------- > > (Updated Jan. 23, 2017, 9:41 p.m.) > > > Review request for atlas, David Kantor and Neeru Gupta. > > > Bugs: ATLAS-1387 > https://issues.apache.org/jira/browse/ATLAS-1387 > > > Repository: atlas > > > Description > ------- > > Adds a compiled query cache to Atlas. This avoids the overhead of parsing > and translating DSL queries that have been previously executed. > > > Diffs > ----- > > common/pom.xml 0226541b2dd73e8a01f050982cb8b37f5fed9cab > common/src/main/java/org/apache/atlas/utils/LruCache.java PRE-CREATION > common/src/main/java/org/apache/atlas/utils/LruMap.java PRE-CREATION > common/src/main/java/org/apache/atlas/utils/LruMapEntryUsageTracker.java > PRE-CREATION > common/src/test/java/org/apache/atlas/utils/LruCacheTest.java PRE-CREATION > > common/src/test/java/org/apache/atlas/utils/LruMapEntryUsageTrackerTest.java > PRE-CREATION > distro/src/conf/atlas-application.properties > 6fa066b666d7948088f57805a8e2d22bf904aade > > repository/src/main/java/org/apache/atlas/discovery/graph/GraphBackedDiscoveryService.java > b637f90967850319910c17f9017e62f68d14f546 > > repository/src/main/java/org/apache/atlas/util/AtlasRepositoryConfiguration.java > 6655085ee11c94addca1564cf77bbdb001c4586f > repository/src/main/java/org/apache/atlas/util/CompiledQueryCacheKey.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/util/NoopGremlinQuery.java > PRE-CREATION > repository/src/main/scala/org/apache/atlas/query/QueryProcessor.scala > 5693c9ecc1a96154a8b0ac48256e5074dc041c5f > > repository/src/test/java/org/apache/atlas/util/CompiledQueryCacheKeyTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/55443/diff/ > > > Testing > ------- > > Ran all atlas tests, no regressions found. There were some tests that failed > both with and without these changes. > > > Thanks, > > Jeff Hagelberg > >
