[GitHub] [hive] scarlin-cloudera commented on a change in pull request #2342: HIVE-25189: Fetch validWriteIdList for tables before table request.

[GitBox <gi...@apache.org>]

scarlin-cloudera commented on a change in pull request #2342:
URL: https://github.com/apache/hive/pull/2342#discussion_r644842695



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CacheTableHelper.java
##########
@@ -0,0 +1,203 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.parse;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.hadoop.hive.common.ValidTxnList;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hadoop.hive.ql.lockmgr.HiveTxnManager;
+import org.apache.hadoop.hive.ql.session.SessionState;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Class to help populate the cache at the beginning of query analysis. We would like
+ * to minimize the number of calls to fetch validWriteIdLists from the metastore. HMS
+ * has an API to request this object for multiple tables within one call, and this class
+ * uses that API.
+ *
+ * The sole purpose of this class is to help populate the HMS query cache. Nothing is returned
+ * from the public methods. In this way, if another method attempts to fetch a validWriteIdList,
+ * the SessionHiveMetaStoreClient query cache will contain the information.
+ *
+ * Because this class is only responsible for cache population, it is not a requirement for
+ * a caller to supply all the tables necessary for the query. It is also not a requirement
+ * for the tables to be part of the query. Of course, the query qill benefit if those
+ * conditions were true, but if the table is not in the cache, a later call fetching the writeids
+ * will hit the HMS server and will not fail.
+ *
+ * One tricky aspect to this class is that if a view is passed in, we want to fetch the
+ * validWriteIdLists for the underlying tables. At the beginning of the query, it is impossible
+ * to know the underlying tables without contacting HMS.
+ *
+ * In order to handle the underlying tables to the views, we keep a cache that holds our
+ * best guess. If we see a view in any query, we set up a server-wide cache that tracks
+ * the underlying tables to the view. If the view doesn't change, then this information
+ * will be accurate and allow us to fetch the underlying tables on our next query. If the
+ * view does change and the underlying tables are different, our fetch won't retrieve the
+ * correct information. But that's ok...remember what was said earlier that it is not
+ * a requirement for the tables to be part of the query. Later on in the query, this class
+ * will be called on the view level via the populateCacheForView call. At that point, if
+ * something changed, it will populate the cache with the newly detected tables. It will also
+ * change the underying table information for the view to optimize the next query using
+ * the view.
+ */
+public class CacheTableHelper {
+  protected static final Logger LOG = LoggerFactory.getLogger(CacheTableHelper.class);
+
+  // Server wide cache used to hold what we currently think are the underlying tables
+  // for a view (which is the key). This information can go stale, but that's ok. The only
+  // repercussion of a stale view is that we will have to make an additional HMS call
+  // to retrieve the validWriteIdList for the changed tables.
+  // Will hold 10000 objects, can't imagine that being more than a couple of M, tops.
+  private static final Cache<String, Set<String>> underlyingTableHints = Caffeine.newBuilder()
+      .maximumSize(10000)
+      .build();
+
+  /**
+   * Populates the cache for the given table pairs. The tables passed in are a Pair
+   * containing the dbname (null if not given) and the table name.
+   */
+  public void populateCache(List<Pair<String, String>> tables, HiveConf conf,
+      HiveTxnManager txnMgr) {
+    // if there is no transaction, then we don't need to fetch the validWriteIds.
+    if (txnMgr == null || !txnMgr.isTxnOpen()) {
+      return;
+    }
+
+    List<String> fullTableNamesList = new ArrayList<>(getAllUniqueViewsAndTables(tables));
+    LOG.debug("Populating query cache");
+    for (String s : fullTableNamesList) {
+      LOG.debug("Populating table " + s);
+    }
+    String validTxnList = conf.get(ValidTxnList.VALID_TXNS_KEY);
+    try {
+      txnMgr.getValidWriteIds(fullTableNamesList, validTxnList);
+    } catch (Exception e) {
+      LOG.info("Population of valid write id list cache failed, will be done later in query.");
+    }
+  }
+
+  /**
+   * Populates the cache for the given table pairs associated with a viewName.
+   * If the table names provided match the table names that we think are associated
+   * with the view, we just return, because we presume that they have already been
+   * popuated via the "populateCache" method. If they are different, we populate the cache
+   * with the new associated tables and change our associated tables for the view.
+   */
+  public void populateCacheForView(List<Pair<String, String>> tables, HiveConf conf,
+      HiveTxnManager txnMgr, String viewName) {
+    if (!conf.getBoolVar(ConfVars.HIVE_OPTIMIZE_VIEW_CACHE_ENABLED)) {
+      return;
+    }
+
+    // The viewname passed in is of the form "view@table", so we replace the "@" with
+    // a ".".
+    viewName = viewName.replace("@", ".");

Review comment:
       Fixed.  Comment should also have said "dbname@table"




-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org