HDDS-7984. [Snapshot] Store Ozone keys ever renamed in renamedKeyTable for efficient lookup in SnapshotDeletingService #4333
Conversation
…kup in SnapshotDeletingService
| * |----------------------------------------------------------------------| | ||
| * | snapshotInfoTable | /volume/bucket/snapshotName -> SnapshotInfo | | ||
| * |----------------------------------------------------------------------| | ||
| * | renamedKeyTable | objectID -> RepeatedString | |
There was a problem hiding this comment.
We need to also make sure that during snapshot creates, we block renames.
There was a problem hiding this comment.
Locking will be done in the follow-up jira that actually uses this table in SDT and KDT
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmString.java
Outdated
Show resolved
Hide resolved
.../interface-storage/src/main/java/org/apache/hadoop/ozone/om/codec/RepeatedOmStringCodec.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
Outdated
Show resolved
Hide resolved
| // Remove all entries from renamedKeyTable | ||
| TableIterator<Long, ? extends Table.KeyValue<Long, RepeatedOmString>> | ||
| iterator = omMetadataManager.getRenamedKeyTable().iterator(); | ||
|
|
||
| while (iterator.hasNext()) { | ||
| Long objectID = iterator.next().getKey(); | ||
| omMetadataManager.getRenamedKeyTable() | ||
| .deleteWithBatch(batchOperation, objectID); | ||
| } |
There was a problem hiding this comment.
This should be done inside createOmSnapshotCheckpoint with lock held like in #4280 . For this PR you can put a TODO here since the other PR is not merged yet (which has the locking and deleteRange implementation).
And it should only remove entries in the snapshot scope in renamedKeyTable, similar to deletedTable.
...r/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotCreateRequest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks @aswinshakil for the speedy implementaion. Structurally lgtm.
Just need the table schema change and I'm +1.
Key: Long -> String, prefixed with volume and bucket name for efficient deletion.
Value: OmKeyRenameInfo, which at least holds the original name of the key (because that is the name captured in the previous snapshot).
|
Thanks @smengcl and @prashantpogde, I have updated the PR with the suggestions. Please take a look at it. |
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyRenameInfo.java
Outdated
Show resolved
Hide resolved
...ger/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotCreateResponse.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
Outdated
Show resolved
Hide resolved
| if (omKeyRenameInfo == null) { | ||
| omKeyRenameInfo = new OmKeyRenameInfo(fromDbKey); | ||
| omMetadataManager.getRenamedKeyTable().putWithBatch( | ||
| batchOperation, renameDbKey, omKeyRenameInfo); | ||
| } |
There was a problem hiding this comment.
This means we are actually only storing the Ozone key's original name correct?
There was a problem hiding this comment.
Yes correct, We only need the original key name. In between key renames are not necessary. But keeping this to store OmKeyRenameInfo instead single String, so we can expand on it further if needed.
3 participants
What changes were proposed in this pull request?
When keys are renamed in between snapshots, it is inefficient to find the renamed keys in the previous snapshot
keyTable, This is because theTableKeyfor the renamed key has changed and it becomes impossible to find if the renamed key existed in the previous snapshot'skeyTablesince there is no way to index the keys with respect to objectID.This patch proposes to add a new column family
renamedKeyTable(objectID --> RenamedKeys) to store this information.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7984
How was this patch tested?
The patch was tested using Unit tests and CI