Github user joshelser commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/298#discussion_r211403555
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/cache/TenantCacheImpl.java ---
@@ -77,57 +152,132 @@ public MemoryManager getMemoryManager() {
return memoryManager;
}
- private Cache<ImmutableBytesPtr,Closeable> getServerCaches() {
+ private Cache<ImmutableBytesPtr,CacheEntry> getServerCaches() {
/* Delay creation of this map until it's needed */
if (serverCaches == null) {
synchronized(this) {
if (serverCaches == null) {
- serverCaches = CacheBuilder.newBuilder()
- .expireAfterAccess(maxTimeToLiveMs,
TimeUnit.MILLISECONDS)
- .ticker(getTicker())
- .removalListener(new
RemovalListener<ImmutableBytesPtr, Closeable>(){
- @Override
- public void
onRemoval(RemovalNotification<ImmutableBytesPtr, Closeable> notification) {
-
Closeables.closeAllQuietly(Collections.singletonList(notification.getValue()));
- }
- })
- .build();
+ serverCaches = buildCache(maxTimeToLiveMs, false);
}
}
}
return serverCaches;
}
+
+ private Cache<ImmutableBytesPtr,CacheEntry>
getPersistentServerCaches() {
+ /* Delay creation of this map until it's needed */
+ if (persistentServerCaches == null) {
+ synchronized(this) {
+ if (persistentServerCaches == null) {
+ persistentServerCaches =
buildCache(maxPersistenceTimeToLiveMs, true);
+ }
+ }
+ }
+ return persistentServerCaches;
+ }
+
+ private Cache<ImmutableBytesPtr, CacheEntry> buildCache(final int ttl,
final boolean isPersistent) {
+ CacheBuilder<Object, Object> builder = CacheBuilder.newBuilder();
+ if (isPersistent) {
+ builder.expireAfterWrite(ttl, TimeUnit.MILLISECONDS);
+ } else {
+ builder.expireAfterAccess(ttl, TimeUnit.MILLISECONDS);
+ }
+ return builder
+ .ticker(getTicker())
+ .removalListener(new RemovalListener<ImmutableBytesPtr,
CacheEntry>(){
+ @Override
+ public void
onRemoval(RemovalNotification<ImmutableBytesPtr, CacheEntry> notification) {
+ if (isPersistent ||
!notification.getValue().getUsePersistentCache()) {
+
Closeables.closeAllQuietly(Collections.singletonList(notification.getValue()));
+ }
+ }
+ })
+ .build();
+ }
- @Override
+ private void evictInactiveEntries(long bytesNeeded) {
+ CacheEntry[] entries =
getPersistentServerCaches().asMap().values().toArray(new CacheEntry[]{});
+ Arrays.sort(entries);
+ long available = this.getMemoryManager().getAvailableMemory();
+ for (int i = 0; i < entries.length && available < bytesNeeded;
i++) {
+ CacheEntry entry = entries[i];
+ if (!entry.isLive()) {
+ getServerCaches().invalidate(entry.getCacheId());
+
getPersistentServerCaches().invalidate(entry.getCacheId());
+ available = this.getMemoryManager().getAvailableMemory();
+ }
+ }
+ }
+
+ private CacheEntry getIfPresent(ImmutableBytesPtr cacheId) {
+ CacheEntry entry =
getPersistentServerCaches().getIfPresent(cacheId);
+ if (entry != null) {
+ getServerCaches().put(cacheId, entry);
+ }return getServerCaches().getIfPresent(cacheId);
+ }
+
+ @Override
public Closeable getServerCache(ImmutableBytesPtr cacheId) {
getServerCaches().cleanUp();
- return getServerCaches().getIfPresent(cacheId);
+ CacheEntry entry = getIfPresent(cacheId);
+ if (entry == null) {
+ return null;
+ }
+ return entry.closeable;
}
-
+
@Override
- public Closeable addServerCache(ImmutableBytesPtr cacheId,
ImmutableBytesWritable cachePtr, byte[] txState, ServerCacheFactory
cacheFactory, boolean useProtoForIndexMaintainer, int clientVersion) throws
SQLException {
+ public Closeable addServerCache(ImmutableBytesPtr cacheId,
ImmutableBytesWritable cachePtr, byte[] txState, ServerCacheFactory
cacheFactory, boolean useProtoForIndexMaintainer, boolean usePersistentCache,
int clientVersion) throws SQLException {
getServerCaches().cleanUp();
- MemoryChunk chunk =
this.getMemoryManager().allocate(cachePtr.getLength() + txState.length);
+ long available = this.getMemoryManager().getAvailableMemory();
--- End diff --
Two scary points (that I hope I just missed via my scanning):
* I don't see a corresponding update to the GlobalMemoryManager. Are we
accounting for the size of `persistentServerCaches` somewhere else?
* Do users who execute queries with persistentCaches have the ability to
deny the queries of users not running persistentCaches (by saturating the total
RS's serverCache). The 30min default timeout to start evicting persistentCaches
makes me worry as that's a long time.
---
