[
https://issues.apache.org/jira/browse/HIVE-25157?focusedWorklogId=602326&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-602326
]
ASF GitHub Bot logged work on HIVE-25157:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 26/May/21 13:14
Start Date: 26/May/21 13:14
Worklog Time Spent: 10m
Work Description: belugabehr commented on a change in pull request #2312:
URL: https://github.com/apache/hive/pull/2312#discussion_r639713247
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/cache/results/QueryResultsCache.java
##########
@@ -416,58 +412,60 @@ public Path getCacheDirPath() {
/**
* Check if the cache contains an entry for the requested LookupInfo.
+ *
* @param request
- * @return The cached result if there is a match in the cache, or null if
no match is found.
+ * @return The cached result if there is a match in the cache, or null if no
+ * match is found.
+ * @throws NullPointerException if request is {@code null}
*/
- public CacheEntry lookup(LookupInfo request) {
- CacheEntry result = null;
+ public CacheEntry lookup(final LookupInfo request) {
+ Objects.requireNonNull(request);
- LOG.debug("QueryResultsCache lookup for query: {}", request.queryText);
+ LOG.debug("Cache lookup for query: {}", request.queryText);
+ CacheEntry result = null;
boolean foundPending = false;
- // Cannot entries while we currently hold read lock, so keep track of them
to delete later.
- Set<CacheEntry> entriesToRemove = new HashSet<CacheEntry>();
- Lock readLock = rwLock.readLock();
+ // Cannot modify entries while we currently hold read lock, so keep track
of
+ // them to delete later.
+ Set<CacheEntry> entriesToRemove = new HashSet<>();
+ cacheReadLock.lock();
try {
- // Note: ReentrantReadWriteLock deos not allow upgrading a read lock to
a write lock.
+ // Note: ReentrantReadWriteLock does not allow upgrading a read lock to
a write lock.
// Care must be taken while under read lock, to make sure we do not
perform any actions
// which attempt to take a write lock.
- readLock.lock();
- Set<CacheEntry> candidates = queryMap.get(request.queryText);
- if (candidates != null) {
- CacheEntry pendingResult = null;
- for (CacheEntry candidate : candidates) {
- if (entryMatches(request, candidate, entriesToRemove)) {
- CacheEntryStatus entryStatus = candidate.status;
- if (entryStatus == CacheEntryStatus.VALID) {
- result = candidate;
- break;
- } else if (entryStatus == CacheEntryStatus.PENDING &&
pendingResult == null) {
- pendingResult = candidate;
- }
+ Collection<CacheEntry> candidates = queryMap.get(request.queryText);
+ // Try to find valid entry, but settle for pending entry if that is all
+ // there is available
+ for (CacheEntry candidate : candidates) {
+ if (entryMatches(request, candidate, entriesToRemove)) {
+ CacheEntryStatus entryStatus = candidate.status;
+ if (entryStatus == CacheEntryStatus.VALID) {
+ result = candidate;
+ break;
+ }
+ if (entryStatus == CacheEntryStatus.PENDING) {
+ // Only accept first pending result
+ result = (result == null) ? candidate : result;
Review comment:
`queryMap` is now a Guava `Multimap` and therefore does not return null
values (it returns empty `Collection`). Otherwise just simplifying the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 602326)
Time Spent: 2h 10m (was: 2h)
> Clean up QueryResultsCache Code
> -------------------------------
>
> Key: HIVE-25157
> URL: https://issues.apache.org/jira/browse/HIVE-25157
> Project: Hive
> Issue Type: Improvement
> Reporter: David Mollitor
> Assignee: David Mollitor
> Priority: Minor
> Labels: pull-request-available
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> * Remove superfluous code
> * Simplify lock usage (remove instances of {{synchronization}})
> * Simplify code with Guava {{Multimap}}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
