[ 
https://issues.apache.org/jira/browse/PHOENIX-4666?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16446183#comment-16446183
 ] 

ASF GitHub Bot commented on PHOENIX-4666:
-----------------------------------------

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

    https://github.com/apache/phoenix/pull/298#discussion_r183130270
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/cache/TenantCacheImpl.java ---
    @@ -77,57 +153,164 @@ 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) {
    +        return CacheBuilder.newBuilder()
    +            .expireAfterAccess(ttl, TimeUnit.MILLISECONDS)
    +            .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 maybeGet(ImmutableBytesPtr cacheId) {
    +        maybePromote(cacheId);
    +        CacheEntry entry = getServerCaches().getIfPresent(cacheId);
    +        return entry;
    +    }
    +
    +    private void maybePromote(ImmutableBytesPtr cacheId) {
    +        CacheEntry entry = 
getPersistentServerCaches().getIfPresent(cacheId);
    +        if (entry == null) {
    +            return;
    +        }
    +        getServerCaches().put(cacheId, entry);
    +    }
    +    
    +    private void maybeDemote(ImmutableBytesPtr cacheId) {
    +        CacheEntry entry = getServerCaches().getIfPresent(cacheId);
    +        if (entry == null) {
    +            return;
    +        }
    +        entry.decrementLiveQueryCount();
    +        if (!entry.isLive()) {
    +            getServerCaches().invalidate(cacheId);
    +        }
    +    }
    +    
    +    public void debugPrintCaches() {
    +           System.out.println("Live cache:" + getServerCaches());
    +           for (ImmutableBytesPtr key : 
getServerCaches().asMap().keySet()) {
    +                   System.out.println("- " + 
Hex.encodeHexString(key.get()) +
    +                                   " -> " + 
getServerCaches().getIfPresent(key).size +
    +                                   " lq:" + 
getServerCaches().getIfPresent(key).liveQueriesCount +
    +                                   " " + 
Hex.encodeHexString(getServerCaches().getIfPresent(key).cachePtr.get()));
    +           }
    +           System.out.println("Persistent cache:" + 
getPersistentServerCaches());
    +           for (ImmutableBytesPtr key : 
getPersistentServerCaches().asMap().keySet()) {
    +                   System.out.println("- " + 
Hex.encodeHexString(key.get()) +
    +                                   " -> " + 
getPersistentServerCaches().getIfPresent(key).size +
    +                                   " " + 
Hex.encodeHexString(getPersistentServerCaches().getIfPresent(key).cachePtr.get()));
    +           }
    +   }
    +
    +   @Override
         public Closeable getServerCache(ImmutableBytesPtr cacheId) {
             getServerCaches().cleanUp();
    -        return getServerCaches().getIfPresent(cacheId);
    +        CacheEntry entry = maybeGet(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 boolean checkServerCache(ImmutableBytesPtr cacheId, boolean 
shouldIncrementLiveQueryCount) {
             getServerCaches().cleanUp();
    -        MemoryChunk chunk = 
this.getMemoryManager().allocate(cachePtr.getLength() + txState.length);
    +        CacheEntry entry = maybeGet(cacheId);
    +        if (entry != null && shouldIncrementLiveQueryCount) {
    +           entry.incrementLiveQueryCount();
    +        }
    +           return entry != null;
    +    }
    +    
    +    @Override
    +    public Closeable addServerCache(ImmutableBytesPtr cacheId, 
ImmutableBytesWritable cachePtr, byte[] txState, ServerCacheFactory 
cacheFactory, boolean useProtoForIndexMaintainer, boolean usePersistentCache, 
int clientVersion) throws SQLException {
    +        getServerCaches().cleanUp();
    +        long available = this.getMemoryManager().getAvailableMemory();
    +        int size = cachePtr.getLength() + txState.length;
    +        if (size > available) {
    +            evictInactiveEntries(Math.max(size - available + 
EVICTION_MARGIN_BYTES, EVICTION_MARGIN_BYTES));
    +        }
    +        MemoryChunk chunk = this.getMemoryManager().allocate(size);
             boolean success = false;
             try {
    -            Closeable element = cacheFactory.newCache(cachePtr, txState, 
chunk, useProtoForIndexMaintainer, clientVersion);
    -            getServerCaches().put(cacheId, element);
    +            CacheEntry entry;
    +            synchronized(this) {
    +                entry = maybeGet(cacheId);
    +                if (entry == null) {
    +                    entry = new CacheEntry(
    +                            cacheId, cachePtr, cacheFactory, txState, 
chunk,
    +                            usePersistentCache, useProtoForIndexMaintainer,
    +                            clientVersion);
    +                    getServerCaches().put(cacheId, entry);
    +                    available = 
this.getMemoryManager().getAvailableMemory();
    +                    if (usePersistentCache) {
    +                        getPersistentServerCaches().put(cacheId, entry);
    +                    }
    +                }
    +                entry.incrementLiveQueryCount();
    +            }
                 success = true;
    -            return element;
    +            return entry;
             } finally {
                 if (!success) {
                     
Closeables.closeAllQuietly(Collections.singletonList(chunk));
                 }
    -        }           
    +        }
         }
    -    
    +
         @Override
    -    public void removeServerCache(ImmutableBytesPtr cacheId) {
    -        getServerCaches().invalidate(cacheId);
    +    synchronized public void removeServerCache(ImmutableBytesPtr cacheId) {
    --- End diff --
    
    Is there any reason why we need to "demote" the cache instead of 
invalidating it here?


> Add a subquery cache that persists beyond the life of a query
> -------------------------------------------------------------
>
>                 Key: PHOENIX-4666
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-4666
>             Project: Phoenix
>          Issue Type: Improvement
>            Reporter: Marcell Ortutay
>            Assignee: Marcell Ortutay
>            Priority: Major
>
> The user list thread for additional context is here: 
> [https://lists.apache.org/thread.html/e62a6f5d79bdf7cd238ea79aed8886816d21224d12b0f1fe9b6bb075@%3Cuser.phoenix.apache.org%3E]
> ----
> A Phoenix query may contain expensive subqueries, and moreover those 
> expensive subqueries may be used across multiple different queries. While 
> whole result caching is possible at the application level, it is not possible 
> to cache subresults in the application. This can cause bad performance for 
> queries in which the subquery is the most expensive part of the query, and 
> the application is powerless to do anything at the query level. It would be 
> good if Phoenix provided a way to cache subquery results, as it would provide 
> a significant performance gain.
> An illustrative example:
>     SELECT * FROM table1 JOIN (SELECT id_1 FROM large_table WHERE x = 10) 
> expensive_result ON table1.id_1 = expensive_result.id_2 AND table1.id_1 = 
> \{id}
> In this case, the subquery "expensive_result" is expensive to compute, but it 
> doesn't change between queries. The rest of the query does because of the 
> \{id} parameter. This means the application can't cache it, but it would be 
> good if there was a way to cache expensive_result.
> Note that there is currently a coprocessor based "server cache", but the data 
> in this "cache" is not persisted across queries. It is deleted after a TTL 
> expires (30sec by default), or when the query completes.
> This is issue is fairly high priority for us at 23andMe and we'd be happy to 
> provide a patch with some guidance from Phoenix maintainers. We are currently 
> putting together a design document for a solution, and we'll post it to this 
> Jira ticket for review in a few days.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to