Skip to content

get used_size of a backup from simplyblock API#547

Open
boddumanohar wants to merge 1 commit intomainfrom
backupup-usedsize
Open

get used_size of a backup from simplyblock API#547
boddumanohar wants to merge 1 commit intomainfrom
backupup-usedsize

Conversation

@boddumanohar
Copy link
Member

partially fixes: simplyblock/vela#195

currently we size_bytes of a Backup from CSI VolumeSnapshot's RESTORESIZE field. But there is not field to get the used size. So as a part of this PR, we get the used size by calling Simplyblock's storage API

Copy link
Collaborator

@mxsrc mxsrc left a comment

Choose a reason for hiding this comment

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

Generally this looks good, as usual I'm hesitant to write things to the database. This doesn't look like it's a high bandwidth thing so I'd prefer simply querying the API until we get issues with that.

Beyond generally that general thought I'm not sure about the semantics of used_size on snapshots, is that constant? If two snapshots contain a specific change and one of them get's deleted, wouldn't the used_size of the other increase?

Comment on lines 181 to 183
async def _snapshot_used_size(content_name: str | None) -> int:
if content_name is None:
raise ValueError("vollume snapshot content_name empty")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we allow for content_name to be None only to immediately raise if that's the case?

Copy link
Member Author

Choose a reason for hiding this comment

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

the was done to deal with type checker.

    content_name = status.get("boundVolumeSnapshotContentName")
    size_bytes = quantity_to_bytes(status.get("restoreSize"))
    try:
        used_size_bytes = await _snapshot_used_size(content_name)

the content_name that is passed as input is possible to be both "str" and "None". That's why have allowed both and parameter.

happy to accept any alternative sugestions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This sanitization should happen on the caller-side, this is not a failure of the function itself.

@boddumanohar
Copy link
Member Author

The reason to store this in Database is:

This PR partially solves, simplyblock/vela#195

As a part of branch list API, we return used_resources. The goal is to add a new additional field snapshot_used_size (the field name is subject to change).

"used_resources": {
             "milli_vcpu": 0,
             "ram_bytes": 0,
             "nvme_bytes": 0,
             "iops": 0,
             "storage_bytes": null
+++      "snapshot_used_size": <sum of all used snapshot sizes of this branch>
    },

If the details are stored in database the computation of sum of all snapshots of a branch becomes easier and less costly.

@boddumanohar boddumanohar requested a review from mxsrc January 30, 2026 05:20
@mxsrc
Copy link
Collaborator

mxsrc commented Jan 30, 2026

According to the discussion with Michael we need to take more care when storing the used resources in the database. Bascially, at any point where snapshots are deleted, the values would have to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The backup size is wrong. It shows the size of the disk, not the consumed storage of the snapshot chain.

2 participants