Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CM] SavedObjectFinder server-side pagination #163043

Closed
Dosant opened this issue Aug 3, 2023 · 7 comments
Closed

[CM] SavedObjectFinder server-side pagination #163043

Dosant opened this issue Aug 3, 2023 · 7 comments
Labels
discuss Feature:Content Management User generated content (saved objects) management performance Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)

Comments

@Dosant
Copy link
Contributor

Dosant commented Aug 3, 2023

Old description: Bring back partial response support

When migrating SavedObjectFinder from so.client to content management for backward compatibility reasons, we dropped includeFields functionality which allowed to return partial objects (for example, only title and description instead of full objects). #162904, #161545

This issue is to discuss and investigate if it is worth getting this functionality back in some form but in a backward-compatible way.

  • we need to implement a way to decouple includeFields transferred over the network and saved object attributes, so that they can be migrated before performing search

Currently ,without this feature we over-fetch the data, but it is mitigated by the following:

  • Only index-pattern, search, maps, lens, annotation-group are supported types. No larger canvas or dashboard
  • There is a LISTING_LIMIT advanced setting (1000 is default), can be reduced if someone sees issues

Alternatively, we can reduce the limit, implement server-side search and pagination.

When migrating SavedObjectFinder from so.client to content management for backward compatibility reasons, we dropped includeFields functionality which allowed to return partial objects (for example, only title and description instead of full objects). #162904, #161545

Currently SavedObjectFinder uses a LISTING_LIMIT advanced setting (1000 is default) to fetch the items and then uses client side in-memory sorting and pagination.

This might become a performance problem in some cases, as now the api always returns full objects.
As discussed with @vadimkibana #163043 (comment), a good and the most straightforward solution would be to refactor the SavedObjectFinder to use server-side pagination.

@Dosant Dosant added performance Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Feature:Content Management User generated content (saved objects) management labels Aug 3, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@Dosant Dosant changed the title [CM] SavedObjectFinder bring back partial object support [CM] SavedObjectFinder bring back partial object support? Aug 3, 2023
Dosant added a commit that referenced this issue Aug 8, 2023
## Summary

close #161545
close #153257

This PR makes `SavedObjectFinder` component backward compatible. It is
achieved by going through content- management layer, more technical
details
[here](https://docs.google.com/document/d/1ssYmqSEUPrsuCR4iz8DohkEWekoYrm2yL4QR_fVxXLg/edit)

### Testing

`SavedObjectFinder` is this component that allows to pick a saved object
(supports: `search` `index-pattern` `map` `visualization` `lens`
`event-annotation-group`:

![Screenshot 2023-08-07 at 16 53
32](https://github.com/elastic/kibana/assets/7784120/5c283ea5-3682-4dc8-a8ff-422e6f4f3195)


It is used in the following places: 

- Dashboard 
  - Add panel
  - Replace panel 
- Discover - Open Search 
- Visualization - Select search as a source for new viz
- Graph - select source
- Cases - markdown editor add lens 
- ML (3 places) 
- Canvas - select embeddable panel 
- Transform 
- Lens > select event annotation 




### Risks / Follow up 

The `SavedObjectFinder` should stay mostly the same, the only notable
functional change is that now `SavedObjectFinder` doesn't support
`includeFields` which allowed partial saved object returns, this was
done to make the call backward-compatible without making the system even
more complicated as otherwise we'll need a way to abstract
`includeFields` from so attributes and allow to run migrations on it
before making a search. follow up issue to bring it back
#163043

The risk with that is that some client that have a lot of large objects
might run into performance issues when using `SavedObjectFinder`. This
can be mitigated by changing listing limit in advanced setting from
default 1000 to something lower
bryce-b pushed a commit to bryce-b/kibana that referenced this issue Aug 9, 2023
## Summary

close elastic#161545
close elastic#153257

This PR makes `SavedObjectFinder` component backward compatible. It is
achieved by going through content- management layer, more technical
details
[here](https://docs.google.com/document/d/1ssYmqSEUPrsuCR4iz8DohkEWekoYrm2yL4QR_fVxXLg/edit)

### Testing

`SavedObjectFinder` is this component that allows to pick a saved object
(supports: `search` `index-pattern` `map` `visualization` `lens`
`event-annotation-group`:

![Screenshot 2023-08-07 at 16 53
32](https://github.com/elastic/kibana/assets/7784120/5c283ea5-3682-4dc8-a8ff-422e6f4f3195)


It is used in the following places: 

- Dashboard 
  - Add panel
  - Replace panel 
- Discover - Open Search 
- Visualization - Select search as a source for new viz
- Graph - select source
- Cases - markdown editor add lens 
- ML (3 places) 
- Canvas - select embeddable panel 
- Transform 
- Lens > select event annotation 




### Risks / Follow up 

The `SavedObjectFinder` should stay mostly the same, the only notable
functional change is that now `SavedObjectFinder` doesn't support
`includeFields` which allowed partial saved object returns, this was
done to make the call backward-compatible without making the system even
more complicated as otherwise we'll need a way to abstract
`includeFields` from so attributes and allow to run migrations on it
before making a search. follow up issue to bring it back
elastic#163043

The risk with that is that some client that have a lot of large objects
might run into performance issues when using `SavedObjectFinder`. This
can be mitigated by changing listing limit in advanced setting from
default 1000 to something lower
@vadimkibana
Copy link
Contributor

There is a LISTING_LIMIT advanced setting (1000 is default), can be reduced if someone sees issues

It sounds like the problem is that existing listing pages don't paginate, but load all items? I think we might want to treat this as high priority tech debt and implement pagination, so 10 items (or so) are loaded at a time instead of 1,000.

@Dosant
Copy link
Contributor Author

Dosant commented Aug 16, 2023

It sounds like the problem is that existing listing pages don't paginate, but load all items? I think we might want to treat this as high priority tech debt and implement pagination, so 10 items (or so) are loaded at a time instead of 1,000.

@vadimkibana, I fully agree. I think this makes the most sense and benefits everyone in the future.
We also have places which are intentionally asking for "*" all the fields and this fix will improve those.

I think we also have the same tech debt for the table list view

@Dosant Dosant changed the title [CM] SavedObjectFinder bring back partial object support? [CM] SavedObjectFinder server-side pagination Aug 17, 2023
@kpatticha
Copy link
Contributor

@Dosant if I understand correctly, the plan is to implement a pagination but return the full object.?

For our case in apm #167763 it is reasonable to have the ability to be able to narrow down the result. Especially when is supported by the Saved object API.

@Dosant
Copy link
Contributor Author

Dosant commented Oct 4, 2023

@kpatticha, yes this issue is about the server-side pagination for our components (savedobjectfinder, tablelistview)
The partial objects support needs to be discussed/worked on separately

@kpatticha
Copy link
Contributor

@Dosant thanks, is there any ticket/thread I can follow regarding the partial object support?

@Dosant
Copy link
Contributor Author

Dosant commented Oct 10, 2023

@Dosant thanks, is there any ticket/thread I can follow regarding the partial object support?

No, afaik, we don't have it.

With content management we thought we won't have partial objects, because longer-term by having only full objects we wanted to make client side caching work (e.g. you show a list of dashboards, click on one, and immediately open a dashboard without waiting for another fetch). We hoped we can address slow list fetching concern with proper server-side pagination.

If this won't work for your use-case, then could you please create an issue, so we'd take a look at this again?

Alternatively, if the use-case is client-side / dashboard specific maybe this could be a separate dashboard specific server-side route that returns only ids/titles. (on the server it would still fetch the whole objects from es)

@Dosant Dosant removed their assignment Jan 23, 2024
@petrklapka petrklapka closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:Content Management User generated content (saved objects) management performance Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Development

No branches or pull requests

5 participants