You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/08/05 20:10:43 UTC
[GitHub] [ozone] smengcl commented on a change in pull request #2489: HDDS-5449 Recon namespace summary 'du' information should return replicated size of a key
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: 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