Skip to content

HDDS-8067. [Snapshot] Revisit locks on deletedTable and deletedDirTable #4701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
smengcl merged 2 commits into from
May 18, 2023
Merged

HDDS-8067. [Snapshot] Revisit locks on deletedTable and deletedDirTable #4701

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd753d4
HDDS-8067. [Snapshot] Revisit locks on deletedTable and deletedDirTable
smengcl May 11, 2023
c97fe4c
Remove deletedTable lock in DDT.
smengcl May 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,6 @@ protected void initializeOmTables(boolean addCacheMetrics)
deletedTable = this.store.getTable(DELETED_TABLE, String.class,
RepeatedOmKeyInfo.class);
checkTableStatus(deletedTable, DELETED_TABLE, addCacheMetrics);
// Currently, deletedTable is the only table that will need the table lock
tableLockMap.put(DELETED_TABLE, new ReentrantReadWriteLock(true));

openKeyTable =
Expand Down Expand Up @@ -675,6 +674,7 @@ protected void initializeOmTables(boolean addCacheMetrics)
deletedDirTable = this.store.getTable(DELETED_DIR_TABLE, String.class,
OmKeyInfo.class);
checkTableStatus(deletedDirTable, DELETED_DIR_TABLE, addCacheMetrics);
tableLockMap.put(DELETED_DIR_TABLE, new ReentrantReadWriteLock(true));

transactionInfoTable = this.store.getTable(TRANSACTION_INFO_TABLE,
String.class, TransactionInfo.class);
Expand Down
11 changes: 8 additions & 3 deletions hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,10 +380,13 @@ public static DBCheckpoint createOmSnapshotCheckpoint(

final DBCheckpoint dbCheckpoint;

// Acquire deletedTable write lock
// Acquire active DB deletedDirectoryTable write lock
omMetadataManager.getTableLock(OmMetadataManagerImpl.DELETED_DIR_TABLE)
.writeLock().lock();
// Acquire active DB deletedTable write lock
omMetadataManager.getTableLock(OmMetadataManagerImpl.DELETED_TABLE)
.writeLock().lock();
// TODO: [SNAPSHOT] HDDS-8067. Acquire deletedDirectoryTable write lock

try {
// Create DB checkpoint for snapshot
dbCheckpoint = store.getSnapshot(snapshotInfo.getCheckpointDirName());
Expand All @@ -395,10 +398,12 @@ public static DBCheckpoint createOmSnapshotCheckpoint(
deleteKeysFromDelDirTableInSnapshotScope(omMetadataManager,
snapshotInfo.getVolumeName(), snapshotInfo.getBucketName());
} finally {
// TODO: [SNAPSHOT] HDDS-8067. Release deletedDirectoryTable write lock
// Release deletedTable write lock
omMetadataManager.getTableLock(OmMetadataManagerImpl.DELETED_TABLE)
.writeLock().unlock();
// Release deletedDirectoryTable write lock
omMetadataManager.getTableLock(OmMetadataManagerImpl.DELETED_DIR_TABLE)
.writeLock().unlock();
}

LOG.info("Created checkpoint : {} for snapshot {}",
Expand Down
26 changes: 21 additions & 5 deletions ...ne-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ public BackgroundTaskResult call() {
List<Pair<String, OmKeyInfo>> allSubDirList
= new ArrayList<>((int) remainNum);

// TODO: [SNAPSHOT] HDDS-8067. Acquire deletedDirectoryTable write lock
// Acquire active DB deletedDirectoryTable write lock
getOzoneManager().getMetadataManager().getTableLock(
OmMetadataManagerImpl.DELETED_DIR_TABLE).writeLock().lock();

Table.KeyValue<String, OmKeyInfo> pendingDeletedDirInfo;
try (TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
Expand Down Expand Up @@ -154,15 +156,29 @@ public BackgroundTaskResult call() {
subFileNum += request.getDeletedSubFilesCount();
}

optimizeDirDeletesAndSubmitRequest(remainNum, dirNum, subDirNum,
subFileNum, allSubDirList, purgePathRequestList, null, startTime);
// Acquire deletedTable write lock this late to allow KeyDeletingTask
// to interleave up until this point
getOzoneManager().getMetadataManager().getTableLock(
Comment thread
smengcl marked this conversation as resolved.
Outdated
OmMetadataManagerImpl.DELETED_TABLE).writeLock().lock();

try {
optimizeDirDeletesAndSubmitRequest(
remainNum, dirNum, subDirNum, subFileNum,
allSubDirList, purgePathRequestList, null, startTime);
} finally {
// Release deletedTable write lock
getOzoneManager().getMetadataManager().getTableLock(
OmMetadataManagerImpl.DELETED_TABLE).writeLock().unlock();
}

} catch (IOException e) {
LOG.error("Error while running delete directories and files " +
"background task. Will retry at next run.", e);
} finally {
// Release deletedDirectoryTable write lock
getOzoneManager().getMetadataManager().getTableLock(
OmMetadataManagerImpl.DELETED_DIR_TABLE).writeLock().unlock();
}
// TODO: [SNAPSHOT] HDDS-8067. Release deletedDirectoryTable write lock
// in finally block
}

// place holder by returning empty results of this call back.
Expand Down
16 changes: 11 additions & 5 deletions ...ne/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol;
import org.apache.hadoop.ozone.common.BlockGroup;
import org.apache.hadoop.ozone.om.KeyManager;
import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.hdds.utils.BackgroundTask;
import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
Expand Down Expand Up @@ -118,15 +119,18 @@ public BackgroundTaskResult call() {
// task.
if (shouldRun()) {
getRunCount().incrementAndGet();

// Acquire active DB deletedTable write lock
manager.getMetadataManager().getTableLock(
OmMetadataManagerImpl.DELETED_TABLE).writeLock().lock();

try {
// TODO: [SNAPSHOT] HDDS-7968. Reclaim eligible key blocks in
// snapshot's deletedTable when active DB's deletedTable
// doesn't have enough entries left.
// OM would have to keep track of which snapshot the key is coming
// from. And PurgeKeysRequest would have to be adjusted to be able
// to operate on snapshot checkpoints.
// from if the above would be done inside getPendingDeletionKeys().

// TODO: [SNAPSHOT] HDDS-8067. Acquire deletedTable write lock
List<BlockGroup> keyBlocksList = manager
.getPendingDeletionKeys(keyLimitPerTask);
if (keyBlocksList != null && !keyBlocksList.isEmpty()) {
Expand All @@ -137,9 +141,11 @@ public BackgroundTaskResult call() {
} catch (IOException e) {
LOG.error("Error while running delete keys background task. Will " +
"retry at next run.", e);
} finally {
// Release deletedTable write lock
manager.getMetadataManager().getTableLock(
OmMetadataManagerImpl.DELETED_TABLE).writeLock().unlock();
}
// TODO: [SNAPSHOT] HDDS-8067. Release deletedTable write lock
// in finally block
}
// By design, no one cares about the results of this call back.
return EmptyTaskResult.newResult();
Expand Down