Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/298#discussion_r211403791
  
    --- 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 --
    
    I am fairly certain that we export serverCache metrics to the HBase RS 
metrics subsystem. Assuming that we do, can we break out the 
persistentServerCache usage as well? That would be nice to monitor.


---

Reply via email to