smengcl commented on a change in pull request #2489:
URL: https://github.com/apache/ozone/pull/2489#discussion_r683675489
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -179,12 +204,15 @@ public Response getDiskUsage(@QueryParam("path") String
path)
EntityType type = getEntityType(normalizedPath, names);
DUResponse duResponse = new DUResponse();
+ duResponse.setPath(normalizedPath);
switch (type) {
case ROOT:
List<OmVolumeArgs> volumes = listVolumes();
duResponse.setCount(volumes.size());
List<DUResponse.DiskUsage> volumeDuData = new ArrayList<>();
+ long totalDataSize = 0;
+ long totalDataSizeWithReplica = 0;
Review comment:
nit
```suggestion
long totalDataSize = 0L;
long totalDataSizeWithReplica = 0L;
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -208,16 +252,32 @@ public Response getDiskUsage(@QueryParam("path") String
path)
// List of DiskUsage data for all buckets
List<DUResponse.DiskUsage> bucketDuData = new ArrayList<>();
- for (OmBucketInfo bucket : buckets) {
+ long volDataSize = 0;
+ long volDataSizeWithReplica = 0;
+ for (OmBucketInfo bucket: buckets) {
String bucketName = bucket.getBucketName();
long bucketObjectID = bucket.getObjectID();
String subpath = omMetadataManager.getBucketKey(volName, bucketName);
DUResponse.DiskUsage diskUsage = new DUResponse.DiskUsage();
diskUsage.setSubpath(subpath);
long dataSize = getTotalSize(bucketObjectID);
+ volDataSize += dataSize;
+ if (withReplica) {
+ long bucketDU = 0;
Review comment:
nit
```suggestion
long bucketDU = 0L;
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -242,9 +303,48 @@ public Response getDiskUsage(@QueryParam("path") String
path)
DUResponse.DiskUsage diskUsage = new DUResponse.DiskUsage();
diskUsage.setSubpath(subpath);
long dataSize = getTotalSize(subdirObjectId);
+ bucketDataSize += dataSize;
+
+ if (withReplica) {
+ long dirDU = 0;
Review comment:
nit
```suggestion
long dirDU = 0L;
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -253,8 +353,9 @@ public Response getDiskUsage(@QueryParam("path") String
path)
reconNamespaceSummaryManager.getNSSummary(dirObjectId);
Set<Long> subdirs = dirNSSummary.getChildDir();
- duResponse.setCount(subdirs.size());
duResponse.setKeySize(dirNSSummary.getSizeOfFiles());
+ long dirDataSize = duResponse.getKeySize();
+ long dirDataSizeWithReplica = 0;
Review comment:
nit
```suggestion
long dirDataSizeWithReplica = 0L;
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -267,26 +368,69 @@ public Response getDiskUsage(@QueryParam("path") String
path)
// reformat the response
diskUsage.setSubpath(subpath);
long dataSize = getTotalSize(subdirObjectId);
+ dirDataSize += dataSize;
+
+ if (withReplica) {
+ long subdirDU = 0;
Review comment:
nit
```suggestion
long subdirDU = 0L;
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -196,9 +224,25 @@ public Response getDiskUsage(@QueryParam("path") String
path)
long bucketObjectID = bucket.getObjectID();
dataSize += getTotalSize(bucketObjectID);
}
+ totalDataSize += dataSize;
+
+ // count replicas
+ if (withReplica) {
+ long volumeDU = 0;
+ List<OmKeyInfo> keys = listKeysUnderVolume(volumeName);
Review comment:
Caveat: if the volume has billions of keys, the jvm heap can be
exhausted and Recon can crash due to OOM.
Is it possible to count replica size without getting all keys under one
batch? this might require pagination.
Or even better: can we count replica size without even retaining the batches
of OmKeyInfo in a variable at all? This can avoid pagination, simplifying the
logic.
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -242,9 +303,48 @@ public Response getDiskUsage(@QueryParam("path") String
path)
DUResponse.DiskUsage diskUsage = new DUResponse.DiskUsage();
diskUsage.setSubpath(subpath);
long dataSize = getTotalSize(subdirObjectId);
+ bucketDataSize += dataSize;
+
+ if (withReplica) {
+ long dirDU = 0;
+
+ List<OmKeyInfo> keys = listKeysUnderDirectory(subdirObjectId);
+ for (OmKeyInfo keyInfo: keys) {
+ dirDU += getKeySizeWithReplication(keyInfo);
+ }
+ diskUsage.setSizeWithReplica(dirDU);
+ bucketDataSizeWithReplica += dirDU;
+ }
diskUsage.setSize(dataSize);
dirDUData.add(diskUsage);
}
+ // Either listFile or withReplica is enabled, we need the directKeys info
+ if (listFile || withReplica) {
+ List<OmKeyInfo> directKeys = listDirectKeys(bucketObjectId);
+
+ for (OmKeyInfo directKey: directKeys) {
Review comment:
nit
```suggestion
for (OmKeyInfo directKey : directKeys) {
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -208,16 +252,32 @@ public Response getDiskUsage(@QueryParam("path") String
path)
// List of DiskUsage data for all buckets
List<DUResponse.DiskUsage> bucketDuData = new ArrayList<>();
- for (OmBucketInfo bucket : buckets) {
+ long volDataSize = 0;
+ long volDataSizeWithReplica = 0;
Review comment:
nits
```suggestion
long volDataSize = 0L;
long volDataSizeWithReplica = 0L;
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -227,9 +287,10 @@ public Response getDiskUsage(@QueryParam("path") String
path)
// get object IDs for all its subdirectories
Set<Long> bucketSubdirs = bucketNSSummary.getChildDir();
- duResponse.setCount(bucketSubdirs.size());
duResponse.setKeySize(bucketNSSummary.getSizeOfFiles());
List<DUResponse.DiskUsage> dirDUData = new ArrayList<>();
+ long bucketDataSize = duResponse.getKeySize();
+ long bucketDataSizeWithReplica = 0;
Review comment:
nit
```suggestion
long bucketDataSizeWithReplica = 0L;
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -196,9 +224,25 @@ public Response getDiskUsage(@QueryParam("path") String
path)
long bucketObjectID = bucket.getObjectID();
dataSize += getTotalSize(bucketObjectID);
}
+ totalDataSize += dataSize;
+
+ // count replicas
+ if (withReplica) {
+ long volumeDU = 0;
Review comment:
nit
```suggestion
long volumeDU = 0L;
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -242,9 +303,48 @@ public Response getDiskUsage(@QueryParam("path") String
path)
DUResponse.DiskUsage diskUsage = new DUResponse.DiskUsage();
diskUsage.setSubpath(subpath);
long dataSize = getTotalSize(subdirObjectId);
+ bucketDataSize += dataSize;
+
+ if (withReplica) {
+ long dirDU = 0;
+
+ List<OmKeyInfo> keys = listKeysUnderDirectory(subdirObjectId);
+ for (OmKeyInfo keyInfo: keys) {
Review comment:
nit
```suggestion
for (OmKeyInfo keyInfo : keys) {
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -267,26 +368,69 @@ public Response getDiskUsage(@QueryParam("path") String
path)
// reformat the response
diskUsage.setSubpath(subpath);
long dataSize = getTotalSize(subdirObjectId);
+ dirDataSize += dataSize;
+
+ if (withReplica) {
+ long subdirDU = 0;
+ List<OmKeyInfo> keys = listKeysUnderDirectory(subdirObjectId);
+ for (OmKeyInfo keyInfo: keys) {
Review comment:
```suggestion
for (OmKeyInfo keyInfo : keys) {
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -242,9 +303,48 @@ public Response getDiskUsage(@QueryParam("path") String
path)
DUResponse.DiskUsage diskUsage = new DUResponse.DiskUsage();
diskUsage.setSubpath(subpath);
long dataSize = getTotalSize(subdirObjectId);
+ bucketDataSize += dataSize;
+
+ if (withReplica) {
+ long dirDU = 0;
+
+ List<OmKeyInfo> keys = listKeysUnderDirectory(subdirObjectId);
Review comment:
Same question here.
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -208,16 +252,32 @@ public Response getDiskUsage(@QueryParam("path") String
path)
// List of DiskUsage data for all buckets
List<DUResponse.DiskUsage> bucketDuData = new ArrayList<>();
- for (OmBucketInfo bucket : buckets) {
+ long volDataSize = 0;
+ long volDataSizeWithReplica = 0;
+ for (OmBucketInfo bucket: buckets) {
String bucketName = bucket.getBucketName();
long bucketObjectID = bucket.getObjectID();
String subpath = omMetadataManager.getBucketKey(volName, bucketName);
DUResponse.DiskUsage diskUsage = new DUResponse.DiskUsage();
diskUsage.setSubpath(subpath);
long dataSize = getTotalSize(bucketObjectID);
+ volDataSize += dataSize;
+ if (withReplica) {
+ long bucketDU = 0;
+ List<OmKeyInfo> keys = listKeysUnderBucket(bucketName);
+ for (OmKeyInfo keyInfo: keys) {
Review comment:
nit
```suggestion
for (OmKeyInfo keyInfo : keys) {
```
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -242,9 +303,48 @@ public Response getDiskUsage(@QueryParam("path") String
path)
DUResponse.DiskUsage diskUsage = new DUResponse.DiskUsage();
diskUsage.setSubpath(subpath);
long dataSize = getTotalSize(subdirObjectId);
+ bucketDataSize += dataSize;
+
+ if (withReplica) {
+ long dirDU = 0;
+
+ List<OmKeyInfo> keys = listKeysUnderDirectory(subdirObjectId);
+ for (OmKeyInfo keyInfo: keys) {
+ dirDU += getKeySizeWithReplication(keyInfo);
+ }
+ diskUsage.setSizeWithReplica(dirDU);
+ bucketDataSizeWithReplica += dirDU;
+ }
diskUsage.setSize(dataSize);
dirDUData.add(diskUsage);
}
+ // Either listFile or withReplica is enabled, we need the directKeys info
+ if (listFile || withReplica) {
+ List<OmKeyInfo> directKeys = listDirectKeys(bucketObjectId);
Review comment:
And here.
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java
##########
@@ -62,17 +72,25 @@
@Path("/namespace")
@Produces(MediaType.APPLICATION_JSON)
public class NSSummaryEndpoint {
+
+ private static final Logger LOG = LoggerFactory.getLogger(
+ NSSummaryEndpoint.class);
@Inject
private ReconNamespaceSummaryManager reconNamespaceSummaryManager;
@Inject
private ReconOMMetadataManager omMetadataManager;
+ private ReconContainerManager containerManager;
+
@Inject
public NSSummaryEndpoint(ReconNamespaceSummaryManager
namespaceSummaryManager,
- ReconOMMetadataManager omMetadataManager) {
+ ReconOMMetadataManager omMetadataManager,
+ OzoneStorageContainerManager reconSCM) {
this.reconNamespaceSummaryManager = namespaceSummaryManager;
this.omMetadataManager = omMetadataManager;
+ this.containerManager =
+ (ReconContainerManager) reconSCM.getContainerManager();
Review comment:
```suggestion
reconSCM.getContainerManager();
```
--
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]
