Add Storage Management API

[davelopez]

This adds some new API endpoints to simplify the user's monitoring and cleanup of used storage.

Before these changes, the Storage Dashboard (#13113) in the client had to do some extra logic and workarounds to use the existing API. This also offers the opportunity to improve the performance of some of the requests.

I'm still a bit unsure about the route naming, so far this is the convention I will follow unless there are other ideas:
/api/storage/{object}/{condition}/[summary], where {object} can be either history or datasets and {condition} refers to the way they are discovered, for example:

• For deleted and non-purged histories (discarded) the relevant endpoints will look like this:

  GET /api/storage/histories/discarded/summary
  GET /api/storage/histories/discarded
  

• For "big" datasets (bigger than a certain user-defined amount) it will look like this:

  GET /api/storage/datasets/big/summary
  GET /api/storage/datasets/big
  

• For old histories and datasets, etc... you get the idea :)

The summary endpoints will just return the total number of objects and total size that is taken by them,

{
  "total_size": 4242283,
  "total_items": 2
}

while the regular endpoint will return a paginated list of objects with the following information for each object:

[
  {
      "id": "79966582feb6c081",
      "name": "The name of the history",
      "type": "history", // or "dataset"
      "size": 2444027,
      "update_time": "2022-12-17T09:54:59.292171"
  },
  //...
]

There is also an endpoint for purging objects in batch ``, you can pass a list of object IDs and it will purge all of them and return detailed information about the result:

{
  "total_item_count": 5,
  "success_item_count": 4,
  "total_free_bytes": 546987560,
  "errors": [
    {
      "item_id": "79966582feb6c081",
      "error": "This item couldn't be cleaned for some reason"
    }
  ]
}

TODO

• Implement endpoints for discarded histories
• Implement endpoints for discarded datasets
• Adapt client to new endpoints

How to test the changes?

• I've included appropriate automated tests.

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[nsoranzo]

I think /api/storage/{object}/{condition}/ feels more "natural" than /api/storage/{condition}/{object}/ but maybe that's just me. I haven't looked at the PR code, so maybe there's a technical reason for the order.

[davelopez]

Thanks, Nicola, there is no technical reason for the order. I just thought that it reads more naturally this way:
GET /api/storage/discarded/histories/summary -> "Give me the discarded histories summary" or "Give me the big datasets summary" since the {condition} will be an adjective.
While:
GET /api/storage/histories/discarded/summary -> "Give me the histories discarded summary" doesn't read as nicely, but it may be more in line with what we already have 🤔

So I'm 100% fine with either option (or a totally different one) 😄

[nsoranzo]

I guess /api/storage/histories could return something like:

{
    "big": 34,
    "discarded": 43,
    "other_condition": N,
}

and similarly for datasets.

[davelopez]

Ok, that is a good point. I won't implement this general endpoint for now, but it might be helpful in the future, so I will change the order to accommodate it.

By the way, I thought the deprecated module was standard but apparently, it isn't. Is it worth installing or better just drop the commit Mark manual service encoding/decoding as deprecated?

[itisAliRH]

ReviewCleanupDialog shows the wrong pagination numbers after permanently deleting items:

storage.dashboard.mp4

select all checkbox state is not correct when clicked:

storage.dashboard.2.mp4

[davelopez]

Thank you for having a look @itisAliRH! 👍

ReviewCleanupDialog shows the wrong pagination numbers after permanently deleting items

Does this happen with Celery disabled? With Celery enabled, it looks like when the cleanup request finishes and reports the results celery tasks in the background are still processing the actual purging of the datasets... so when you get the updated summary after the cleanup it still thinks that some of the items are not purged. Then when you open the review dialog some more might have been purged by then... this is definitely a bug. I need to think about how to get the requests in sync... 🤔

select all checkbox state is not correct when clicked

This is intentional behavior, the checkbox at the top left is only for the current "page" due to the items being paginated. If you really want to select "all" then you need to click the "select all X items" link at the top right. But it looks like it may be confusing... any UX ideas to communicate this better to the user?

[davelopez]

I think I fixed the issue by splitting the cleanup of datasets into 2 phases:
1- Mark all HDAs as purged in a single transaction
2- Launch a single task for removing all the underlying datasets from the store (or alternatively just synchronously if celery is disabled)

This should make every subsequent request consistent independently of the status of the scheduled task.
The cleanup result also will be more accurate now, meaning that removing a copy of a dataset will actually report that has freed 0 bytes since it was just a copy. I will try to add more information to the cleanup result dialog so users don't get confused by these "differences" in expectations of storage space recovered.

@itisAliRH can you give it another try with these changes? Thank you!

[itisAliRH]

I think I fixed the issue by splitting the cleanup of datasets into 2 phases: 1- Mark all HDAs as purged in a single transaction 2- Launch a single task for removing all the underlying datasets from the store (or alternatively just synchronously if celery is disabled)

This should make every subsequent request consistent independently of the status of the scheduled task. The cleanup result also will be more accurate now, meaning that removing a copy of a dataset will actually report that has freed 0 bytes since it was just a copy. I will try to add more information to the cleanup result dialog so users don't get confused by these "differences" in expectations of storage space recovered.

@itisAliRH can you give it another try with these changes? Thank you!

I just tried it again, and the first issue is fixed, thanks @davelopez!

This is intentional behavior, the checkbox at the top left is only for the current "page" due to the items being paginated. If you really want to select "all" then you need to click the "select all X items" link at the top right. But it looks like it may be confusing... any UX ideas to communicate this better to the user?

I see, but in this case, the select all icon shows the wrong status. Even when clicking on the "select all X items", shows the indeterminate status, not the full check.

Screen.Recording.2023-02-09.at.12.00.53.mp4

[davelopez]

I see, but in this case, the select all icon shows the wrong status. Even when clicking on the "select all X items", shows the indeterminate status, not the full check.

Thanks for noticing this! I've removed the limit for items that can be reviewed at once and forced the general check box in the table to behave like the "select all X items" link. I think I had to do all this juggling to deal with pagination because the request for the items was a bit slow. Now with the new API, in my local tests, with hundreds of items is surprisingly fast so I could overcome the select all checkbox limitation, and the selection should be simpler now.

@itisAliRH give it another try when you have time and let me know if now you like it better 😉

[itisAliRH]

I see, but in this case, the select all icon shows the wrong status. Even when clicking on the "select all X items", shows the indeterminate status, not the full check.

Thanks for noticing this! I've removed the limit for items that can be reviewed at once and forced the general check box in the table to behave like the "select all X items" link. I think I had to do all this juggling to deal with pagination because the request for the items was a bit slow. Now with the new API, in my local tests, with hundreds of items is surprisingly fast so I could overcome the select all checkbox limitation, and the selection should be simpler now.

@itisAliRH give it another try when you have time and let me know if now you like it better wink

@davelopez Thanks! I prefer to select all items on the current page by clicking on the general checkbox and select all items on all pages by clicking select all X items. Having two ways to do the same behaviour would be confusing.

ReviewCleanupDialog shows the wrong pagination numbers after permanently deleting items:

This problem came back again :(

[davelopez]

Ok, I think the least confusing option here is to remove the select all link and leave just the checkbox. Enabling the checkbox for selecting individual pages can be confusing too and the link was just to try to workaround the pagination issue, which seems to not be a problem anymore.

[davelopez]

I think this one is ready for review, especially the backend part around the new database queries and the way datasets are purged now in 2 phases. I'm happy to rethink this if this approach is not desirable.

The client part is mostly refactorings and simplifications to adapt to the new API, and also a few fixes/enhancements around the cleanable items review dialog.

[davelopez]

This requires some adjustments to make it compatible with #14073 (working on it)

[jmchilton]

Very nice polish - thank you!