timoh / craftbeer

the Craft Beer finding app for Finland, written on Rails
1 stars 1 forks source link

Loading the drinks based on distance takes forever #24

Closed timoh closed 8 years ago

timoh commented 8 years ago

The data model / how the data is queried needs a redesign & refactoring.

techstalgist commented 8 years ago

The time between REQUEST_DRINKS and RECEIVE_DRINKS actions was about 22 seconds. So I suppose the backend performance should be improved first?

techstalgist commented 8 years ago

Current relations:

class AlcoDrink
  has_many :alco_avails
  has_one :review
class AlcoAvail
  belongs_to :alco_location
  belongs_to :alco_drink
class AlcoLocation
  has_many :alco_avails
class Review
  belongs_to :alco_drink

So now the problem is that there is one query to AlcoAvails per AlcoDrink, one query to Reviews per AlcoDrink, and one query to AlcoLocation per AlcoAvail.

techstalgist commented 8 years ago

Another comment: given the data model shown above, what is the benefit of using document-database like MongoDB? Wouldn't an SQL database be easier to use in this case? I don't know much about MongoDB, but I know that an SQL database would handle the JOINs very quickly given this amount of data (from my development DB):

[1] pry(main)> AlcoDrink.count
=> 326
[2] pry(main)> AlcoAvail.count
=> 4130
[3] pry(main)> AlcoLocation.count
=> 35

Of course, if we switch to embedding, then the data can be stored in the database in the format that is suitable for our purposes (AlcoAvails embedded in AlcoDrinks, instead of in a separate table like in SQL DB). But what I don't get is how AlcoLocation should be stored in MongoDB so that we could store one location only once in DB, not once per AlcoDrink.

timoh commented 8 years ago

I think this calls for a whiteboarding session.

MongoDB is slower to query than SQL in a general case, yes. MongoDB, on the other hand, is migration free and has a dynamic data model (schemaless). Mongo is thus great for prototyping.

So I would first try to optimize the data model and only if the perf still isn't enough (which I doubt), we can look into switching the whole DB.

timoh commented 8 years ago

It ours is similar to the N+1 problem, some solutions discussed in these posts:

Eager loading for Mongoid:

timoh commented 8 years ago

Yet another solution, which could (and perhaps should?) be combined with the previously mentioned could be pagination of the API call.

Pagination with Rails and Mongoid (ORM):

timoh commented 8 years ago

Also about general schema design:

techstalgist commented 8 years ago

Pagination would definitely improve the user experience as well. How should sorting be implemented with pagination in place? It should sort all drinks, not just the ones visible on the current page. Now sorting has been implemented on the client side.

timoh commented 8 years ago

Sorting a paginated Mongoid model in Rails:

timoh commented 8 years ago
[2] pry(main)> AlcoDrink.all.size
=> 448
[3] pry(main)> AlcoLocation.all.size
=> 35

Relations: 448 drinks x 35 locations => up to 15 680 queries

Actual:

[1] pry(main)> AlcoAvail.all.size
=> 11005

Due to https://github.com/timoh/craftbeer/commit/f50ff1cfe048237dffdc1f221a21b8c08d84709c

With .limit(50) and eager loading + indexing:

Local: Completed 200 OK in 957ms (Views: 394.1ms)
Heroku (app[web.1]): Completed 200 OK in 2403ms (Views: 716.5ms)

Optimal pagination could be perhaps 25 results per page to be around one second processing time on Heroku.

timoh commented 8 years ago

With commit https://github.com/timoh/craftbeer/commit/d7d42d4dbf2dc352df6d052028196f17a4f26c90

Now pagination with 25 drinks per page:

app[web.1]: Completed 200 OK in 1105ms (Views: 314.4ms)

..we're down to that one second on Heroku. Mission accomplished.