You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smengcl (via GitHub)" <gi...@apache.org> on 2023/08/11 07:39:23 UTC
[GitHub] [ozone] smengcl opened a new pull request, #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
smengcl opened a new pull request, #5175:
URL: https://github.com/apache/ozone/pull/5175
## What changes were proposed in this pull request?
A snapshot's referenced size is identical to the bucket's point-in-time `omBucketInfo.getUsedBytes()`. Note it is the size AFTER replication.
- Snapshot referenced size is stored directly into `SnapshotInfoTable` during snapshot creation so as to avoid opening every single snapshot under a bucket just to display the size.
- Currently referenced size can be shown under `fs -ls`:
```
bash-4.2$ ozone fs -mkdir -p ofs://om/vol1/buck2/dir3/subdir4/
bash-4.2$ ozone fs -put README.md ofs://om/vol1/buck2/dir3/subdir4/
bash-4.2$ ozone fs -createSnapshot ofs://om/vol1/buck2/ snap1
Created snapshot ofs://om/vol1/buck2/.snapshot/snap1
bash-4.2$ ozone fs -ls ofs://om/vol1/buck2/.snapshot/
Found 1 items
drwxrwxrwx - hadoop hadoop 4068 2023-08-11 07:30 ofs://om/vol1/buck2/.snapshot/snap1
bash-4.2$
```
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9159
## How was this patch tested?
- Tested manually in Docker compose cluster.
- Tested via `testFsLsSnapshot`.
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5175:
URL: https://github.com/apache/ozone/pull/5175#discussion_r1310698243
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java:
##########
@@ -586,15 +688,24 @@ public boolean equals(Object o) {
Objects.equals(
globalPreviousSnapshotId, that.globalPreviousSnapshotId) &&
snapshotPath.equals(that.snapshotPath) &&
- checkpointDir.equals(that.checkpointDir);
+ checkpointDir.equals(that.checkpointDir) &&
+ deepClean == that.deepClean &&
Review Comment:
Depending on how `SnapshotInfo` is used in comparisons.
We could deliberately ignore them if it fits the use case.
If not I would keep it here for completeness.
--
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
[GitHub] [ozone] smengcl commented on pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #5175:
URL: https://github.com/apache/ozone/pull/5175#issuecomment-1703860866
Thanks @hemantk-12 @swamirishi @aswinshakil for reviewing this.
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5175:
URL: https://github.com/apache/ozone/pull/5175#discussion_r1295141929
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -1209,7 +1209,9 @@ private static FileStatusAdapter getFileStatusAdapterForBucketSnapshot(
ozoneSnapshot.getName(), pathStr);
}
Path path = new Path(pathStr);
- return new FileStatusAdapter(0L, 0L, path, true, (short)0, 0L,
+ return new FileStatusAdapter(
+ ozoneSnapshot.getReferencedSize(), ozoneSnapshot.getReferencedSize(),
+ path, true, (short)0, 0L,
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
[GitHub] [ozone] smengcl commented on pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #5175:
URL: https://github.com/apache/ozone/pull/5175#issuecomment-1683116257
> Thanks @smengcl for the patch. nit: An assert statement should also be created in TestOMSnapshotCreateRequest#testValidateAndUpdateCache & TestOmSnapshotCreateResponse#testAddToDBBatch. Otherwise the patch looks good to me
Added in `TestOMSnapshotCreateRequest`.
It is not making much sense to add the check in `TestOmSnapshotCreateResponse` since it does not concern the `Request` change in this PR. `testAddToDBBatch` is crafting its own `OMSnapshotCreateResponse` object just to test `addToDBBatch` and does not go through `validateAndUpdateCache` at all.
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5175:
URL: https://github.com/apache/ozone/pull/5175#discussion_r1310699720
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -728,6 +728,8 @@ protected void initializeOmTables(boolean addCacheMetrics)
String.class, OmDBTenantState.class);
checkTableStatus(tenantStateTable, TENANT_STATE_TABLE, addCacheMetrics);
+ // TODO: Consider FULL_CACHE for snapshotInfoTable since exclusiveSize in
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5175:
URL: https://github.com/apache/ozone/pull/5175#discussion_r1295130027
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java:
##########
@@ -589,14 +610,17 @@ public boolean equals(Object o) {
Objects.equals(
globalPreviousSnapshotId, that.globalPreviousSnapshotId) &&
snapshotPath.equals(that.snapshotPath) &&
- checkpointDir.equals(that.checkpointDir);
+ checkpointDir.equals(that.checkpointDir) &&
+ Objects.equals(deepClean, that.deepClean) &&
Review Comment:
Yup let me add that as well for the sake of completeness.
--
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
[GitHub] [ozone] umamaheswararao commented on pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "umamaheswararao (via GitHub)" <gi...@apache.org>.
umamaheswararao commented on PR #5175:
URL: https://github.com/apache/ozone/pull/5175#issuecomment-1677662595
@swamirishi @hemantk-12 one of you take a look at it?
--
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
[GitHub] [ozone] smengcl merged pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #5175:
URL: https://github.com/apache/ozone/pull/5175
--
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
[GitHub] [ozone] aswinshakil commented on a diff in pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil commented on code in PR #5175:
URL: https://github.com/apache/ozone/pull/5175#discussion_r1309500740
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java:
##########
@@ -586,15 +688,24 @@ public boolean equals(Object o) {
Objects.equals(
globalPreviousSnapshotId, that.globalPreviousSnapshotId) &&
snapshotPath.equals(that.snapshotPath) &&
- checkpointDir.equals(that.checkpointDir);
+ checkpointDir.equals(that.checkpointDir) &&
+ deepClean == that.deepClean &&
Review Comment:
I was wondering if we need to add `deepClean` and `sstFiltered` for `equals` as these can change depending on the moment if it was deep cleaned or sst filtered. I guess it should be fine.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/QuotaUtil.java:
##########
@@ -55,8 +60,36 @@ public static long getReplicatedSize(
+ partialFirstChunk * rc.getParity();
return dataSize + replicationOverhead;
} else {
+ LOG.warn("Unknown replication type '{}'. Returning original data size.",
+ repConfig.getReplicationType());
return dataSize;
}
}
+ /**
+ * Get an estimated data size (before replication) from the replicated size.
+ * An (inaccurate) reverse of getReplicatedSize().
+ * @param replicatedSize size after replication.
+ * @param repConfig The replicationConfig used to store the data.
+ * @return Data size before replication.
+ */
+ public static long getDataSize(long replicatedSize,
+ ReplicationConfig repConfig) {
+ if (repConfig.getReplicationType() == RATIS) {
+ final int ratisReplicationFactor = ((RatisReplicationConfig) repConfig)
+ .getReplicationFactor().getNumber();
+ // May not be divisible. But it's fine to ignore remainder in our use case
+ return replicatedSize / ratisReplicationFactor;
+ } else if (repConfig.getReplicationType() == EC) {
+ ECReplicationConfig rc = (ECReplicationConfig) repConfig;
+ // In the case of EC, we won't know if keys have partial chunks or not,
+ // so we assume no partial chunks as an estimate.
+ return replicatedSize * rc.getData() / rc.getRequiredNodes();
Review Comment:
For a rough estimate, This should be fine because only the last strip will be partial.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java:
##########
@@ -160,6 +166,22 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
.getLatestSequenceNumber();
snapshotInfo.setDbTxSequenceNumber(dbLatestSequenceNumber);
+ // Snapshot referenced size should be bucket's used bytes
+ OmBucketInfo omBucketInfo =
+ getBucketInfo(omMetadataManager, volumeName, bucketName);
+ snapshotInfo.setReferencedReplicatedSize(omBucketInfo.getUsedBytes());
+
+ // Snapshot referenced size in this case is an *estimate* inferred from
+ // the bucket default replication policy right now.
+ // This may well not be the actual sum of all key data sizes in this
+ // bucket because each key can have its own replication policy,
+ // depending on the choice of the client at the time of writing that key.
+ // And we will NOT do an O(n) walk over the keyTable (fileTable) here
+ // because it is a design goal of CreateSnapshot to be an O(1) operation.
+ // TODO: Assign actual data size once we have the pre-replicated key size
Review Comment:
```suggestion
// TODO: [SNAPSHOT] Assign actual data size once we have the pre-replicated key size
```
For easier grep.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -728,6 +728,8 @@ protected void initializeOmTables(boolean addCacheMetrics)
String.class, OmDBTenantState.class);
checkTableStatus(tenantStateTable, TENANT_STATE_TABLE, addCacheMetrics);
+ // TODO: Consider FULL_CACHE for snapshotInfoTable since exclusiveSize in
Review Comment:
```suggestion
// TODO: [SNAPSHOT] Consider FULL_CACHE for snapshotInfoTable since exclusiveSize in
```
--
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
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5175:
URL: https://github.com/apache/ozone/pull/5175#discussion_r1293910828
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java:
##########
@@ -589,14 +610,17 @@ public boolean equals(Object o) {
Objects.equals(
globalPreviousSnapshotId, that.globalPreviousSnapshotId) &&
snapshotPath.equals(that.snapshotPath) &&
- checkpointDir.equals(that.checkpointDir);
+ checkpointDir.equals(that.checkpointDir) &&
+ Objects.equals(deepClean, that.deepClean) &&
Review Comment:
Thanks for adding `deepClean`. Should we add `sstFiltered` as well?
Same for `hashCode()`.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -1209,7 +1209,9 @@ private static FileStatusAdapter getFileStatusAdapterForBucketSnapshot(
ozoneSnapshot.getName(), pathStr);
}
Path path = new Path(pathStr);
- return new FileStatusAdapter(0L, 0L, path, true, (short)0, 0L,
+ return new FileStatusAdapter(
+ ozoneSnapshot.getReferencedSize(), ozoneSnapshot.getReferencedSize(),
+ path, true, (short)0, 0L,
Review Comment:
nit:
```suggestion
path, true, (short) 0, 0L,
```
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5175: HDDS-9159. [Snapshot] Implement snapshot disk usage: Referenced size
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5175:
URL: https://github.com/apache/ozone/pull/5175#discussion_r1296486485
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -820,7 +820,8 @@ message SnapshotInfo {
optional int64 dbTxSequenceNumber = 12;
optional bool deepClean = 13;
optional bool sstFiltered = 14;
- }
+ optional uint64 referencedSize = 15;
Review Comment:
TODO: Rename to `referencedSizeReplicated`.
--
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