aries-cloudagent-python icon indicating copy to clipboard operation
aries-cloudagent-python copied to clipboard

Support paginated querying of storage records

Open ff137 opened this issue 1 year ago • 10 comments

In a multi-tenant environment with potentially millions of wallets, pagination becomes a critical feature.

Currently, the list wallets endpoint is configured to fetch all. It takes a good few minutes to execute when there are 100s of 1'000s of tenants.

There are probably a lot of endpoints that can benefit from pagination, but off the top of my head, these ones stand out:

  • [x] list wallets
    • #3000
  • [x] list connection records
  • [x] list credential exchange records
  • [x] list presentation exchange records
    • #3033

Additional features:

  • [x] fix pagination with post-filter query params
    • #3082
  • [ ] support for ordering in pagination
    • #3173

I've started working on adding pagination support here: #3000, and so far I've managed to get it working for listing wallets 🎉.

For our use case we need support for the above listed bullet points, so I'll try get those all working for an initial implementation.

Any comments, recommendations or further discussion is welcome!


Related issues: #1919, #2373

ff137 avatar May 31 '24 13:05 ff137

@jamshale it's still in progress (I can't reopen it) -- still need to make a PR for connection / cred ex / presentation records. And adding support for ordering in askar

ff137 avatar Jun 13 '24 20:06 ff137

Potential endpoints that may need pagination as well:

  • listing created rev regs, in the revocation API
  • listing transaction records, in the endorse_transaction API
  • listing mediation requests, in coordinate_mediation

Those are the only other ones I can see, with potentially large responses

ff137 avatar Jun 19 '24 13:06 ff137

I've discovered a bug with pagination + additional post-filter query parameters.

e.g. for fetching connection records, specifying limit=1 and alias="some alias", then it will return 0 records, despite that alias appearing in some records (i.e. it'll return relevant records when limit is set higher). Post-filtering is being done after the limited fetch, which messes up expected output

ff137 avatar Jul 03 '24 13:07 ff137

This impacts:

  • fetching v1 or v2 credential or presentation exchange records, with query params: connection_id, role, state
  • fetching connection records with: alias, state, their_role, connection_protocol

These are all the post-filter query params for the routes that now have paginated query support. So, limit + offset will not work as expected when using these query params

A workaround is to ignore limit/offset when these query params exist, and to only apply limit/offset after the post filtering ... this would resolve the expected behavior, but there will still be a performance/scalability cost when there are potentially 1000s of records.

@jamshale Do you think this is a reasonable workaround to implement? Behavior <= 0.12.1 was anyway fetching all records. So, while it's not ideal to apply limit/offset after post-filtering, it's still better than having no pagination support ...

ff137 avatar Jul 03 '24 14:07 ff137

@ff137 Yes, I think that's the correct way forward for now. I've never understood why these post filters have to be applied the way they do but I think that's good enough as is and we can try and address it in the future.

jamshale avatar Jul 03 '24 15:07 jamshale

Agreed. It's of course highly inefficient -- loading each record result as json and post filtering on fields (edit: I see the json deserialization is necessary either way, nvm). I'll do the workaround for now, but will also take a look at askar library for how this may be improved

ff137 avatar Jul 03 '24 15:07 ff137

I'm working on the following issue in Askar to add support for custom ordering: https://github.com/hyperledger/aries-askar/issues/290

@swcurran @jamshale I think this would be an essential feature for the 1.0.0 release, so that paginating through storage records guarantees that missing or duplicate records aren't returned across pages.

ff137 avatar Jul 25 '24 10:07 ff137

Thanks for taking this on. This is an important feature.

My only concern about having it in the 1.0.0 release is that it keeps getting delayed. We really want to get it out and have a more steady release cadence.

This pagination still works correctly, correct? It's just extremely inefficient with the post filtering?

jamshale avatar Jul 25 '24 15:07 jamshale

That's all good. Writing tests for the new feature might take some time too. Is the idea to release 1.0.0 ASAP? If there's a rough date set, then I can try get this ready by then. If not, I'll just see how far I get!

Also yes, the pagination works -- I've just run into trouble when trying to assert that each page returns distinct results, with no duplicates or missing records. I managed to create 100'000 wallets, and fetch 1000 at a time, asserting each page has distinct results. So that was successful. But then when fetching connection records with post-filter query params, I couldn't get the same guarantees. Creating new connections, or new wallets, might cause some shuffling. Difficult to pin down exactly what the cause or frequency of the problem is. But all I can say is there's currently no "guarantee" that scanning pages, by increasing the offset, will always return distinct results. Having an ordering option will provide guarantee / more control for the end user.

ff137 avatar Jul 25 '24 19:07 ff137

We don't have a date. We want to release ASAP.

I think this problem is acceptable to have in the release. It would make a good release milestone once it's completed.

jamshale avatar Jul 25 '24 20:07 jamshale