OAK-11946 : migrate cache APIs to Caffeine#2807
Conversation
reschke
left a comment
There was a problem hiding this comment.
- We should do this in smaller pieces; easier to review. Let's start with oak-store-document.
- If we just replace we just swapped one present problem with a potential future one. We need to get rid of Oak APIs that depend on an implementation that we do not control. In practice this means that we need to define an Oak caching API, and implement wrappers.
|
@reschke how would you suggest to split this ? we are only removing one guava import and rest all the linked to it. |
… order, version format
|
|
I share the concern of @reschke. Do we really want all our modules (or many of them) to depend on caffeine? Do we know if caffeine follows semantic versioning of its OSGi exports? Defining an Oak API and implementing it with a (thin) wrapper would compartmentalize this issue to a single module. I would suggest for this wrapper to live in |
@jsedding We did a discussion on this topic here: https://issues.apache.org/jira/browse/OAK-11791 and IIUC, we decided to move to caffeine. Also, all the packages exporting these are internal so won't be a problem in case we would want to switch again. But I am open to discussion on using OAK APIs as well, just don't see a pretty solid reason to do it apart from internal exported packages. |
|
In the past, guava has been problematic, because it needed to be installed alongside the oak bundles in the same OSGi container. Applications built on top of Oak would then start using guava as well. This caused two points of contention, a bit of a catch 22:
Matters were made worse in guava's case due to the fact that guava does not follow semantic versioning. IIUC, the whole exercise of removing guava is done in order to get rid of this sort of problem. Switching caches to caffeine, imported as a dependency in OSGi, does not IMHO solve the problem. Defining a custom caching API (which could be pretty much a copy of the caffeine API, or whatever) creates a level of decoupling. Embedding the implementation in the bundle implementing this API maximizes the level of decoupling and, for me, is optional, as it could be done at a later time if needed. BTW, the ticket you linked is closed "Won't do" and concerns itself only with oak-documentstore IIUC. BTW 2: I am not objecting to caffeine as the implementation, it looks like a solid choice 🙂 I only object to splashing its API all over Oak. |
|
Yup. |


Summary
Migrates all Guava cache API usages to Caffeine (
com.github.benmanes.caffeine) across the Oak codebase.Changes
CacheLIRS,CacheStats,AbstractCacheStats,EmpiricalWeigher— replace GuavaCache/CacheBuilder/Weigher/CacheStats; bumppackage-info.javaversionDocumentNodeStore,DocumentNodeStoreBuilder,NodeDocumentCache,NodeCache,PersistentCache,CachingCommitValueResolver,LocalDiffCache,MemoryDiffCache,TieredDiffCache,NodeDocument,ForwardingListener,EvictionListener— replace Guava cache imports; adaptCallable→Function,reload()→asyncReload()SegmentCache,ReaderCache,RecordCache,WriterCacheManager,PriorityCache,CacheWeights,RecordCacheStats,SegmentCacheStats— replace Guava cache imports; add.executor(Runnable::run)andcleanUp()for synchronous evictionFileCache,CompositeDataStoreCache,UploadStagingCache,CachingBlobStore,AbstractSharedCachingDataStore,DataStoreBlobStore— replace GuavaCache/CacheBuilder/CacheLoader/AbstractCache/Weigher/RemovalCauseBlobIdSet— replaceCacheBuilderwithCaffeineS3Backend— replaceCacheBuilderwithCaffeineAzureBlobStoreBackend,AzureBlobStoreBackendV8— replaceCacheBuilderwithCaffeineExtractedTextCache— replace GuavaCache/CacheBuilder/Weigherwith CaffeineElasticIndexStatistics— replaceCacheBuilder/CacheLoader/LoadingCache/Ticker;reload()→asyncReload()returningCompletableFuture;getUnchecked()→get()DocumentNodeStoreHelper— replace GuavaCacheimportPersistentCacheTest— replace GuavaCacheimportCacheStatsMetricsTest— adapt to CaffeineCacheStats.of()Test Plan
mvn test -pl oak-core-spi— CacheLIRS, CacheStats testsmvn test -pl oak-store-document— NodeCache, DocumentNodeStore, persistent cache testsmvn test -pl oak-segment-tar— SegmentCache, PriorityCache testsmvn test -pl oak-blob-plugins— FileCache, CompositeDataStoreCache testsmvn test -pl oak-search-elastic— ElasticIndexStatistics testsLinks