7

In my controller action, I included all associations needed by the view, to avoid multiple calls to the database. (I'm trying to isolate the the views layer to only render the data collected by the controller).


I'v found out that the view still communicates with the database (17 Queries): MiniProfiler


These 17 extra queries are not needed. Since I have tested the controller queries from the console, and it successfully collects all the data needed by the partial _dropdown (in 5 Queries) without any further database communication.

Here is the query in my controller, It meants to avoid the N+1 problem. (Including all the variables called in the view)


Here is the dropdown code:

- @messages.each do |message|
    %li.conversation-container
        %a{href: conversation_path(message.conversation_id)}
            - if message.sender != current_user 
                .notification-avatar{style: "background: url(#{message.sender.avatar_url}); background-size: contain; background-repeat: no-repeat; background-position: 50% 50%;"}
            - else
                - other_participant = message.conversation.conversation_participants.select{|p| p.user_id != current_user.id }.first.user 
                .notification-avatar{style: "background: url(#{other_participant.avatar_url}); background-size: contain; background-repeat: no-repeat; background-position: 50% 50%;"}
            %p
                %strong
                    - if message.sender != current_user 
                        = message.sender.name
                    - else
                        = other_participant.name
                %br
                - if message.sender == current_user
                    %i.fa.fa-mail-reply-all
                = truncate(message.body,length: 25)

                .time
                    = time_ago_in_words(message.created_at)
                    ago
- if @messages.count == 0
    %li
        .empty-state-text-white
            No messages

Output from console:

2.0.0-p353 :006 > ms = Message.dropdown_for(3).all
  Message Load (1.2ms)  SELECT "messages".* FROM "messages" LEFT JOIN messages AS m ON messages.id != m.id 
 AND m.conversation_id = messages.conversation_id 
 AND messages.created_at < m.created_at INNER JOIN conversation_participants AS cp ON cp.conversation_id = messages.conversation_id AND cp.user_id = 3 WHERE (m.id IS NULL) ORDER BY cp.seen , cp.updated_at DESC LIMIT 5
  User Load (0.7ms)  SELECT "users".* FROM "users" WHERE "users"."id" IN (6, 4, 5)
  Conversation Load (0.4ms)  SELECT "conversations".* FROM "conversations" WHERE "conversations"."id" IN (4, 2, 3)
  ConversationParticipant Load (0.2ms)  SELECT "conversation_participants".* FROM "conversation_participants" WHERE "conversation_participants"."conversation_id" IN (4, 2, 3)
  User Load (0.6ms)  SELECT "users".* FROM "users" WHERE "users"."id" IN (6, 3, 4, 5)
 => [#<Message id: 8, body: "saSasa", sender_id: 6, conversation_id: 4, sent: true, attachment_id: nil, attachment_type: nil, created_at: "2014-11-17 16:05:40", updated_at: "2014-11-17 16:05:40">, #<Message id: 2, body: "asdnas dagsdashjdg jahs d", sender_id: 4, conversation_id: 2, sent: true, attachment_id: nil, attachment_type: nil, created_at: "2014-11-17 11:32:36", updated_at: "2014-11-17 11:32:36">, #<Message id: 6, body: "SADASD A DSA ", sender_id: 5, conversation_id: 3, sent: true, attachment_id: nil, attachment_type: nil, created_at: "2014-11-17 13:43:34", updated_at: "2014-11-17 13:43:34">] 

2.0.0-p353 :007 > ms.first.conversation.conversation_participants.select{|cp| cp.user_id != 3}.first.user
 => #<User id: 6, first_name: "Ddsfsd", middle_name: nil, last_name: "Fsdfsd", photo: nil, email: "1@k.com", encrypted_password: "$2a$10$5sGIb2DbQ1ctMrTzD3AJ0uV18hhiC5Ei1wcfE7MSAvRU...", reset_password_token: nil, reset_password_sent_at: nil, remember_created_at: nil, sign_in_count: 1, current_sign_in_at: "2014-11-17 15:27:06", last_sign_in_at: "2014-11-17 15:27:06", current_sign_in_ip: "127.0.0.1", last_sign_in_ip: "127.0.0.1", confirmation_token: nil, confirmed_at: "2014-11-17 15:27:48", confirmation_sent_at: "2014-11-17 15:27:05", unconfirmed_email: nil, failed_attempts: 0, unlock_token: nil, locked_at: nil, authentication_token: nil, created_at: "2014-11-17 15:27:05", updated_at: "2014-11-17 15:27:48", slug: "ddsfsd_fsdfsd"> 

2.0.0-p353 :008 > ms.count
 => 3 

How can I stop these queries from running without a purpose?

Community
  • 1
  • 1
mohameddiaa27
  • 3,587
  • 1
  • 16
  • 23
  • Actually I have tried to disconnect from console, everything is working as expected, but I get an error when I add it in my controller (accessing same attributes) from the server – mohameddiaa27 Nov 14 '14 at 21:03
  • 1
    could you please post the code for your index and dropdown view files and the conversations controller index action? – konyak Nov 17 '14 at 17:51
  • Rails scopes let you build up queries and execute when you use them. To execute, you can call .all, .count, .each or .first. Have you tried adding `.all` to the end of your query in the controller action? – konyak Nov 17 '14 at 19:04
  • Yes I did, also `to_a`. The thing is preloading seems to take more queries than it should. – mohameddiaa27 Nov 17 '14 at 19:16
  • 1
    From the log, could you paste the 17 queries that you want to eliminate / don't think they're needed? You could just paste one of the queries if they're all about the same. – konyak Nov 17 '14 at 19:59
  • I've posted the same query from console, showing no extra database communication while calling the associations. – mohameddiaa27 Nov 17 '14 at 20:13
  • I expected those 5 select statements, but don't see the 17 extra queries. Do you have any (before/after) filters that tie to your controller action or any other code in your controller action? – konyak Nov 17 '14 at 22:13
  • These generated queries disappear when I comment the `@messages` line. I'm starting to think its a problem with profiler or my rails version. – mohameddiaa27 Nov 17 '14 at 22:25
  • Instead of comparing `message.sender` to `current_user`, compare `message.sender_id == current_user.id`: one query less per message? :) Also: store the result in a variable, and do the test only once. – nathanvda Nov 18 '14 at 09:37
  • The sender is already preloaded, as I need the `sender.name` – mohameddiaa27 Nov 18 '14 at 11:30
  • Thanks Everyone, I finally found the solution to my problem. I really appreciate your effort. – mohameddiaa27 Nov 19 '14 at 22:11

6 Answers6

6

* Debugging

Well after debugging every possible factor that could cause this issue. I have tried to set config.cache_classes to true, in my development.rb. This successfully removed all the extra queries.

I concluded that (by default) the schema is not loaded for any model when the classes are not cached. In other words, when config.cache_classes is set to false, the schema for each model is loaded for every request as a separate query.

Here is a similar issue column_definitions method being called before and after every single SQL statement on PostgreSQL.

* Conclusion

cache_classes should be set to false in development environment. Ignore the extra internal queries from postgresql connection adapter loading the schema for each model since it not going to affect your production environment (production has config.cache_classes set to true).

mohameddiaa27
  • 3,587
  • 1
  • 16
  • 23
  • config.cache_clases has nothing to do with queries. What I have provided is the correct solution and should not cause the queries to become 18. – Arkhitech Nov 20 '14 at 01:26
  • @Arkhitech your solution is nothing other than an extra query. I've tested it although I know its a wrong solution. I'm using postgresql, checkout the similar issue. – mohameddiaa27 Nov 20 '14 at 01:31
  • hmmm...I don't have the environment setup to manually test this, but config.cache_classes cannot possible have any effect on the queries. – Arkhitech Nov 20 '14 at 01:35
  • Loading the class (model) schema appears as a query in mini-profiler – mohameddiaa27 Nov 20 '14 at 01:37
  • 1
    Issue was reported to mini-profiler here: [link1](https://github.com/SamSaffron/MiniProfiler/issues/116) and [link2](https://github.com/SamSaffron/MiniProfiler/issues/166) – konyak Nov 20 '14 at 14:35
3

You can try bullet gem which will tell you is there any N+1 prolem in query. If there is no problem of N+1 problem then you should try to implement fragment caching.

Jagdish
  • 752
  • 8
  • 23
  • How could this occur only from the server (In console everthing is going fine) ? – mohameddiaa27 Nov 16 '14 at 21:05
  • Try rails-footnotes gem which will show all queries of views as well controller with line numbers. https://github.com/josevalim/rails-footnotes – Jagdish Nov 16 '14 at 21:11
2

Simple question. Have you tried put a .to_a in the end of your method call? Like @messages.to_a ?

Rudy Seidinger
  • 1,059
  • 13
  • 22
  • This is really embracing that I've not tried `to_a`. Now the `17` moved to the controller, and the `5` disappeared, If I figured out how to eliminate the useless ones, I'm gonna give you the bounty. Thanks for the hint. – mohameddiaa27 Nov 17 '14 at 15:50
  • Glad i could help you, in the end! Let us know! – Rudy Seidinger Nov 17 '14 at 17:26
2

I would check the log to see what those 17 queries are, or perhaps clicking on the 17 sql link will show those queries. From there, you may be able to see that you forgot to includes a table that causes the N+1 problem.

EDIT:

As noted in the 'Lazy Loading' section of this site, you can add .all to the end of your query in your controller action to trigger its execution, and prevent the query from lazily executing in your view. As mentioned in my comment, Rails scopes let you build up queries and execute when you use them. To execute, you can call .all, .count, .each or .first. In Rails 4, you can use .load to execute the query in the controller.

konyak
  • 10,818
  • 4
  • 59
  • 65
  • In Rails 4+, `.all` will build an `ActiveRecord::Relation` object and not trigger the query instantly. Instead, you'll need to call `to_a` on your query. – Thomas Klemm Nov 17 '14 at 19:52
  • true, however for Rails 4, I would use `all.load` to return an ActiveRecordRelation over `to_a`. – konyak Nov 17 '14 at 20:17
1

This may be so called 'N + 1' problem, it happens due to lazy loading. I can't say for sure without application log. You can use eager loading as described here.

sashaegorov
  • 1,821
  • 20
  • 26
1

Your query is not well formulated. You should either use includes or joins.

Break down your query into two as follows:

message_ids = Message.joins("LEFT JOIN messages AS m ON messages.id != m.id 
          AND m.conversation_id = messages.conversation_id 
          AND messages.created_at < m.created_at")
   .where('m.id IS NULL')
   .joins("INNER JOIN conversation_participants AS cp 
          ON cp.conversation_id = messages.conversation_id 
          AND cp.user_id = #{user_id}")
   .order("cp.seen, cp.updated_at DESC")
   .limit(5).map(&:id)

messages = Message.includes(:sender).
    includes(conversation: [{conversation_participants: :user}]).
    where(id: message_ids)
Arkhitech
  • 475
  • 3
  • 11
  • Well, I respect your approach, but this adds 1 more query. I can't exactly understand what is the difference between console and server. – mohameddiaa27 Nov 17 '14 at 21:32
  • I didn't say anything about console and server. Basically, not able to use joins with includes is a limitation of ActiveRecords. What I have provided you is the best and most correct solution. You just have one additional query instead of the 17 you originally had and that is how you develop these solutions. – Arkhitech Nov 18 '14 at 08:00
  • 1
    Well your approach made them 18. And this is not a limitation if you look at the console output. – mohameddiaa27 Nov 18 '14 at 11:27