symious commented on code in PR #10937:
URL: https://github.com/apache/ozone/pull/10937#discussion_r3834584021
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -697,6 +719,48 @@ private OmKeyInfo getOmKeyInfo(String volumeName, String
bucketName,
.get(keyBytes);
}
+ /**
+ * Resolves the version addressed by {@code args} for a key whose current
+ * version is {@code current}. The current version is checked first, so a
+ * request naming the current version costs no extra read; otherwise the
+ * version is looked up in the versionedKeyTable.
+ *
+ * @param current the key's current version, or null when the key has none
+ * @return the addressed version, or null when it does not exist
+ */
+ private OmKeyInfo getAddressedVersion(OmKeyArgs args, String volumeName,
+ String bucketName, String keyName, OmKeyInfo current) throws IOException
{
+ if (args.isNullVersion()) {
+ if (current != null && current.isNullVersion()) {
+ return current;
+ }
+ // The null version carries a normally generated versionId, so it can
only
+ // be found by scanning the key's versions. The scan is bounded by the
+ // number of versions the key has and stops at the first match, since a
key
+ // has at most one null version.
+ String prefix = metadataManager
+ .getVersionedOzoneKeyPrefix(volumeName, bucketName, keyName);
+ try (Table.KeyValueIterator<String, OmKeyInfo> versions =
+ metadataManager.getVersionedKeyTable().iterator(prefix)) {
+ while (versions.hasNext()) {
+ OmKeyInfo version = versions.next().getValue();
+ if (version.isNullVersion()) {
+ return version;
+ }
+ }
+ }
+ return null;
Review Comment:
@rich7420 Thank you for the review.
The cache-blindness is real — fixed.
But the null version isn't always UNSET_VERSION_ID: a record written while
versioning was SUSPENDED carries a normally allocated id plus isNullVersion
(`setNullVersion(suspendedWrite)` in OMKeyCommitRequest), and keeps that id
when it's later demoted. `.get(UNSET_VERSION_ID)` would miss it —
TestKeyManagerUnit#testLookupKeyByVersionId seeds the null version at id 10.
So the search by attribute stays; it just has to consult the cache too. Now
reuses the cache-aware search the write paths already had, extracted into
`NoncurrentVersions` so there's one copy.
Two regression tests added.
--
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]
