> On Feb. 1, 2017, 10:42 a.m., Vimal Sharma wrote: > > The cache size can grow large if the number of registered entities is high. > > It would be good to have a cache eviction policy just like it was done for > > DSL Query caching in ATLAS-1387 > > Jeff Hagelberg wrote: > We could do that. It seems like overkill though. The big difference is > that RequestContext is thread local, and a new RequestContext is created > every time a http request comes in. Once the http request finishes, > RequestContext.clear() is called, and the cache goes out of scope and will be > garbage collected. (See AuditFilter for details). The cache is limited to > the instances used for processsing the request. In the case of the compiled > query cache, there is one effectively static instance of the cache that is > used for all requests. Without the eviction policy, that would continue > growing until the JVM runs out of memory. > > Jeff Hagelberg wrote: > Is there a specific use case you are concerned about? For example, if > someone runs a DSL query that needs to return 100,000 entities, those > entities would be cached. However, even without the cache we would still > need to have the 100,000 entities in memory so they can be serialized to json > and sent back to the client. I'm not sure that the cache increases the > amount of memory required by much. What it does do, though, is keep the > instances that are created in the java heap for a longer period of time. > > David Kantor wrote: > There no need for a cache eviction policy as this cache is only active > for the duration of the request. Perhaps to address the comment, the cache > could be cleared upon completion of the request to make the entries available > for garbage collection sooner. > > Jeff Hagelberg wrote: > Sure, I've changed it so that we explicitly clear the cache.
Got it. I didn't observe that the scope of cache is limited to a single request context. The current implementation looks good. - Vimal ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56102/#review163800 ----------------------------------------------------------- On Feb. 1, 2017, 9:41 p.m., Jeff Hagelberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56102/ > ----------------------------------------------------------- > > (Updated Feb. 1, 2017, 9:41 p.m.) > > > Review request for atlas and David Kantor. > > > Bugs: ATLAS-1388 > https://issues.apache.org/jira/browse/ATLAS-1388 > > > Repository: atlas > > > Description > ------- > > Cache created entities in RequestContext when they are created. > Update/refactor DefaultMetadataService.loadEntities() to use the cached > created entities. > Use the cache in the following places: > FullTextMapper > DefaultMetadataService.onEntitiesAdded - check cache before calling > DefaultMetadataService.loadEntities > DefaultMetadataService.onEntitiesUpdated - check cache before calling > DefaultMetadataService.loadEntities > EntityResource.getEntityDefinition - check cache before calling > DefaultMetadataService.getEntityDefinition > EntityResource.getResponse - check cache before calling > DefaultMetadataService.getEntityDefinition > > > Diffs > ----- > > > repository/src/main/java/org/apache/atlas/discovery/graph/DefaultGraphPersistenceStrategy.java > f4d8f00da863deeb98209aec2dfc4ccb22734cfa > > repository/src/main/java/org/apache/atlas/repository/graph/FullTextMapper.java > 5be8d0bc349294dc4340ffe5831bfb1265825ed8 > > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepository.java > 6608551be44033f07ddbd2ac3f3764d1ac3b0f22 > > repository/src/main/java/org/apache/atlas/repository/graph/GraphToTypedInstanceMapper.java > 38a553abf0151221c5d802e9ce1977a693d1a698 > > repository/src/main/java/org/apache/atlas/repository/graph/TypedInstanceToGraphMapper.java > bae8b2ac0cae6bf6392f91f24030d81089ae660b > repository/src/test/java/org/apache/atlas/TestUtils.java > cda9eac38861fd55c1494d6d90b5e750e6133545 > > repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java > d447c2d90fec6c03c7fecb8d8ef57a6ea072d35a > > repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryDeleteTestBase.java > 9e850a9dc0ac122cf3fcdbf7df72a93643abc98d > > repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryTest.java > 7444bf38834f59454d67cf763c932d665d7ef31c > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java > c902f8126f6b516e84ab9d89b0ec80ac8ed819fd > > repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java > 03ef4fe02f9b4eba273297ea954f00c9181f1f02 > server-api/src/main/java/org/apache/atlas/RequestContext.java > 651a71dc238a3adaac9504a77ba439785fa07ca8 > > Diff: https://reviews.apache.org/r/56102/diff/ > > > Testing > ------- > > Ran full build, no regresssions found. > > > Thanks, > > Jeff Hagelberg > >
