[ 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)