Use the $expand parameter when querying collections

[Arno500]

When querying subcollections, gofish is currently iterating manually 3 by 3 to get the data. This is painfully slow, especially when the collections are big and the server is far away.

I would propose to include support for the $expand OData parameter when supported (this needs to be checked when querying a collection for the first time on this entity). It would allow the device to answer once for all the sub elements when asked, making the query much faster and reducing the impact of latency.

My specific case was requesting a PDU with 48 plugs, that is pretty slow in Redfish already. Querying all of them individually is not exactly fun as you can guess :P

[stmcginnis]

Thanks for bringing this up. It is an interesting idea. The Redfish service can be a little slow responding, but that network latency of making multiple calls to get the elements is definitely the bigger bottleneck.

I haven't spent a ton of time on this, but what I had thought about doing was to set a flag if it's supported. Luckily, the ServiceRoot object that you get at the root of the object hierarchy should contain ServiceRoot.ProtocolFeaturesSupported.ExpandQuery.ExpandAll. So right away on connection it should be possible to determine if the service will support expanding collections or not.

The trickier part is probably including that in a uniform interface for the library consumer. But I think during the custom unmarshalling that we need to do, we could try to collect that array of sub-objects in a private property. Then the consumer would still call the same method as they would if expand wasn't being used (e.g. outlets, err := pdu.Outlets()). Then internally, if the expanded data is there we can just return the collection of objects. If it is not there, then we can use the existing method of getting the collection of objects the slower way.

I will try to get some time to try this out some time soon.

[mwieczorek]

Hi @stmcginnis

It would be awesome to move forward with this idea.

Did you think how you'd like to expose it to the library consumer? F.e.:

• would $expand URL param be always sent by redfish client (of course when server supports it based on ServiceRoot.ProtocolFeaturesSupported.ExpandQuery.ExpandAll) or would it be 'opt-in' / 'opt-out' feature?
• if opt-in/opt-out - would it be a client's setting or 'per-request'? (or both, so you can control the default at the client level and override it per 'request')

And are you open to review a draft PR, where I can try implementing it for one resource and 'expanded' child collection?

[stmcginnis]

I honestly haven't had a lot of time to ponder this. Yes, PRs definitely welcome! I'm trying to remember now, but I was thinking we could have some flag set so the library user could opt in to operating in a way that could potentially consume a lot more memory. Then in the accessor methods (e.g. computerSystem.VirtualMedia()) it would determine if it already has the objects or if it needs to fetch them.

[joeyberkovitz]

Just bumping this

I think the rough idea in #408 looks reasonable - basically leaving the decision as to when to use expand up to the user. I'm also not opposed to having the user enable expand at the client level instead of the request level, although there may still be cases where an override is useful.

We could possibly default to an auto option which does:

• Check if ServiceRoot claims to support expand
• If so, try $expand=. (~ or * if only one is supported)
• If we get some error back indicating that expand isn't supported (ex: 507 Insufficient Storage) - fallback to the non-expand version

#412 I'm less confident in. It seems the big change was the expandOnlyCollection feature which isn't fully implemented. Regardless, if i understand correctly, #412 is proposing having gofish logic to expand a second level down via additional HTTP requests. IMO - the user can just do this themselves.

[stmcginnis]

Thanks for bumping this. I pretty much agree with your assessment. I was waiting for the lint errors to be addressed with #408, but looks like that needs a little nudge to get back to addressing. I would like to start with that and see what else we need. Very much in favor of anything that does not break existing users and does not introduce any surprises.

[joeyberkovitz]

sounds good, I'd be happy to expand on #408 but @mwieczorek - LMK if you want to work on it

[mwieczorek]

hi @joeyberkovitz
thanks for the feedback, I closed #412
If you'd like to continue on #408, please go ahead. Especially I'd have less time to work on it in following days/weeks.
If not, I'll try to fix the linter errors during the weekend.

[joeyberkovitz]

went to look at this again and it makes more sense to me now, so to summarize (mostly for my understanding), we have two objectives:

• setting query params $expand on a generic resource, such as Memory
• setting $expand on a collection

then those might mix, so say you want MemoryMetrics on Memory

• you could query the MemoryCollection with ?$expand=.($levels=2) with the downside being that it then expands every link under Memory. You could then combine that with $select to limit what's returned
• if a device doesn't support this, you'd want to provide unique query options for both the collection and any sub-objects (at level 1)

I'll play with combining the changes from #412 in a possibly less breaking way, but it does at least make more sense to me now.

Side-note - there's already the FilterOption type which I think should probably be deprecated and replaced long-term with a more generic QueryOption as we have defined in #408

[joeyberkovitz]

put up #482 which I think combines everything

I didn't break the existing structs, but did move over to using more generic structs that mirror the spec. LMK if any thoughts

I'd probably want some unit tests before merging it