Skip to content

Commit 024d57a

Browse files
infvgtdcmeehan
infvg
authored and
tdcmeehan
committed
Added iceberg.engine.hive.lock-enabled configuration
Added the `iceberg.engine.hive.lock-enabled` to enable or disable table locks when iceberg accesses a hive table. This can be overridden with the table property `engine.hive.lock-enabled`
1 parent 1d61f3e commit 024d57a

File tree

19 files changed

+307
-10
lines changed

19 files changed

+307
-10
lines changed

‎presto-docs/src/main/sphinx/connector/iceberg.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ Property Name Description
9393

9494
``iceberg.hive.table-refresh.backoff-scale-factor`` The multiple used to scale subsequent wait time between 4.0
9595
retries.
96+
97+
``iceberg.engine.hive.lock-enabled`` Whether to use locks to ensure atomicity of commits. true
98+
This will turn off locks but is overridden at a table level
99+
with the table configuration ``engine.hive.lock-enabled``.
96100
======================================================== ============================================================= ============
97101

98102
Nessie catalog

‎presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/ExtendedHiveMetastore.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Optional;
3232
import java.util.Set;
3333
import java.util.function.Function;
34+
import java.util.function.Supplier;
3435

3536
public interface ExtendedHiveMetastore
3637
{
@@ -89,6 +90,8 @@ default void dropTableFromMetastore(MetastoreContext metastoreContext, String da
8990
*/
9091
MetastoreOperationResult replaceTable(MetastoreContext metastoreContext, String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges);
9192

93+
MetastoreOperationResult persistTable(MetastoreContext metastoreContext, String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Supplier<PartitionStatistics> update, Map<String, String> additionalParameters);
94+
9295
MetastoreOperationResult renameTable(MetastoreContext metastoreContext, String databaseName, String tableName, String newDatabaseName, String newTableName);
9396

9497
MetastoreOperationResult addColumn(MetastoreContext metastoreContext, String databaseName, String tableName, String columnName, HiveType columnType, String columnComment);

‎presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/InMemoryCachingHiveMetastore.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import java.util.concurrent.ThreadLocalRandom;
5050
import java.util.function.Function;
5151
import java.util.function.Predicate;
52+
import java.util.function.Supplier;
5253

5354
import static com.facebook.presto.hive.HiveErrorCode.HIVE_CORRUPTED_PARTITION_CACHE;
5455
import static com.facebook.presto.hive.HiveErrorCode.HIVE_PARTITION_DROPPED_DURING_QUERY;
@@ -450,6 +451,18 @@ private Map<KeyAndContext<HivePartitionName>, PartitionStatistics> loadPartition
450451
return result.build();
451452
}
452453

454+
@Override
455+
public MetastoreOperationResult persistTable(MetastoreContext metastoreContext, String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Supplier<PartitionStatistics> update, Map<String, String> additionalParameters)
456+
{
457+
try {
458+
return getDelegate().persistTable(metastoreContext, databaseName, tableName, newTable, principalPrivileges, update, additionalParameters);
459+
}
460+
finally {
461+
invalidateTableCache(databaseName, tableName);
462+
invalidateTableCache(newTable.getDatabaseName(), newTable.getTableName());
463+
}
464+
}
465+
453466
@Override
454467
public void updateTableStatistics(MetastoreContext metastoreContext, String databaseName, String tableName, Function<PartitionStatistics, PartitionStatistics> update)
455468
{

‎presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/RecordingHiveMetastore.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,13 @@ public MetastoreOperationResult replaceTable(MetastoreContext metastoreContext,
315315
return delegate.replaceTable(metastoreContext, databaseName, tableName, newTable, principalPrivileges);
316316
}
317317

318+
@Override
319+
public MetastoreOperationResult persistTable(MetastoreContext metastoreContext, String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Supplier<PartitionStatistics> update, Map<String, String> additionalParameters)
320+
{
321+
verifyRecordingMode();
322+
return delegate.persistTable(metastoreContext, databaseName, tableName, newTable, principalPrivileges, update, additionalParameters);
323+
}
324+
318325
@Override
319326
public MetastoreOperationResult renameTable(MetastoreContext metastoreContext, String databaseName, String tableName, String newDatabaseName, String newTableName)
320327
{

‎presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/file/FileHiveMetastore.java‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
import java.util.Optional;
8787
import java.util.Set;
8888
import java.util.function.Function;
89+
import java.util.function.Supplier;
8990

9091
import static com.facebook.presto.hive.HiveErrorCode.HIVE_METASTORE_ERROR;
9192
import static com.facebook.presto.hive.HiveErrorCode.HIVE_PARTITION_DROPPED_DURING_QUERY;
@@ -134,6 +135,7 @@ public class FileHiveMetastore
134135

135136
protected final HdfsEnvironment hdfsEnvironment;
136137
protected final HdfsContext hdfsContext;
138+
137139
protected final FileSystem metadataFileSystem;
138140

139141
private final Path catalogDirectory;
@@ -477,6 +479,44 @@ public synchronized MetastoreOperationResult replaceTable(MetastoreContext metas
477479
return EMPTY_RESULT;
478480
}
479481

482+
@Override
483+
public synchronized MetastoreOperationResult persistTable(
484+
MetastoreContext metastoreContext,
485+
String databaseName,
486+
String tableName,
487+
Table newTable,
488+
PrincipalPrivileges principalPrivileges,
489+
Supplier<PartitionStatistics> update,
490+
Map<String, String> additionalParameters)
491+
{
492+
checkArgument(!newTable.getTableType().equals(TEMPORARY_TABLE), "temporary tables must never be stored in the metastore");
493+
494+
Table oldTable = getRequiredTable(metastoreContext, databaseName, tableName);
495+
validateReplaceTableType(oldTable, newTable);
496+
if (!oldTable.getDatabaseName().equals(databaseName) || !oldTable.getTableName().equals(tableName)) {
497+
throw new PrestoException(HIVE_METASTORE_ERROR, "Replacement table must have same name");
498+
}
499+
500+
Path tableMetadataDirectory = getTableMetadataDirectory(oldTable);
501+
502+
deleteTablePrivileges(oldTable);
503+
for (Entry<String, Collection<HivePrivilegeInfo>> entry : principalPrivileges.getUserPrivileges().asMap().entrySet()) {
504+
setTablePrivileges(metastoreContext, new PrestoPrincipal(USER, entry.getKey()), databaseName, tableName, entry.getValue());
505+
}
506+
for (Entry<String, Collection<HivePrivilegeInfo>> entry : principalPrivileges.getRolePrivileges().asMap().entrySet()) {
507+
setTablePrivileges(metastoreContext, new PrestoPrincipal(ROLE, entry.getKey()), databaseName, tableName, entry.getValue());
508+
}
509+
PartitionStatistics updatedStatistics = update.get();
510+
511+
TableMetadata updatedMetadata = new TableMetadata(newTable)
512+
.withParameters(updateStatisticsParameters(newTable.getParameters(), updatedStatistics.getBasicStatistics()))
513+
.withColumnStatistics(updatedStatistics.getColumnStatistics());
514+
515+
writeSchemaFile("table", tableMetadataDirectory, tableCodec, updatedMetadata, true);
516+
517+
return EMPTY_RESULT;
518+
}
519+
480520
@Override
481521
public synchronized MetastoreOperationResult renameTable(MetastoreContext metastoreContext, String databaseName, String tableName, String newDatabaseName, String newTableName)
482522
{
@@ -1292,7 +1332,6 @@ private List<Path> getChildSchemaDirectories(Path metadataDirectory)
12921332
if (!metadataFileSystem.isDirectory(metadataDirectory)) {
12931333
return ImmutableList.of();
12941334
}
1295-
12961335
ImmutableList.Builder<Path> childSchemaDirectories = ImmutableList.builder();
12971336
for (FileStatus child : metadataFileSystem.listStatus(metadataDirectory)) {
12981337
if (!child.isDirectory()) {

‎presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/glue/GlueHiveMetastore.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@
116116
import java.util.concurrent.ExecutorCompletionService;
117117
import java.util.concurrent.Future;
118118
import java.util.function.Function;
119+
import java.util.function.Supplier;
119120

120121
import static com.facebook.presto.hive.HiveErrorCode.HIVE_METASTORE_ERROR;
121122
import static com.facebook.presto.hive.HiveErrorCode.HIVE_PARTITION_DROPPED_DURING_QUERY;
@@ -589,6 +590,30 @@ public MetastoreOperationResult replaceTable(MetastoreContext metastoreContext,
589590
}
590591
}
591592

593+
public MetastoreOperationResult persistTable(MetastoreContext metastoreContext, String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Supplier<PartitionStatistics> update, Map<String, String> additionalParameters)
594+
{
595+
PartitionStatistics updatedStatistics = update.get();
596+
if (!updatedStatistics.getColumnStatistics().isEmpty()) {
597+
throw new PrestoException(NOT_SUPPORTED, "Glue metastore does not support column level statistics");
598+
}
599+
try {
600+
TableInput newTableInput = GlueInputConverter.convertTable(newTable);
601+
newTableInput.setParameters(updateStatisticsParameters(newTableInput.getParameters(), updatedStatistics.getBasicStatistics()));
602+
stats.getUpdateTable().record(() -> glueClient.updateTable(new UpdateTableRequest()
603+
.withCatalogId(catalogId)
604+
.withDatabaseName(databaseName)
605+
.withTableInput(newTableInput)));
606+
607+
return EMPTY_RESULT;
608+
}
609+
catch (EntityNotFoundException e) {
610+
throw new TableNotFoundException(new SchemaTableName(databaseName, tableName));
611+
}
612+
catch (AmazonServiceException e) {
613+
throw new PrestoException(HIVE_METASTORE_ERROR, e);
614+
}
615+
}
616+
592617
@Override
593618
public MetastoreOperationResult renameTable(MetastoreContext metastoreContext, String databaseName, String tableName, String newDatabaseName, String newTableName)
594619
{

‎presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/thrift/BridgingHiveMetastore.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,18 @@
4545
import com.facebook.presto.spi.statistics.ColumnStatisticType;
4646
import com.google.common.collect.ImmutableList;
4747
import com.google.common.collect.ImmutableMap;
48+
import com.google.common.collect.Maps;
4849
import jakarta.inject.Inject;
50+
import org.apache.hadoop.hive.common.StatsSetupConst;
51+
import org.apache.hadoop.hive.metastore.api.EnvironmentContext;
4952
import org.apache.hadoop.hive.metastore.api.FieldSchema;
5053

5154
import java.util.List;
5255
import java.util.Map;
5356
import java.util.Optional;
5457
import java.util.Set;
5558
import java.util.function.Function;
59+
import java.util.function.Supplier;
5660
import java.util.stream.Collectors;
5761

5862
import static com.facebook.presto.hive.metastore.MetastoreUtil.getPartitionNamesWithEmptyVersion;
@@ -276,6 +280,21 @@ private MetastoreOperationResult alterTable(MetastoreContext metastoreContext, S
276280
return delegate.alterTable(metastoreContext, databaseName, tableName, table);
277281
}
278282

283+
private MetastoreOperationResult alterTableWithEnvironmentContext(MetastoreContext metastoreContext, String databaseName, String tableName, org.apache.hadoop.hive.metastore.api.Table table, EnvironmentContext environmentContext)
284+
{
285+
return delegate.alterTableWithEnvironmentContext(metastoreContext, databaseName, tableName, table, environmentContext);
286+
}
287+
288+
@Override
289+
public MetastoreOperationResult persistTable(MetastoreContext metastoreContext, String databaseName, String tableName, Table newTable, PrincipalPrivileges principalPrivileges, Supplier<PartitionStatistics> update, Map<String, String> additionalParameters)
290+
{
291+
checkArgument(!newTable.getTableType().equals(TEMPORARY_TABLE), "temporary tables must never be stored in the metastore");
292+
Map<String, String> env = Maps.newHashMapWithExpectedSize(additionalParameters.size() + 1);
293+
env.putAll(additionalParameters);
294+
env.put(StatsSetupConst.DO_NOT_UPDATE_STATS, StatsSetupConst.TRUE);
295+
return alterTableWithEnvironmentContext(metastoreContext, databaseName, tableName, toMetastoreApiTable(newTable, principalPrivileges, metastoreContext.getColumnConverter()), new EnvironmentContext(env));
296+
}
297+
279298
@Override
280299
public Optional<Partition> getPartition(MetastoreContext metastoreContext, String databaseName, String tableName, List<String> partitionValues)
281300
{

‎presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/thrift/HiveMetastore.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import com.facebook.presto.spi.statistics.ColumnStatisticType;
3737
import com.google.common.collect.ImmutableList;
3838
import org.apache.hadoop.hive.metastore.api.Database;
39+
import org.apache.hadoop.hive.metastore.api.EnvironmentContext;
3940
import org.apache.hadoop.hive.metastore.api.FieldSchema;
4041
import org.apache.hadoop.hive.metastore.api.Partition;
4142
import org.apache.hadoop.hive.metastore.api.Table;
@@ -62,6 +63,8 @@ public interface HiveMetastore
6263

6364
MetastoreOperationResult alterTable(MetastoreContext metastoreContext, String databaseName, String tableName, Table table);
6465

66+
MetastoreOperationResult alterTableWithEnvironmentContext(MetastoreContext metastoreContext, String databaseName, String tableName, Table table, EnvironmentContext environmentContext);
67+
6568
default List<String> getDatabases(MetastoreContext metastoreContext, String pattern)
6669
{
6770
return getAllDatabases(metastoreContext);

‎presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/thrift/HiveMetastoreClient.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.apache.hadoop.hive.metastore.api.CheckLockRequest;
1717
import org.apache.hadoop.hive.metastore.api.ColumnStatisticsObj;
1818
import org.apache.hadoop.hive.metastore.api.Database;
19+
import org.apache.hadoop.hive.metastore.api.EnvironmentContext;
1920
import org.apache.hadoop.hive.metastore.api.FieldSchema;
2021
import org.apache.hadoop.hive.metastore.api.HiveObjectPrivilege;
2122
import org.apache.hadoop.hive.metastore.api.HiveObjectRef;
@@ -86,6 +87,9 @@ void dropTable(String databaseName, String name, boolean deleteData)
8687
void alterTable(String databaseName, String tableName, Table newTable)
8788
throws TException;
8889

90+
void alterTableWithEnvironmentContext(String databaseName, String tableName, Table newTable, EnvironmentContext context)
91+
throws TException;
92+
8993
Table getTable(String databaseName, String tableName)
9094
throws TException;
9195

‎presto-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/thrift/ThriftHiveMetastore.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import org.apache.hadoop.hive.metastore.api.CheckLockRequest;
6565
import org.apache.hadoop.hive.metastore.api.ColumnStatisticsObj;
6666
import org.apache.hadoop.hive.metastore.api.Database;
67+
import org.apache.hadoop.hive.metastore.api.EnvironmentContext;
6768
import org.apache.hadoop.hive.metastore.api.FieldSchema;
6869
import org.apache.hadoop.hive.metastore.api.HiveObjectPrivilege;
6970
import org.apache.hadoop.hive.metastore.api.HiveObjectRef;
@@ -1160,6 +1161,35 @@ public MetastoreOperationResult alterTable(MetastoreContext metastoreContext, St
11601161
}
11611162
}
11621163

1164+
@Override
1165+
public MetastoreOperationResult alterTableWithEnvironmentContext(MetastoreContext metastoreContext, String databaseName, String tableName, Table table, EnvironmentContext environmentContext)
1166+
{
1167+
try {
1168+
retry()
1169+
.stopOn(InvalidOperationException.class, MetaException.class)
1170+
.stopOnIllegalExceptions()
1171+
.run("alterTableWithEnvironmentContext", stats.getAlterTableWithEnvironmentContext().wrap(() ->
1172+
getMetastoreClientThenCall(metastoreContext, client -> {
1173+
Optional<Table> source = getTable(metastoreContext, databaseName, tableName);
1174+
if (!source.isPresent()) {
1175+
throw new TableNotFoundException(new SchemaTableName(databaseName, tableName));
1176+
}
1177+
client.alterTableWithEnvironmentContext(databaseName, tableName, table, environmentContext);
1178+
return null;
1179+
})));
1180+
return EMPTY_RESULT;
1181+
}
1182+
catch (NoSuchObjectException e) {
1183+
throw new TableNotFoundException(new SchemaTableName(databaseName, tableName));
1184+
}
1185+
catch (TException e) {
1186+
throw new PrestoException(HIVE_METASTORE_ERROR, e);
1187+
}
1188+
catch (Exception e) {
1189+
throw propagate(e);
1190+
}
1191+
}
1192+
11631193
@Override
11641194
public Optional<List<String>> getPartitionNames(MetastoreContext metastoreContext, String databaseName, String tableName)
11651195
{

0 commit comments

Comments
 (0)