avo-hq / avo

Build Ruby on Rails apps 10x faster
https://avohq.io
Other
1.48k stars 232 forks source link

Prevent memory bloat in associations when searchable is set to false #2406

Open adrienpoly opened 7 months ago

adrienpoly commented 7 months ago

Feature

By default an association field has a searchable: false attribute

When attaching a record. A modal loads with a select field and the associated records. This works perfectly when the list of associated records is small. But when the collection is large it is either very slow or worse can create a significant memory bloat and crash the app.

This is the typical example where locally it will run fine as you might not have lots of users in the development db but will crash in prod when displaying the association modal.

field :users, as: :has_many 

Current workarounds

After creating 2 crashes in prod, I currently ensure that all association fields have searchable True with a custom Cop.

I like the simplicity of a simple select when the collection has a reasonable size but when this size grows it should not create a memory bloat

Several solution were discussed in: https://discord.com/channels/740892036978442260/1199641108880298127

I don't have a preferred solution and understand that this could be confusing. But something should be done to avoid bloat in prod as this is a critical issue.

https://github.com/avo-hq/avo/issues/2085 will help as we will be able to set a default value for searchabe. But it will still require for the user to set it and in the mean time could experience a bloat

Screenshots or screen recordings

Additional context

adrianthedev commented 7 months ago

Context

We want to give the best experience for both free and paying users, so we need this solution to fit both parties. We also want to have the least surprise for the developer and the user. The solution should be intuitive and provide at least to the developer how and why it behaves the way it does.

The problem

If you don't use the searchable: true option on a belongs_to field, it will render a select dropdown with all the records it finds for that query. The number of records may very well be in the tens of thousands, or even millions, raising issues with loading all those records and sometimes creashing the browser. The resolve for this is to mark the field as searchable.

The other issue is that developers might overlook or not have the proper visibility into how many records they have in different environments (eg: production). This leads to crashing edit pages.

This happens to the attach modal of has_many as well.

Another thing to consider is that the Community tier does not have the searchable option. It's a pro feature.

Possible solutions

We'd like to offer a semi-automatic way of preventing that crash.

Options:

  1. Automatically limit the number of results to 1000 (or another arbitrary value) so it will never crash the app in prod.
  1. Count the number of items, limit and show a notice

After the count we can figure out if we want to apply the limit or not.

  1. Count the number of items and turn the field into searchable automatically (Pro users)
  1. Add a global "safe" config that would turn on option 1.

Things to consider

What is the arbitrary number we'd like to apply the limiting? 1000, 5000, 10_000? We should run some tests.

Please send your feedback

adrienpoly commented 7 months ago

I would like to throw an idea. looking at the upcoming combobox from Jose Farias. It looks like we could have the best of both world.

https://x.com/fariastweets/status/1752494684474380717?s=20

Both free and pro user have the same dropdown :

To be honest I think it would be an improvement for pro users too. The current UX of the searchable field is OK but once I search for a term I am limited to 10 results and often I would like to scroll down to see more but that is not possible.

edit : the gem is not officially open but is it possible to play with it here

Paul-Bob commented 5 months ago

Let's check the combobox possibility and also make the 10 results after search configurable.

adrianthedev commented 5 months ago

We might be able to use something like this https://stackoverflow.com/a/52604805/1067281

Paul-Bob commented 5 months ago

We might be able to use something like this https://stackoverflow.com/a/52604805/1067281

The main issue persists with that suggestions: loading all options on page load

stevegeek commented 3 weeks ago

Just throwing in my thought here on Option 1 above (regardless of the idea of adding the lazily loaded select, which sounds great):

I would personally lean on the defensive side here and always apply a limit to queries destined for rendering a table view, or UI element, eg 1000 records.

I doubt any Rails app will ever gracefully handle rendering very large numbers of records, but also the user experience of say a select box with thousands of options is so limited anyway that very few would normally intentionally create such a thing.

Anyway the limit could be configurable on a resource level or globally so could be changed if needed.

Even if the limit was unexpected in a sense, its even more unexpected to try to render a page with millions of UI elements, so my guess is that this is a much bigger problem than the few devs who actually wants a select box with 10,000 options in it.