Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
49ef9e8
Rebased the PR
ArafatKhan2198 Oct 23, 2023
a47e09f
Rebased the PR
ArafatKhan2198 Oct 23, 2023
0c2005f
New changes
ArafatKhan2198 Oct 25, 2023
6031ad5
Fixed NSSummaryTaskWithOBS reprocess method
ArafatKhan2198 Oct 29, 2023
4c28bc9
Refactored NSSummaryTaskWithOBS and removed unnecessary methods
ArafatKhan2198 Oct 29, 2023
fd81752
Added process() method to NSSummaryTaskWithOBS
ArafatKhan2198 Oct 29, 2023
606044a
Removed unnecessary debugging code
ArafatKhan2198 Oct 29, 2023
4fe5ef8
Added Unit test for reprocess() for NSSummaryTaskWithOBS
ArafatKhan2198 Oct 29, 2023
17df34d
Added UT's for the process() method in NSSummaryWithOBS
ArafatKhan2198 Oct 30, 2023
da7f4e1
Fixed first set of review comments
ArafatKhan2198 Dec 7, 2023
37c0438
Took care of the review comments and also fixed the failing UT's
ArafatKhan2198 Dec 11, 2023
e9f0c4b
Merge branch 'master' into HDDS-7810
ArafatKhan2198 Dec 11, 2023
2655425
Migrated junit for TestNSSummaryTaskWithOBS to jUnit5
ArafatKhan2198 Dec 11, 2023
f610de0
Removed unecessary changes
ArafatKhan2198 Dec 11, 2023
9c53399
Made the latest review chanegs
ArafatKhan2198 Jan 29, 2024
c5d3b27
Made review changes
ArafatKhan2198 Jan 29, 2024
68ab58f
Fixed failing CI checks in fork
ArafatKhan2198 Jan 30, 2024
ddd1a3b
Added apache licence for class
ArafatKhan2198 Jan 30, 2024
76597a4
Fixed checkstyle issues for HDDS-7810
ArafatKhan2198 Jan 31, 2024
5ab09d3
Potential fix for serilization error
ArafatKhan2198 Jan 31, 2024
ac2f003
HDDS-7810 fixed the find bug problem
ArafatKhan2198 Jan 31, 2024
297be0c
Fixed checkstyle issues
ArafatKhan2198 Jan 31, 2024
01f817b
Fixed failing UT
ArafatKhan2198 Feb 7, 2024
913662e
Fixed failing UT's and also added tests for the NSSummaryEndpoint for…
ArafatKhan2198 Feb 11, 2024
57801b2
Fixed checkstyle
ArafatKhan2198 Feb 11, 2024
3e8ec2a
Added license
ArafatKhan2198 Feb 11, 2024
576b4c0
Merge branch 'master' into HDDS-7810
ArafatKhan2198 Feb 13, 2024
76158c9
Made changes for code review
ArafatKhan2198 Feb 15, 2024
cf04f83
Fixed review comments
ArafatKhan2198 Feb 27, 2024
dca31d3
Fixed more review comments
ArafatKhan2198 Feb 27, 2024
1d2d0b5
Made changes to the TestNSSummaryTaskWithOBS
ArafatKhan2198 Feb 28, 2024
e1120ae
Change the names of variable constants
ArafatKhan2198 Feb 29, 2024
0555a7f
Fixed potential NPE being thrown by value
ArafatKhan2198 Feb 29, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,7 @@ public static BucketHandler getBucketHandler(
omMetadataManager, reconSCM, bucketInfo);
} else if (bucketInfo.getBucketLayout()
.equals(BucketLayout.OBJECT_STORE)) {
// TODO: HDDS-7810 Write a handler for object store bucket
// We can use LegacyBucketHandler for OBS bucket for now.
return new LegacyBucketHandler(reconNamespaceSummaryManager,
return new OBSBucketHandler(reconNamespaceSummaryManager,
omMetadataManager, reconSCM, bucketInfo);
} else {
LOG.error("Unsupported bucket layout: " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.apache.hadoop.ozone.recon.api.types.NSSummary;
import org.apache.hadoop.ozone.recon.recovery.ReconOMMetadataManager;
import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;

Expand All @@ -46,6 +48,10 @@
*/
public abstract class EntityHandler {


private static final Logger LOG = LoggerFactory.getLogger(
Comment thread
ArafatKhan2198 marked this conversation as resolved.
Outdated
EntityHandler.class);

private final ReconNamespaceSummaryManager reconNamespaceSummaryManager;

private final ReconOMMetadataManager omMetadataManager;
Expand Down Expand Up @@ -118,67 +124,67 @@ public String[] getNames() {
* @return the entity handler of client's request
*/
public static EntityHandler getEntityHandler(
ReconNamespaceSummaryManager reconNamespaceSummaryManager,
ReconOMMetadataManager omMetadataManager,
OzoneStorageContainerManager reconSCM,
String path) throws IOException {
ReconNamespaceSummaryManager reconNamespaceSummaryManager,
ReconOMMetadataManager omMetadataManager,
OzoneStorageContainerManager reconSCM,
String path) throws IOException {
BucketHandler bucketHandler;

String normalizedPath = normalizePath(path);
String[] names = parseRequestPath(normalizedPath);
if (path.equals(OM_KEY_PREFIX)) {
return EntityType.ROOT.create(reconNamespaceSummaryManager,
omMetadataManager, reconSCM, null, path);
omMetadataManager, reconSCM, null, path);
}

if (names.length == 0) {
return EntityType.UNKNOWN.create(reconNamespaceSummaryManager,
omMetadataManager, reconSCM, null, path);
omMetadataManager, reconSCM, null, path);
} else if (names.length == 1) { // volume level check
String volName = names[0];
if (!volumeExists(omMetadataManager, volName)) {
return EntityType.UNKNOWN.create(reconNamespaceSummaryManager,
omMetadataManager, reconSCM, null, path);
omMetadataManager, reconSCM, null, path);
}
return EntityType.VOLUME.create(reconNamespaceSummaryManager,
omMetadataManager, reconSCM, null, path);
omMetadataManager, reconSCM, null, path);
} else if (names.length == 2) { // bucket level check
String volName = names[0];
String bucketName = names[1];

bucketHandler = BucketHandler.getBucketHandler(
reconNamespaceSummaryManager,
omMetadataManager, reconSCM,
volName, bucketName);
reconNamespaceSummaryManager,
omMetadataManager, reconSCM,
volName, bucketName);

if (bucketHandler == null
|| !bucketHandler.bucketExists(volName, bucketName)) {
return EntityType.UNKNOWN.create(reconNamespaceSummaryManager,
omMetadataManager, reconSCM, null, path);
omMetadataManager, reconSCM, null, path);
}
return EntityType.BUCKET.create(reconNamespaceSummaryManager,
omMetadataManager, reconSCM, bucketHandler, path);
omMetadataManager, reconSCM, bucketHandler, path);
} else { // length > 3. check dir or key existence
String volName = names[0];
String bucketName = names[1];

String keyName = BucketHandler.getKeyName(names);

bucketHandler = BucketHandler.getBucketHandler(
reconNamespaceSummaryManager,
omMetadataManager, reconSCM,
volName, bucketName);
reconNamespaceSummaryManager,
omMetadataManager, reconSCM,
volName, bucketName);

// check if either volume or bucket doesn't exist
if (bucketHandler == null
|| !volumeExists(omMetadataManager, volName)
|| !bucketHandler.bucketExists(volName, bucketName)) {
return EntityType.UNKNOWN.create(reconNamespaceSummaryManager,
omMetadataManager, reconSCM, null, path);
omMetadataManager, reconSCM, null, path);
}
return bucketHandler.determineKeyPath(keyName)
.create(reconNamespaceSummaryManager,
omMetadataManager, reconSCM, bucketHandler, path);
omMetadataManager, reconSCM, bucketHandler, path);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,223 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.hadoop.ozone.recon.api.handlers;


import org.apache.hadoop.hdds.scm.server.OzoneStorageContainerManager;
import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.hadoop.hdds.utils.db.TableIterator;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.recon.api.types.DUResponse;
import org.apache.hadoop.ozone.recon.api.types.EntityType;
import org.apache.hadoop.ozone.recon.api.types.NSSummary;
import org.apache.hadoop.ozone.recon.recovery.ReconOMMetadataManager;
import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.List;

import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;

/**
* Class for handling Legacy buckets.
Comment thread
ArafatKhan2198 marked this conversation as resolved.
Outdated
*/
public class OBSBucketHandler extends BucketHandler {

private static final Logger LOG = LoggerFactory.getLogger(
OBSBucketHandler.class);

private final String vol;
private final String bucket;
private final OmBucketInfo omBucketInfo;

public OBSBucketHandler(
ReconNamespaceSummaryManager reconNamespaceSummaryManager,
ReconOMMetadataManager omMetadataManager,
OzoneStorageContainerManager reconSCM,
OmBucketInfo bucketInfo) {
super(reconNamespaceSummaryManager, omMetadataManager,
reconSCM);
this.omBucketInfo = bucketInfo;
this.vol = omBucketInfo.getVolumeName();
this.bucket = omBucketInfo.getBucketName();
}

/**
* Helper function to check if a path is a key, or invalid.
*
* @param keyName key name
* @return KEY, or UNKNOWN
* @throws IOException
*/
public EntityType determineKeyPath(String keyName)
Comment thread
ArafatKhan2198 marked this conversation as resolved.
Outdated
throws IOException {
String key = OM_KEY_PREFIX + vol +
OM_KEY_PREFIX + bucket +
OM_KEY_PREFIX + keyName;

Table<String, OmKeyInfo> keyTable = getKeyTable();

TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
iterator = keyTable.iterator();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TableIterator needs to be closed. Please use in try-with-resources to ensure that.

Also in all other similar code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. I have implemented the recommended changes.


iterator.seek(key);
if (iterator.hasNext()) {
Table.KeyValue<String, OmKeyInfo> kv = iterator.next();
String dbKey = kv.getKey();
if (dbKey.equals(key)) {
return EntityType.KEY;
}
}
return EntityType.UNKNOWN;
}

/**
* This method handles disk usage of direct keys.
*
* @param parentId parent OBS bucket
* @param withReplica if withReplica is enabled, set sizeWithReplica
* for each direct key's DU
* @param listFile if listFile is enabled, append key DU as a children
* keys
* @param duData the current DU data
* @param normalizedPath the normalized path request
* @return the total DU of all direct keys
* @throws IOException IOE
*/
public long handleDirectKeys(long parentId, boolean withReplica,
Comment thread
ArafatKhan2198 marked this conversation as resolved.
boolean listFile,
List<DUResponse.DiskUsage> duData,
String normalizedPath) throws IOException {

Table<String, OmKeyInfo> keyTable = getKeyTable();
long keyDataSizeWithReplica = 0L;

TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
iterator = keyTable.iterator();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one needs to be closed, too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing it out.
I have made the changes.


String seekPrefix = OM_KEY_PREFIX +
vol +
OM_KEY_PREFIX +
bucket +
OM_KEY_PREFIX;

NSSummary nsSummary = getReconNamespaceSummaryManager()
.getNSSummary(parentId);
// Handle the case of an empty bucket.
if (nsSummary == null) {
return 0;
}
Comment on lines +109 to +114

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check at the start of the method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. I have implemented the recommended changes.


iterator.seek(seekPrefix);

while (iterator.hasNext()) {
// KeyName : OmKeyInfo-Object
Table.KeyValue<String, OmKeyInfo> kv = iterator.next();
String dbKey = kv.getKey();

// Exit loop if the key doesn't match the seekPrefix.
if (!dbKey.startsWith(seekPrefix)) {
break;
}

OmKeyInfo keyInfo = kv.getValue();
if (keyInfo != null) {
DUResponse.DiskUsage diskUsage = new DUResponse.DiskUsage();
String objectName = keyInfo.getKeyName();
diskUsage.setSubpath(objectName);
diskUsage.setKey(true);
diskUsage.setSize(keyInfo.getDataSize());

if (withReplica) {
long keyDU = keyInfo.getReplicatedSize();
keyDataSizeWithReplica += keyDU;
diskUsage.setSizeWithReplica(keyDU);
}
// List all the keys for the OBS bucket if requested.
if (listFile) {
duData.add(diskUsage);
}
}
}

return keyDataSizeWithReplica;
}

/**
* Object stores do not support directories, hence return null.
*
* @return null
*/
public long calculateDUUnderObject(long parentId)
Comment thread
ArafatKhan2198 marked this conversation as resolved.
Outdated
throws IOException {
return Long.parseLong(null);
Comment thread
ArafatKhan2198 marked this conversation as resolved.
Outdated
}

/**
* Object stores do not support directories, hence return null.
*
* @return null
*/
public long getDirObjectId(String[] names) throws IOException {
Comment thread
ArafatKhan2198 marked this conversation as resolved.
Outdated
return Long.parseLong(null);
}

/**
* Object stores do not support directories, hence return null.
*
* @return null
*/
public long getDirObjectId(String[] names, int cutoff) throws IOException {
Comment thread
ArafatKhan2198 marked this conversation as resolved.
Outdated
return Long.parseLong(null);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will throw NumberFormatException as null is not valid long. Can throw UnsupportedOperationException as this is not supported

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

NumberFormatException: Cannot parse null string

(also for other similar code)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. I have implemented the recommended changes.

}


public OmKeyInfo getKeyInfo(String[] names) throws IOException {
Comment thread
ArafatKhan2198 marked this conversation as resolved.
String ozoneKey = OM_KEY_PREFIX;
ozoneKey += String.join(OM_KEY_PREFIX, names);
Comment on lines +243 to +244

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
String ozoneKey = OM_KEY_PREFIX;
ozoneKey += String.join(OM_KEY_PREFIX, names);
String ozoneKey = OM_KEY_PREFIX + String.join(OM_KEY_PREFIX, names);

I'm guessing there must be some utility method for this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have checked the utility classes in recon, and currently there is no utility method available for this specific line. All other handlers within the codebase also utilize the same join method.


OmKeyInfo keyInfo = getKeyTable().getSkipCache(ozoneKey);
return keyInfo;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: inline the variable keyInfo

Suggested change
OmKeyInfo keyInfo = getKeyTable().getSkipCache(ozoneKey);
return keyInfo;
return getKeyTable().getSkipCache(ozoneKey);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. I have implemented the recommended changes.

}

/**
* Object stores do not support directories, hence return null.
*
* @return null
*/
@Override
public OmDirectoryInfo getDirInfo(String[] names) throws IOException {
return null;
}

public Table<String, OmKeyInfo> getKeyTable() {
Table keyTable =
getOmMetadataManager().getKeyTable(getBucketLayout());
return keyTable;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
Table keyTable =
getOmMetadataManager().getKeyTable(getBucketLayout());
return keyTable;
return getOmMetadataManager().getKeyTable(getBucketLayout());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. I have implemented the recommended changes.

}

public BucketLayout getBucketLayout() {
return BucketLayout.OBJECT_STORE;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public class NSSummaryTask implements ReconOmTask {
private final ReconOMMetadataManager reconOMMetadataManager;
private final NSSummaryTaskWithFSO nsSummaryTaskWithFSO;
private final NSSummaryTaskWithLegacy nsSummaryTaskWithLegacy;
private final NSSummaryTaskWithOBS nsSummaryTaskWithOBS;
private final OzoneConfiguration ozoneConfiguration;

@Inject
Expand All @@ -85,6 +86,9 @@ public NSSummaryTask(ReconNamespaceSummaryManager
this.nsSummaryTaskWithLegacy = new NSSummaryTaskWithLegacy(
reconNamespaceSummaryManager,
reconOMMetadataManager, ozoneConfiguration);
this.nsSummaryTaskWithOBS = new NSSummaryTaskWithOBS(
reconNamespaceSummaryManager,
reconOMMetadataManager, ozoneConfiguration);
}

@Override
Expand All @@ -101,6 +105,11 @@ public Pair<String, Boolean> process(OMUpdateEventBatch events) {
} else {
LOG.error("processWithFSO failed.");
}
if (success) {
Comment thread
ArafatKhan2198 marked this conversation as resolved.
Outdated
success = nsSummaryTaskWithOBS.processWithOBS(events);
} else {
LOG.error("processWithOBS failed.");
}
return new ImmutablePair<>(getTaskName(), success);
}

Expand All @@ -121,6 +130,8 @@ public Pair<String, Boolean> reprocess(OMMetadataManager omMetadataManager) {
.reprocessWithFSO(omMetadataManager));
tasks.add(() -> nsSummaryTaskWithLegacy
.reprocessWithLegacy(reconOMMetadataManager));
tasks.add(() -> nsSummaryTaskWithOBS
.reprocessWithOBS(reconOMMetadataManager));

List<Future<Boolean>> results;
ExecutorService executorService = Executors
Comment thread
ArafatKhan2198 marked this conversation as resolved.
Outdated
Expand Down
Loading