Skip to content

Commit

Permalink
Remove invalid query ID cache from HBO cache manager
Browse files Browse the repository at this point in the history
This cache adds a query ID when HBO failed to fetch history stats for
this query. Later during cost based planning, it will skip reading history
stats. However, the saving is small as the reading will be from cache.
And currently we run HBO optimizer twice, getting empty stats for one run
does not necessarily mean getting empty stats for the other run. This is
an over optimization and I remove it here.
  • Loading branch information
feilong-liu committed Nov 9, 2023
1 parent 278c285 commit d39ce0f
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ private Map<PlanCanonicalizationStrategy, PlanNodeWithHash> getPlanNodeHashes(Pl

private PlanNodeStatsEstimate getStatistics(PlanNode planNode, Session session, Lookup lookup, PlanNodeStatsEstimate delegateStats)
{
if (!useHistoryBasedPlanStatisticsEnabled(session) || historyBasedStatisticsCacheManager.loadHistoryFailed(session.getQueryId())) {
if (!useHistoryBasedPlanStatisticsEnabled(session)) {
return delegateStats;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ public class HistoryBasedStatisticsCacheManager

private final Map<QueryId, Map<CachingPlanCanonicalInfoProvider.InputTableCacheKey, PlanStatistics>> inputTableStatistics = new ConcurrentHashMap<>();

// Whether the history is loaded successfully
private final Set<QueryId> invalidQueryId = ConcurrentHashMap.newKeySet();

public HistoryBasedStatisticsCacheManager() {}

public LoadingCache<PlanNodeWithHash, HistoricalPlanStatistics> getStatisticsCache(QueryId queryId, Supplier<HistoryBasedPlanStatisticsProvider> historyBasedPlanStatisticsProvider, long timeoutInMilliSeconds)
Expand All @@ -59,24 +56,13 @@ public LoadingCache<PlanNodeWithHash, HistoricalPlanStatistics> getStatisticsCac
@Override
public HistoricalPlanStatistics load(PlanNodeWithHash key)
{
if (invalidQueryId.contains(queryId)) {
return empty();
}
return loadAll(Collections.singleton(key)).values().stream().findAny().orElseGet(HistoricalPlanStatistics::empty);
}

@Override
public Map<PlanNodeWithHash, HistoricalPlanStatistics> loadAll(Iterable<? extends PlanNodeWithHash> keys)
{
if (invalidQueryId.contains(queryId)) {
Map<PlanNodeWithHash, HistoricalPlanStatistics> emptyResult = new HashMap<>();
keys.forEach(x -> emptyResult.put(x, empty()));
return emptyResult;
}
Map<PlanNodeWithHash, HistoricalPlanStatistics> statistics = new HashMap<>(historyBasedPlanStatisticsProvider.get().getStats(ImmutableList.copyOf(keys), timeoutInMilliSeconds));
if (statistics.isEmpty()) {
invalidQueryId.add(queryId);
}
// loadAll excepts all keys to be written
for (PlanNodeWithHash key : keys) {
statistics.putIfAbsent(key, empty());
Expand All @@ -101,17 +87,11 @@ public void invalidate(QueryId queryId)
statisticsCache.remove(queryId);
canonicalInfoCache.remove(queryId);
inputTableStatistics.remove(queryId);
invalidQueryId.remove(queryId);
}

@VisibleForTesting
public Map<QueryId, Map<CachingPlanCanonicalInfoProvider.CacheKey, PlanNodeCanonicalInfo>> getCanonicalInfoCache()
{
return canonicalInfoCache;
}

public boolean loadHistoryFailed(QueryId queryId)
{
return invalidQueryId.contains(queryId);
}
}

0 comments on commit d39ce0f

Please sign in to comment.