HDDS-14987. OM Web UI dashboard for Ozone Snapshot (list snapshot)#10055
HDDS-14987. OM Web UI dashboard for Ozone Snapshot (list snapshot)#10055jojochuang wants to merge 3 commits intoapache:masterfrom
Conversation
sadanand48
left a comment
sadanand48
left a comment
There was a problem hiding this comment.
Looked at the java changes and looks mostly good to me. It is a good idea to have a separate servlet that will only obtain snapshot list on demand based on the input bucket instead of dumping the entire table.
|
Now that #10027 is squash merged, you can rebase this PR |
jojochuang
force-pushed
the
lssnapshotui
branch
from
83d9092 to
2b80fa0
Compare
| response.setContentType("application/json; charset=utf8"); | ||
| PrintWriter writer = response.getWriter(); | ||
| try { | ||
| ListSnapshotResponse listSnapshotResponse = om.listSnapshot(volume, bucket, prefix, startItem, maxKeys); |
There was a problem hiding this comment.
It does not do pagination. maxKeys is set to 100 so looks like it won't see more than 100 snapshots on the bucket.
| </td> | ||
| <td>{{snapshot.creationTime | date:'yyyy-MM-dd HH:mm:ss'}}</td> | ||
| <td>{{$ctrl.formatBytes(snapshot.referencedSize)}}</td> | ||
| <td>{{$ctrl.formatBytes(snapshot.exclusiveSize)}}</td> |
There was a problem hiding this comment.
exclusiveSize should be offset by exclusiveSizeDeltaFromDirDeepCleaning for end user display. e.g.:
There was a problem hiding this comment.
hmm i don't recall what exclusiveSizeDeltaFromDirDeepCleaning is.
There was a problem hiding this comment.
do we want to update this section then? https://ozone.apache.org/docs/next/administrator-guide/operations/snapshots/overview#snapshot-sizing
There was a problem hiding this comment.
It was added in 50e6d61
The idea is, a snapshot's accurate exclusive size should be the combination of both. And the end user should always see the combined value. I suggested a renaming the existing exclusizeSize field to more accurately reflect what that represents but iirc it wasn't done because of compatiblity or something like that. Thus the user doc does not need to be changed.
Change-Id: Iea21514269d37a233c816a2cc55be44fe788382a
Change-Id: I55dd9b8a4fa31bcacefd83fbb827627c2d006fda
jojochuang
force-pushed
the
lssnapshotui
branch
from
923dd3d to
72a1b13
Compare
| do { | ||
| ListSnapshotResponse listSnapshotResponse = om.listSnapshot(volume, bucket, prefix, lastSnapshot, maxKeys); | ||
| writer.write(objectMapper.writeValueAsString(listSnapshotResponse.getSnapshotInfos())); | ||
| lastSnapshot = listSnapshotResponse.getLastSnapshot(); | ||
| } while (StringUtils.isNotEmpty(lastSnapshot)); |
There was a problem hiding this comment.
I am still a bit concerned about this logic.
This runs on OM. If there are tons of snapshots and/or client requests this endpoint a ton of times it would consume a lot of CPU and worsen young gen churn. Potential DoS.
The ideal way is for the client (web page) to request batch of snapshot on demand. OM can return only the ones that needs to be displayed on the web UI.
There was a problem hiding this comment.
This is on-demand itself right, The OM web UI access should only be with the admin so only when admin inputs the vol/bucket it will do a list snapshot request.
3 participants
What changes were proposed in this pull request?
HDDS-14987. OM Web UI dashboard for Ozone Snapshot (list snapshot)
Please describe your PR in detail:
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14987
How was this patch tested?
Unit test