You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2025/10/02 20:02:07 UTC
[PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
swamirishi opened a new pull request, #9089:
URL: https://github.com/apache/ozone/pull/9089
## What changes were proposed in this pull request?
Currently there are no traces of bucket information in deletedKeyTable. However a bucket could be deleted and recreated with the same name even having leftover keys from older bucket in the deletedTable and the bucket quota could be erroneously be updated for the newer bucket when updated with just the bucket name check. Thus for each RepeatedKeyInfo in deletedTable should have bucketId which would notify whether a bucket quota needs to be updated or not.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13733
## How was this patch tested?
Updated existing unit tests
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2408570133
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/OzoneRpcClientTests.java:
##########
@@ -4431,7 +4431,7 @@ public void testKeyReadWriteForGDPR() throws Exception {
verifyReplication(volumeName, bucketName, keyName,
RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.ONE));
}
-
+ ozoneManager.awaitDoubleBufferFlush();
Review Comment:
This line appears to be added without context or explanation. The change seems unrelated to the bucket ID tracking functionality being implemented in this PR.
```suggestion
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409044918
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -531,14 +531,15 @@ public static File createOMDir(String dirPath) {
* repeatedOmKeyInfo instance.
* 3. Set the updateID to the transactionLogIndex.
* @param keyInfo args supplied by client
+ * @param bucketId bucket id
* @param trxnLogIndex For Multipart keys, this is the transactionLogIndex
* of the MultipartUploadAbort request which needs to
* be set as the updateID of the partKeyInfos.
* For regular Key deletes, this value should be set to
* the same updateID as is in keyInfo.
* @return {@link RepeatedOmKeyInfo}
*/
- public static RepeatedOmKeyInfo prepareKeyForDelete(OmKeyInfo keyInfo,
+ public static RepeatedOmKeyInfo prepareKeyForDelete(long bucketId, OmKeyInfo keyInfo,
Review Comment:
I thought it should be reverse since bucket comes hierarchically before a key
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2410649842
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotMoveDeletedKeysResponse.java:
##########
@@ -52,21 +52,25 @@ public class OMSnapshotMoveDeletedKeysResponse extends OMClientResponse {
private List<SnapshotMoveKeyInfos> reclaimKeysList;
private List<HddsProtos.KeyValue> renamedKeysList;
private List<String> movedDirs;
+ private long bucketId;
+ @SuppressWarnings("checkstyle:ParameterNumber")
Review Comment:
I created a follow up jira for this.
https://issues.apache.org/jira/browse/HDDS-13757
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9089:
URL: https://github.com/apache/ozone/pull/9089#issuecomment-3376918620
thank you @smengcl and @priyeshkaratha for reviewing the patch
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409012224
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -531,14 +531,15 @@ public static File createOMDir(String dirPath) {
* repeatedOmKeyInfo instance.
* 3. Set the updateID to the transactionLogIndex.
* @param keyInfo args supplied by client
+ * @param bucketId bucket id
* @param trxnLogIndex For Multipart keys, this is the transactionLogIndex
* of the MultipartUploadAbort request which needs to
* be set as the updateID of the partKeyInfos.
* For regular Key deletes, this value should be set to
* the same updateID as is in keyInfo.
* @return {@link RepeatedOmKeyInfo}
*/
- public static RepeatedOmKeyInfo prepareKeyForDelete(OmKeyInfo keyInfo,
+ public static RepeatedOmKeyInfo prepareKeyForDelete(long bucketId, OmKeyInfo keyInfo,
Review Comment:
nit: order of argument
```suggestion
public static RepeatedOmKeyInfo prepareKeyForDelete(OmKeyInfo keyInfo, long bucketId,
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409083392
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -531,14 +531,15 @@ public static File createOMDir(String dirPath) {
* repeatedOmKeyInfo instance.
* 3. Set the updateID to the transactionLogIndex.
* @param keyInfo args supplied by client
+ * @param bucketId bucket id
* @param trxnLogIndex For Multipart keys, this is the transactionLogIndex
* of the MultipartUploadAbort request which needs to
* be set as the updateID of the partKeyInfos.
* For regular Key deletes, this value should be set to
* the same updateID as is in keyInfo.
* @return {@link RepeatedOmKeyInfo}
*/
- public static RepeatedOmKeyInfo prepareKeyForDelete(OmKeyInfo keyInfo,
+ public static RepeatedOmKeyInfo prepareKeyForDelete(long bucketId, OmKeyInfo keyInfo,
Review Comment:
ok. sounds fine
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9089:
URL: https://github.com/apache/ozone/pull/9089#issuecomment-3363966584
> What happens to existing `RepeatedOmKeyInfo` entries in `deletedKeyTable` after an upgrade? Is this case handled correctly?
Oh we would be handling that case with quota repair tool. We don't have to be worried about the older keys since those wouldn't be included in the quota usage. Upgrade would become expensive since this would mean we would have to rewrite the entire deletedKeyTable again and it would be even more expensive in presence of snapshots
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409037557
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveDeletedKeysRequest.java:
##########
@@ -98,11 +100,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
List<SnapshotMoveKeyInfos> reclaimKeysList = moveDeletedKeysRequest.getReclaimKeysList();
List<HddsProtos.KeyValue> renamedKeysList = moveDeletedKeysRequest.getRenamedKeysList();
List<String> movedDirs = moveDeletedKeysRequest.getDeletedDirsToMoveList();
-
+ OmBucketInfo omBucketInfo = OMKeyRequest.getBucketInfo(omMetadataManager, snapshotInfo.getVolumeName(),
+ snapshotInfo.getBucketName());
OMSnapshotMoveUtils.updateCache(ozoneManager, fromSnapshot, nextSnapshot, context);
omClientResponse = new OMSnapshotMoveDeletedKeysResponse(
omResponse.build(), fromSnapshot, nextSnapshot,
- nextDBKeysList, reclaimKeysList, renamedKeysList, movedDirs);
+ nextDBKeysList, reclaimKeysList, renamedKeysList, movedDirs, omBucketInfo.getObjectID());
Review Comment:
Could `omBucketInfo` be `null` here?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409034702
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java:
##########
@@ -706,19 +706,18 @@ public static void addVolumeToDB(String volumeName, String ownerName,
* @param omMetadataManager
* @throws Exception
*/
- public static void addBucketToDB(String volumeName, String bucketName,
+ public static long addBucketToDB(String volumeName, String bucketName,
OMMetadataManager omMetadataManager) throws Exception {
-
- addBucketToDB(volumeName, bucketName, omMetadataManager,
- BucketLayout.DEFAULT);
+ return addBucketToDB(volumeName, bucketName, omMetadataManager,
+ BucketLayout.DEFAULT).getObjectID();
}
public static OmBucketInfo addBucketToDB(String volumeName,
String bucketName, OMMetadataManager omMetadataManager,
BucketLayout bucketLayout)
throws Exception {
return addBucketToDB(omMetadataManager,
- OmBucketInfo.newBuilder().setVolumeName(volumeName)
+ OmBucketInfo.newBuilder().setVolumeName(volumeName).setObjectID(System.currentTimeMillis())
.setBucketName(bucketName)
.setBucketLayout(bucketLayout)
Review Comment:
```suggestion
OmBucketInfo.newBuilder().setVolumeName(volumeName)
.setObjectID(System.currentTimeMillis())
.setBucketName(bucketName)
.setBucketLayout(bucketLayout)
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2410654241
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java:
##########
@@ -41,6 +41,15 @@ public class RepeatedOmKeyInfo implements CopyObject<RepeatedOmKeyInfo> {
private static final Codec<RepeatedOmKeyInfo> CODEC_FALSE = newCodec(false);
private final List<OmKeyInfo> omKeyInfoList;
+ /**
+ * Represents the unique identifier for a bucket. This variable is used to
+ * distinguish between different instances of a bucket, even if a bucket
+ * with the same name is deleted and recreated.
+ *
+ * It is particularly useful for tracking and updating the quota usage
+ * associated with a bucket.
Review Comment:
Will make this in the follow up patch. I don't want a redundant CI run
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2410654241
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java:
##########
@@ -41,6 +41,15 @@ public class RepeatedOmKeyInfo implements CopyObject<RepeatedOmKeyInfo> {
private static final Codec<RepeatedOmKeyInfo> CODEC_FALSE = newCodec(false);
private final List<OmKeyInfo> omKeyInfoList;
+ /**
+ * Represents the unique identifier for a bucket. This variable is used to
+ * distinguish between different instances of a bucket, even if a bucket
+ * with the same name is deleted and recreated.
+ *
+ * It is particularly useful for tracking and updating the quota usage
+ * associated with a bucket.
Review Comment:
Will make this in the follow up patch.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #9089:
URL: https://github.com/apache/ozone/pull/9089#issuecomment-3382165919
Thanks @devmadhuu !
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409028918
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java:
##########
@@ -139,18 +142,18 @@ private void processResults(OMMetrics omMetrics, long numSubmittedOpenKeys,
private void updateOpenKeyTableCache(OzoneManager ozoneManager,
long trxnLogIndex, OpenKeyBucket keysPerBucket,
- Map<String, OmKeyInfo> deletedOpenKeys) throws IOException {
+ Map<String, Pair<Long, OmKeyInfo>> deletedOpenKeys) throws IOException {
boolean acquiredLock = false;
String volumeName = keysPerBucket.getVolumeName();
String bucketName = keysPerBucket.getBucketName();
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
-
try {
mergeOmLockDetails(omMetadataManager.getLock()
.acquireWriteLock(BUCKET_LOCK, volumeName, bucketName));
acquiredLock = getOmLockDetails().isLockAcquired();
-
+ OmBucketInfo omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);
+ long bucketId = omBucketInfo == null ? 0L : omBucketInfo.getObjectID();
Review Comment:
`omBucketInfo == null` means the bucket is deleted already?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409084604
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java:
##########
@@ -41,6 +41,15 @@ public class RepeatedOmKeyInfo implements CopyObject<RepeatedOmKeyInfo> {
private static final Codec<RepeatedOmKeyInfo> CODEC_FALSE = newCodec(false);
private final List<OmKeyInfo> omKeyInfoList;
+ /**
+ * Represents the unique identifier for a bucket. This variable is used to
+ * distinguish between different instances of a bucket, even if a bucket
+ * with the same name is deleted and recreated.
+ *
+ * It is particularly useful for tracking and updating the quota usage
+ * associated with a bucket.
Review Comment:
nit: bucketId is only used for quota tracking right now
```suggestion
* It is used for tracking and updating the quota usage
* associated with a bucket.
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #9089:
URL: https://github.com/apache/ozone/pull/9089#issuecomment-3378506279
> @ArafatKhan2198 @devmadhuu historically RepeatedOmKeyInfo caused some issues in Recon. Could you double check this change is fine from Recon perspective?
Here an optional field is added. It in theory shouldn't cause parsing problems, but good idea to double check.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "priyeshkaratha (via GitHub)" <gi...@apache.org>.
priyeshkaratha commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409443995
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotMoveDeletedKeysResponse.java:
##########
@@ -52,21 +52,25 @@ public class OMSnapshotMoveDeletedKeysResponse extends OMClientResponse {
private List<SnapshotMoveKeyInfos> reclaimKeysList;
private List<HddsProtos.KeyValue> renamedKeysList;
private List<String> movedDirs;
+ private long bucketId;
+ @SuppressWarnings("checkstyle:ParameterNumber")
Review Comment:
Can we create builder for OMSnapshotMoveDeletedKeysResponse class instead of skiping checkstyle?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #9089:
URL: https://github.com/apache/ozone/pull/9089#issuecomment-3380128064
@jojochuang @smengcl , I verified the change, this addition of bucketId will not create problem in Recon as of today.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #9089:
URL: https://github.com/apache/ozone/pull/9089#issuecomment-3363432570
What happens to existing `RepeatedOmKeyInfo` entries in `deletedKeyTable` after an upgrade? Is this case handled correctly?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409041759
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveDeletedKeysRequest.java:
##########
@@ -98,11 +100,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
List<SnapshotMoveKeyInfos> reclaimKeysList = moveDeletedKeysRequest.getReclaimKeysList();
List<HddsProtos.KeyValue> renamedKeysList = moveDeletedKeysRequest.getRenamedKeysList();
List<String> movedDirs = moveDeletedKeysRequest.getDeletedDirsToMoveList();
-
+ OmBucketInfo omBucketInfo = OMKeyRequest.getBucketInfo(omMetadataManager, snapshotInfo.getVolumeName(),
+ snapshotInfo.getBucketName());
OMSnapshotMoveUtils.updateCache(ozoneManager, fromSnapshot, nextSnapshot, context);
omClientResponse = new OMSnapshotMoveDeletedKeysResponse(
omResponse.build(), fromSnapshot, nextSnapshot,
- nextDBKeysList, reclaimKeysList, renamedKeysList, movedDirs);
+ nextDBKeysList, reclaimKeysList, renamedKeysList, movedDirs, omBucketInfo.getObjectID());
Review Comment:
No, since we cannot delete a bucket without deleting all snapshots
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409054070
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java:
##########
@@ -139,18 +142,18 @@ private void processResults(OMMetrics omMetrics, long numSubmittedOpenKeys,
private void updateOpenKeyTableCache(OzoneManager ozoneManager,
long trxnLogIndex, OpenKeyBucket keysPerBucket,
- Map<String, OmKeyInfo> deletedOpenKeys) throws IOException {
+ Map<String, Pair<Long, OmKeyInfo>> deletedOpenKeys) throws IOException {
boolean acquiredLock = false;
String volumeName = keysPerBucket.getVolumeName();
String bucketName = keysPerBucket.getBucketName();
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
-
try {
mergeOmLockDetails(omMetadataManager.getLock()
.acquireWriteLock(BUCKET_LOCK, volumeName, bucketName));
acquiredLock = getOmLockDetails().isLockAcquired();
-
+ OmBucketInfo omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);
+ long bucketId = omBucketInfo == null ? 0L : omBucketInfo.getObjectID();
Review Comment:
yeah
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409009161
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java:
##########
@@ -41,6 +41,7 @@ public class RepeatedOmKeyInfo implements CopyObject<RepeatedOmKeyInfo> {
private static final Codec<RepeatedOmKeyInfo> CODEC_FALSE = newCodec(false);
private final List<OmKeyInfo> omKeyInfoList;
+ private final long bucketId;
Review Comment:
nit: add a javadoc to explain what this field is for
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409066453
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java:
##########
@@ -706,19 +706,18 @@ public static void addVolumeToDB(String volumeName, String ownerName,
* @param omMetadataManager
* @throws Exception
*/
- public static void addBucketToDB(String volumeName, String bucketName,
+ public static long addBucketToDB(String volumeName, String bucketName,
OMMetadataManager omMetadataManager) throws Exception {
-
- addBucketToDB(volumeName, bucketName, omMetadataManager,
- BucketLayout.DEFAULT);
+ return addBucketToDB(volumeName, bucketName, omMetadataManager,
+ BucketLayout.DEFAULT).getObjectID();
}
public static OmBucketInfo addBucketToDB(String volumeName,
String bucketName, OMMetadataManager omMetadataManager,
BucketLayout bucketLayout)
throws Exception {
return addBucketToDB(omMetadataManager,
- OmBucketInfo.newBuilder().setVolumeName(volumeName)
+ OmBucketInfo.newBuilder().setVolumeName(volumeName).setObjectID(System.currentTimeMillis())
.setBucketName(bucketName)
.setBucketLayout(bucketLayout)
Review Comment:
done
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotMoveDeletedKeysResponse.java:
##########
@@ -156,7 +160,8 @@ private void processReclaimKeys(BatchOperation batchOp,
throws IOException {
for (SnapshotMoveKeyInfos dBKey : reclaimKeysList) {
RepeatedOmKeyInfo omKeyInfos =
- createRepeatedOmKeyInfo(dBKey.getKeyInfosList());
+ createRepeatedOmKeyInfo(dBKey.
+ getKeyInfosList(), bucketId);
Review Comment:
done
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java:
##########
@@ -41,6 +41,7 @@ public class RepeatedOmKeyInfo implements CopyObject<RepeatedOmKeyInfo> {
private static final Codec<RepeatedOmKeyInfo> CODEC_FALSE = newCodec(false);
private final List<OmKeyInfo> omKeyInfoList;
+ private final long bucketId;
Review Comment:
done
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9089:
URL: https://github.com/apache/ozone/pull/9089#discussion_r2409017240
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotMoveDeletedKeysResponse.java:
##########
@@ -156,7 +160,8 @@ private void processReclaimKeys(BatchOperation batchOp,
throws IOException {
for (SnapshotMoveKeyInfos dBKey : reclaimKeysList) {
RepeatedOmKeyInfo omKeyInfos =
- createRepeatedOmKeyInfo(dBKey.getKeyInfosList());
+ createRepeatedOmKeyInfo(dBKey.
+ getKeyInfosList(), bucketId);
Review Comment:
nit: unexpected line break
```suggestion
createRepeatedOmKeyInfo(dBKey.getKeyInfosList(), bucketId);
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi merged PR #9089:
URL: https://github.com/apache/ozone/pull/9089
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13733. RepeatedOmKeyInfo in deletedKeyTable should track bucket id [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #9089:
URL: https://github.com/apache/ozone/pull/9089#issuecomment-3377590202
@ArafatKhan2198 @devmadhuu historically RepeatedOmKeyInfo caused some issues in Recon. Could you double check this change is fine from Recon perspective?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org