wecharyu commented on code in PR #6935:
URL: https://github.com/apache/hive/pull/6935#discussion_r4508982690
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3826,7 +3826,8 @@ public void testAlterTableCascadeExceedsPartitionLimits()
throws Throwable {
List<FieldSchema> cols = tbl.getSd().getCols();
cols.add(new FieldSchema("new_col", ColumnType.STRING_TYPE_NAME, ""));
tbl.getSd().setCols(cols);
- //add new column with cascade option
+ // With PartitionsIterable, alter_table doesn't load all partitions into
+ // memory, it will change the partition's schema in bulk.
Review Comment:
Should we still throw exception when the configured batch_size is greater
than the `hive.metastore.limit.partition.request`? If so we can change the
config to get the expected `MetaException`.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/PartitionsIterable.java:
##########
@@ -0,0 +1,182 @@
+/*
+ * 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.metastore.metastore;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.RawStore;
+import org.apache.hadoop.hive.metastore.api.GetPartitionsFilterSpec;
+import org.apache.hadoop.hive.metastore.api.GetPartitionsRequest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Partition;
+import org.apache.hadoop.hive.metastore.api.PartitionFilterMode;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.client.builder.GetPartitionsArgs;
+import org.apache.hadoop.hive.metastore.metastore.iface.TableStore;
+
+/**
+ * PartitionsIterable - effectively a lazy
Iterable<List<Partition>>
+ * Similar to {@link org.apache.hadoop.hive.metastore.PartitionIterable}, but
+ * each iteration step returns up to {@code batchSize} partitions. Because a
metastore
+ * lookup by name can return fewer objects than names requested (missing
partitions),
+ * the iterator may perform multiple fetches in one {@code next()} until the
batch
+ * is full or partition names are exhausted.
+ * Note that remove() is an illegal call on this, and will result in an
+ * IllegalStateException.
+ */
+public class PartitionsIterable implements Iterable<List<Partition>> {
+
+ private final TableStore tableStore;
+ private final int batchSize;
+ private final TableName tableName;
+ private final List<String> partNames;
+ private GetPartitionsRequest request;
+ private Boolean skipColumnSchemaForPartition;
+
+ @Override
+ public Iterator<List<Partition>> iterator() {
+ if (partNames.isEmpty()) {
+ return Collections.emptyIterator();
+ }
+
+ return new Iterator<>() {
+ private final Iterator<String> partitionNamesIter = partNames.iterator();
+ private Table table;
+ private List<Partition> leftParts;
+
+ @Override
+ public boolean hasNext() {
+ return hasPendingPartitions() || partitionNamesIter.hasNext();
+ }
+
+ private boolean hasPendingPartitions() {
+ return leftParts != null && !leftParts.isEmpty();
+ }
+
+ @Override
+ public List<Partition> next() {
+ if (!hasNext()) {
+ throw new NoSuchElementException();
+ }
+ List<Partition> result = new ArrayList<>(batchSize);
+ if (hasPendingPartitions()) {
+ appendPartitions(result, leftParts);
+ leftParts = null;
+ }
+ while (result.size() < batchSize && partitionNamesIter.hasNext()) {
+ List<String> nameBatch = nextNameBatch();
+ appendPartitions(result, fetchBatch(nameBatch));
+ }
+ return result;
+ }
+
+ private void appendPartitions(List<Partition> result, List<Partition>
fetched) {
+ if (fetched.isEmpty()) {
+ return;
+ }
+ int space = batchSize - result.size();
+ if (fetched.size() <= space) {
+ result.addAll(fetched);
+ } else {
+ result.addAll(fetched.subList(0, space));
+ leftParts = new ArrayList<>(fetched.subList(space, fetched.size()));
+ }
+ }
+
+ private List<String> nextNameBatch() {
+ List<String> nameBatch = new ArrayList<>(batchSize);
+ for (int i = 0; i < batchSize && partitionNamesIter.hasNext(); i++) {
+ nameBatch.add(partitionNamesIter.next());
+ }
+ return nameBatch;
+ }
+
+ private List<Partition> fetchBatch(List<String> nameBatch) {
+ try {
+ List<Partition> partitions;
+ if (request != null) {
+ if (table == null) {
+ table = tableStore.getTable(tableName, null, -1);
+ }
+ GetPartitionsFilterSpec filterSpec = new GetPartitionsFilterSpec();
+ filterSpec.setFilterMode(PartitionFilterMode.BY_NAMES);
+ filterSpec.setFilters(nameBatch);
+ partitions = tableStore.getPartitionSpecsByFilterAndProjection(
+ table, request.getProjectionSpec(), filterSpec);
+ } else {
+ GetPartitionsArgs.GetPartitionsArgsBuilder argsBuilder =
+ new
GetPartitionsArgs.GetPartitionsArgsBuilder().partNames(nameBatch);
+ if (skipColumnSchemaForPartition != null) {
+
argsBuilder.skipColumnSchemaForPartition(skipColumnSchemaForPartition);
+ }
+ partitions = tableStore.getPartitionsByNames(tableName,
argsBuilder.build());
+ }
+ return partitions == null ? Collections.emptyList() : partitions;
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ @Override
+ public void remove() {
+ throw new IllegalStateException(
+ "PartitionsIterable is a read-only iterable and remove() is
unsupported");
+ }
+ };
+ }
+
+ private PartitionsIterable(TableStore tableStore,
+ TableName tableName,
+ int batchSize,
+ List<String> partNames) {
+ this.tableStore = Objects.requireNonNull(tableStore);
+ this.tableName = Objects.requireNonNull(tableName);
+ this.partNames = Objects.requireNonNull(partNames);
+ this.batchSize = batchSize < 0 ? 1000 : batchSize;
+ }
+
+ public PartitionsIterable withProjectSpec(GetPartitionsRequest request) {
+ this.request = request;
+ return this;
+ }
+
+ public PartitionsIterable withSkipColumnSchemaForPartition(boolean
skipColumnSchemaForPartition) {
+ this.skipColumnSchemaForPartition = skipColumnSchemaForPartition;
+ return this;
+ }
Review Comment:
`withProjectSpec()` and `withSkipColumnSchemaForPartition()` aren't called
anywhere outside the tests, could we drop them?
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/TruncateTableHandler.java:
##########
@@ -142,112 +122,171 @@ protected TruncateTableResult execute() throws
TException, IOException {
.map(Boolean::parseBoolean)
.orElse(false);
- if (TxnUtils.isTransactionalTable(table) || !skipDataDeletion) {
- if (!skipDataDeletion) {
- isSkipTrash = MetaStoreUtils.isSkipTrash(table.getParameters());
- Database db = handler.get_database_core(catName, dbName);
- needCmRecycle = ReplChangeManager.shouldEnableCm(db, table);
- }
- List<Path> locations = new ArrayList<>();
- if (0 != table.getPartitionKeysSize()) {
- for (Partition partition : partitions) {
- locations.add(new Path(partition.getSd().getLocation()));
+ boolean success;
+ if (isPartitioned) {
+ success = truncatePartitionedTable(isSkipTrash, needCmRecycle,
skipDataDeletion);
Review Comment:
`truncatePartitionedTable` only ever receives false for
`isSkipTrash`/`needCmRecycle` and recomputes them itself, so they don't need to
be parameters.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/TruncateTableHandler.java:
##########
@@ -142,112 +122,171 @@ protected TruncateTableResult execute() throws
TException, IOException {
.map(Boolean::parseBoolean)
.orElse(false);
- if (TxnUtils.isTransactionalTable(table) || !skipDataDeletion) {
- if (!skipDataDeletion) {
- isSkipTrash = MetaStoreUtils.isSkipTrash(table.getParameters());
- Database db = handler.get_database_core(catName, dbName);
- needCmRecycle = ReplChangeManager.shouldEnableCm(db, table);
- }
- List<Path> locations = new ArrayList<>();
- if (0 != table.getPartitionKeysSize()) {
- for (Partition partition : partitions) {
- locations.add(new Path(partition.getSd().getLocation()));
+ boolean success;
+ if (isPartitioned) {
+ success = truncatePartitionedTable(isSkipTrash, needCmRecycle,
skipDataDeletion);
+ } else {
+ if (TxnUtils.isTransactionalTable(table) || !skipDataDeletion) {
+ if (!skipDataDeletion) {
+ isSkipTrash = MetaStoreUtils.isSkipTrash(table.getParameters());
+ Database db = handler.get_database_core(catName, dbName);
+ needCmRecycle = ReplChangeManager.shouldEnableCm(db, table);
}
- } else {
- locations.add(new Path(table.getSd().getLocation()));
- }
- // This is not transactional
- for (Path location : locations) {
+ Path location = new Path(table.getSd().getLocation());
if (!skipDataDeletion) {
truncateDataFiles(location, isSkipTrash, needCmRecycle);
} else {
- // For Acid tables we don't need to delete the old files, only write
an empty baseDir.
- // Compaction and cleaner will take care of the rest
addTruncateBaseFile(location, request.getWriteId(),
handler.getConf(), AcidMetaDataFile.DataFormat.TRUNCATED);
}
}
- }
- // Alter the table/partition stats and also notify truncate table event
- boolean success;
- if (isPartitioned) {
- success = alterPartitionsForTruncate();
- } else {
success = alterTableStatsForTruncate();
}
return new TruncateTableResult(success);
}
- private void updateStatsForTruncate(Map<String,String> props,
EnvironmentContext environmentContext) {
- if (null == props) {
- return;
+ private PartitionsIterable newPartitionBatches(int batchSize) throws
MetaException {
+ TableName qualifiedTable = new TableName(catName, dbName,
request.getTableName());
+ if (reqPartNames != null) {
+ return PartitionsIterable.create(
+ ms.unwrap(TableStore.class), qualifiedTable, batchSize,
reqPartNames);
}
- for (String stat : StatsSetupConst.SUPPORTED_STATS) {
- String statVal = props.get(stat);
- if (statVal != null) {
- //In the case of truncate table, we set the stats to be 0.
- props.put(stat, "0");
+ return PartitionsIterable.create(ms, qualifiedTable, batchSize);
+ }
+
+ private boolean truncatePartitionedTable(boolean isSkipTrash, boolean
needCmRecycle, boolean skipDataDeletion)
+ throws TException, IOException {
+ if (reqPartNames != null && reqPartNames.isEmpty()) {
+ return true;
+ }
+ int batchSize = MetastoreConf.getIntVar(handler.getConf(),
+ MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX);
+ PartitionsIterable partitionBatches = newPartitionBatches(batchSize);
+
+ boolean truncatePartitionData = TxnUtils.isTransactionalTable(table) ||
!skipDataDeletion;
+ if (truncatePartitionData && !skipDataDeletion) {
+ isSkipTrash = MetaStoreUtils.isSkipTrash(table.getParameters());
+ Database db = handler.get_database_core(catName, dbName);
+ needCmRecycle = ReplChangeManager.shouldEnableCm(db, table);
Review Comment:
After the split, the `isSkipTrash`/`needCmRecycle` setup and the
`truncateDataFiles`/`addTruncateBaseFile` branch are duplicated between the
partitioned and non-partitioned paths. Could we compute these once in
`execute()` and share a `truncateLocation(Path, ...)` helper?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
