Skip to content

Commit 5d57f9e

Browse files
author
Venu Reddy
committed
HIVE-29528: Managed database directory not deleted after DROP DATABASE when hive.acid.lockless.reads.enabled is true
1 parent 291cfd1 commit 5d57f9e

File tree

7 files changed

+78
-3
lines changed

7 files changed

+78
-3
lines changed

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CleanupRequest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ public class CleanupRequest {
3030
private final String location;
3131
private final List<Path> obsoleteDirs;
3232
private final boolean purge;
33+
private final boolean softDelete;
34+
private final boolean sourceOfReplication;
3335
private final String runAs;
3436
private final String dbName;
3537
private final String fullPartitionName;
@@ -38,6 +40,8 @@ public CleanupRequest(CleanupRequestBuilder builder) {
3840
this.location = builder.location;
3941
this.obsoleteDirs = builder.obsoleteDirs;
4042
this.purge = builder.purge;
43+
this.softDelete = builder.isSoftDelete;
44+
this.sourceOfReplication = builder.sourceOfReplication;
4145
this.runAs = builder.runAs;
4246
this.dbName = builder.dbName;
4347
this.fullPartitionName = builder.fullPartitionName;
@@ -55,6 +59,14 @@ public boolean isPurge() {
5559
return purge;
5660
}
5761

62+
public boolean isSoftDelete() {
63+
return softDelete;
64+
}
65+
66+
public boolean isSourceOfReplication() {
67+
return sourceOfReplication;
68+
}
69+
5870
public String runAs() {
5971
return runAs;
6072
}
@@ -74,6 +86,8 @@ public static class CleanupRequestBuilder {
7486
private String location;
7587
private List<Path> obsoleteDirs;
7688
private boolean purge;
89+
private boolean isSoftDelete;
90+
private boolean sourceOfReplication;
7791
private String runAs;
7892
private String dbName;
7993
private String fullPartitionName;
@@ -93,6 +107,16 @@ public CleanupRequestBuilder setPurge(boolean purge) {
93107
return this;
94108
}
95109

110+
public CleanupRequestBuilder setSoftDelete(boolean softDelete) {
111+
this.isSoftDelete = softDelete;
112+
return this;
113+
}
114+
115+
public CleanupRequestBuilder setSourceOfReplication(boolean sourceOfReplication) {
116+
this.sourceOfReplication = sourceOfReplication;
117+
return this;
118+
}
119+
96120
public CleanupRequestBuilder setDbName(String dbName) {
97121
this.dbName = dbName;
98122
return this;

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/FSRemover.java

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
*/
1818
package org.apache.hadoop.hive.ql.txn.compactor;
1919

20+
import org.apache.hadoop.fs.FileStatus;
2021
import org.apache.hadoop.fs.FileSystem;
2122
import org.apache.hadoop.fs.Path;
23+
import org.apache.hadoop.hive.common.AcidConstants;
2224
import org.apache.hadoop.hive.conf.HiveConf;
2325
import org.apache.hadoop.hive.metastore.ReplChangeManager;
2426
import org.apache.hadoop.hive.metastore.api.Database;
@@ -99,17 +101,18 @@ private List<Path> removeFiles(CleanupRequest cr)
99101
LOG.info("About to remove {} obsolete directories from {}. {}", cr.getObsoleteDirs().size(),
100102
cr.getLocation(), CompactorUtil.getDebugInfo(cr.getObsoleteDirs()));
101103
boolean needCmRecycle;
104+
Database db = null;
102105
try {
103-
Database db = metadataCache.computeIfAbsent(cr.getDbName(),
106+
db = metadataCache.computeIfAbsent(cr.getDbName(),
104107
() -> CompactorUtil.resolveDatabase(conf, cr.getDbName()));
105108
needCmRecycle = ReplChangeManager.isSourceOfReplication(db);
106109
} catch (NoSuchObjectException ex) {
107110
// can not drop a database which is a source of replication
108-
needCmRecycle = false;
111+
needCmRecycle = cr.isSourceOfReplication();
109112
} catch (RuntimeException ex) {
110113
if (ex.getCause() instanceof NoSuchObjectException) {
111114
// can not drop a database which is a source of replication
112-
needCmRecycle = false;
115+
needCmRecycle = cr.isSourceOfReplication();
113116
} else {
114117
throw ex;
115118
}
@@ -126,6 +129,25 @@ private List<Path> removeFiles(CleanupRequest cr)
126129
deleted.add(dead);
127130
}
128131
}
132+
if (db == null && cr.isSoftDelete()) {
133+
Path databasePath = new Path(cr.getLocation()).getParent();
134+
if (canRemoveDatabaseDir(fs, databasePath)) {
135+
if (needCmRecycle) {
136+
replChangeManager.recycle(databasePath, ReplChangeManager.RecycleType.MOVE, cr.isPurge());
137+
}
138+
FileUtils.deleteDir(fs, databasePath, cr.isPurge(), conf);
139+
}
140+
}
129141
return deleted;
130142
}
143+
144+
private boolean canRemoveDatabaseDir(FileSystem fs, Path databasePath) throws IOException {
145+
for (FileStatus status : fs.listStatus(databasePath)) {
146+
if (status.isDirectory() &&
147+
status.getPath().getName().matches("(.*)" + AcidConstants.SOFT_DELETE_TABLE_PATTERN)) {
148+
return false;
149+
}
150+
}
151+
return true;
152+
}
131153
}

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/handler/CompactionCleaner.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ private CleanupRequest getCleaningRequestBasedOnLocation(CompactionInfo ci, Stri
286286
.setFullPartitionName(ci.getFullPartitionName())
287287
.setRunAs(ci.runAs)
288288
.setPurge(ifPurge)
289+
.setSoftDelete(ci.isDatabaseSoftDelete())
290+
.setSourceOfReplication(ci.isSourceOfReplication())
289291
.setObsoleteDirs(Collections.singletonList(obsoletePath))
290292
.build();
291293
}

standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/hive_metastoreConstants.java

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AcidEventListener.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
package org.apache.hadoop.hive.metastore;
2020

2121
import org.apache.hadoop.conf.Configuration;
22+
import org.apache.hadoop.hive.common.AcidConstants;
23+
import org.apache.hadoop.hive.common.repl.ReplConst;
2224
import org.apache.hadoop.hive.metastore.api.CompactionRequest;
2325
import org.apache.hadoop.hive.metastore.api.CompactionType;
2426
import org.apache.hadoop.hive.metastore.api.Database;
@@ -91,6 +93,14 @@ public void onDropTable(DropTableEvent tableEvent) throws MetaException {
9193
rqst.setRunas(TxnUtils.findUserToRunAs(table.getSd().getLocation(), table, conf));
9294
rqst.putToProperties("location", table.getSd().getLocation());
9395
rqst.putToProperties("ifPurge", Boolean.toString(isMustPurge(tableEvent.getEnvironmentContext(), table)));
96+
if ("true".equalsIgnoreCase(tableEvent.getEnvironmentContext().getProperties().get(
97+
hive_metastoreConstants.SOFT_DELETE_DATABASE))) {
98+
rqst.putToProperties(hive_metastoreConstants.SOFT_DELETE_DATABASE, Boolean.TRUE.toString());
99+
if ("true".equalsIgnoreCase(tableEvent.getEnvironmentContext().getProperties().get(
100+
ReplConst.SOURCE_OF_REPLICATION))) {
101+
rqst.putToProperties(ReplConst.SOURCE_OF_REPLICATION, Boolean.TRUE.toString());
102+
}
103+
}
94104
txnHandler.submitForCleanup(rqst, table.getWriteId(), currentTxn);
95105
} catch (InterruptedException | IOException e) {
96106
throwMetaException(e);

standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DropDatabaseHandler.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.concurrent.atomic.AtomicReference;
2929

3030
import org.apache.hadoop.fs.Path;
31+
import org.apache.hadoop.hive.common.repl.ReplConst;
3132
import org.apache.hadoop.hive.metastore.Batchable;
3233
import org.apache.hadoop.hive.metastore.HMSHandler;
3334
import org.apache.hadoop.hive.metastore.IHMSHandler;
@@ -136,6 +137,10 @@ public DropDatabaseResult execute() throws TException, IOException {
136137
if (isSoftDelete) {
137138
context = new EnvironmentContext();
138139
context.putToProperties(hive_metastoreConstants.TXN_ID, String.valueOf(request.getTxnId()));
140+
context.putToProperties(hive_metastoreConstants.SOFT_DELETE_DATABASE, Boolean.TRUE.toString());
141+
if (ReplChangeManager.isSourceOfReplication(db)) {
142+
context.putToProperties(ReplConst.SOURCE_OF_REPLICATION, Boolean.TRUE.toString());
143+
}
139144
request.setDeleteManagedDir(false);
140145
}
141146
DropTableRequest dropRequest = new DropTableRequest(name, table.getTableName());

standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/entities/CompactionInfo.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919

2020
import org.apache.commons.lang3.builder.ToStringBuilder;
2121
import org.apache.hadoop.hive.common.ValidCompactorWriteIdList;
22+
import org.apache.hadoop.hive.common.repl.ReplConst;
2223
import org.apache.hadoop.hive.metastore.api.CompactionInfoStruct;
2324
import org.apache.hadoop.hive.metastore.api.CompactionType;
25+
import org.apache.hadoop.hive.metastore.api.hive_metastoreConstants;
2426
import org.apache.hadoop.hive.metastore.api.MetaException;
2527
import org.apache.hadoop.hive.metastore.api.OptionalCompactionInfoStruct;
2628
import org.apache.hadoop.hive.metastore.api.TableValidWriteIds;
@@ -368,4 +370,12 @@ public void setWriteIds(boolean hasUncompactedAborts, Set<Long> writeIds) {
368370
public boolean isAbortedTxnCleanup() {
369371
return type == CompactionType.ABORT_TXN_CLEANUP;
370372
}
373+
374+
public boolean isDatabaseSoftDelete() {
375+
return "true".equalsIgnoreCase(getProperty(hive_metastoreConstants.SOFT_DELETE_DATABASE));
376+
}
377+
378+
public boolean isSourceOfReplication() {
379+
return "true".equalsIgnoreCase(getProperty(ReplConst.SOURCE_OF_REPLICATION));
380+
}
371381
}

0 commit comments

Comments
 (0)