xichen01 commented on code in PR #8909:
URL: https://github.com/apache/ozone/pull/8909#discussion_r2267134001
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -348,16 +350,22 @@ private void evaluateFSOBucket(OmVolumeArgs volume,
OmBucketInfo bucket, String
directoryPath.append(dir.getName()).append(OzoneConsts.OM_KEY_PREFIX);
}
if (directoryPath.toString().equals(rule.getEffectivePrefix() +
OzoneConsts.OM_KEY_PREFIX)) {
- expiredDirList.add(directoryPath.toString());
- expiredDirUpdateIDList.add(dirList.get(dirList.size() -
1).getUpdateID());
+ try {
+ expiredDirUpdateIDList.add(dirList.get(dirList.size() -
1).getUpdateID());
+ expiredDirList.add(directoryPath.toString());
+ } catch (IOException e) {
+ // send delete request for pending deletion directories
+ sendDeleteKeysRequestAndClearList(volume.getVolume(),
bucket.getBucketName(),
Review Comment:
Maybe we can trigger send without throwing an exception, such as we can add
a `isFull` method for `LimitedSizeList` can we can check the `isFull` and send
request if it is true.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -457,40 +493,52 @@ private void evaluateDirTable(Table<String,
OmDirectoryInfo> directoryInfoTable,
String dirPath = directoryPath + dir.getName();
numDirIterated++;
if (rule.match(dir, dirPath)) {
- // mark dir as expired, check next key
- dirList.add(dirPath);
- dirUpdateIDList.add(dir.getUpdateID());
+ try {
+ // mark dir as expired, check next key
+ dirUpdateIDList.add(dir.getUpdateID());
+ dirList.add(dirPath);
+ } catch (IOException e) {
+ // send delete request for pending deletion directories
+ sendDeleteKeysRequestAndClearList(volumeName, bucketName,
dirList, dirUpdateIDList, true);
+ }
}
}
} catch (IOException e) {
// log failure and continue the process to delete/move files already
identified in this run
- LOG.warn("Failed to iterate directoryInfoTable for bucket {}",
bucketKey, e);
+ LOG.warn("Failed to iterate directoryInfoTable for bucket {}/{}",
volumeName, bucketName, e);
}
}
- private void evaluateBucket(String bucketKey,
+ private void evaluateBucket(OmBucketInfo bucketInfo,
Table<String, OmKeyInfo> keyTable, List<OmLCRule> ruleList,
- List<String> expiredKeyList, List<Long> expiredKeyUpdateIDList) {
+ LimitedSizeList<String> expiredKeyList, List<Long>
expiredKeyUpdateIDList) {
+ String volumeName = bucketInfo.getVolumeName();
+ String bucketName = bucketInfo.getBucketName();
// use bucket name as key iterator prefix
try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
keyTblItr =
- keyTable.iterator(bucketKey)) {
+ keyTable.iterator(omMetadataManager.getBucketKey(volumeName,
bucketName))) {
while (keyTblItr.hasNext()) {
Table.KeyValue<String, OmKeyInfo> keyValue = keyTblItr.next();
OmKeyInfo key = keyValue.getValue();
numKeyIterated++;
for (OmLCRule rule : ruleList) {
if (rule.match(key)) {
- // mark key as expired, check next key
- expiredKeyList.add(key.getKeyName());
- expiredKeyUpdateIDList.add(key.getUpdateID());
+ try {
+ // mark key as expired, check next key
+ expiredKeyUpdateIDList.add(key.getUpdateID());
+ expiredKeyList.add(key.getKeyName());
+ } catch (IOException e) {
+ // send delete request for pending deletion keys
+ sendDeleteKeysRequestAndClearList(volumeName, bucketName,
expiredKeyList, expiredKeyUpdateIDList, true);
Review Comment:
the parameter `boolean dir` should be false too.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -100,10 +100,11 @@ public KeyLifecycleService(OzoneManager ozoneManager,
super(KeyLifecycleService.class.getSimpleName(), serviceInterval,
TimeUnit.MILLISECONDS,
poolSize, serviceTimeout, ozoneManager.getThreadNamePrefix());
this.ozoneManager = ozoneManager;
- this.keyLimitPerIterator =
conf.getInt(OZONE_KEY_LIFECYCLE_SERVICE_DELETE_BATCH_SIZE,
+ this.keyDeleteBatchSize =
conf.getInt(OZONE_KEY_LIFECYCLE_SERVICE_DELETE_BATCH_SIZE,
OZONE_KEY_LIFECYCLE_SERVICE_DELETE_BATCH_SIZE_DEFAULT);
- Preconditions.checkArgument(keyLimitPerIterator >= 0,
+ Preconditions.checkArgument(keyDeleteBatchSize >= 0,
Review Comment:
The keyDeleteBatchSize should be large than 0, otherwise the below `i +=
batchSize;` in the `sendDeleteKeysRequestAndClearList` will cause a dead loop.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -391,8 +405,14 @@ private void evaluateFSOBucket(OmVolumeArgs volume,
OmBucketInfo bucket, String
for (OmLCRule rule : noPrefixRuleList) {
if (rule.match(key)) {
// mark key as expired, check next key
- expiredKeyList.add(key.getKeyName());
- expiredKeyUpdateIDList.add(key.getUpdateID());
+ try {
+ expiredKeyUpdateIDList.add(key.getUpdateID());
+ expiredKeyList.add(key.getKeyName());
+ } catch (IOException e) {
+ // send delete request for pending deletion keys
+ sendDeleteKeysRequestAndClearList(volume.getVolume(),
bucket.getBucketName(),
Review Comment:
the parameter `boolean dir` should be false.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -316,9 +318,9 @@ public BackgroundTaskResult call() {
@SuppressWarnings("checkstyle:parameternumber")
private void evaluateFSOBucket(OmVolumeArgs volume, OmBucketInfo bucket,
String bucketKey,
- Table<String, OmKeyInfo> keyTable,
List<OmLCRule> ruleList,
- List<String> expiredKeyList, List<Long>
expiredKeyUpdateIDList,
- List<String> expiredDirList, List<Long>
expiredDirUpdateIDList) {
+ Table<String, OmKeyInfo> keyTable, List<OmLCRule> ruleList,
+ LimitedSizeList<String> expiredKeyList, List<Long>
expiredKeyUpdateIDList,
Review Comment:
NIT: if we can directly use the `LimitedSizeList<String> ` or a simple wrap
class for keyName and updateId, so that we can only use a variable.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -435,20 +463,28 @@ private void evaluateKeyTable(Table<String, OmKeyInfo>
keyTable, String prefix,
String keyPath = directoryPath + key.getKeyName();
numKeyIterated++;
if (rule.match(key, keyPath)) {
- // mark key as expired, check next key
- keyList.add(keyPath);
- keyUpdateIDList.add(key.getUpdateID());
+ try {
+ // mark key as expired, check next key
+ keyUpdateIDList.add(key.getUpdateID());
+ keyList.add(keyPath);
+ } catch (IOException e) {
+ // send delete request for pending deletion keys
+ sendDeleteKeysRequestAndClearList(volumeName, bucketName,
keyList, keyUpdateIDList, true);
Review Comment:
the parameter `boolean dir` should be `false`.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -411,9 +431,15 @@ private void evaluateFSOBucket(OmVolumeArgs volume,
OmBucketInfo bucket, String
numDirIterated++;
for (OmLCRule rule : noPrefixRuleList) {
if (rule.match(dir, dir.getPath())) {
- // mark key as expired, check next key
- expiredDirList.add(dir.getPath());
- expiredDirUpdateIDList.add(dir.getUpdateID());
+ try {
Review Comment:
Nit: the IOException for iterator should be `directoryInfoTable` instead of
`keyTable `
--
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]
