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

qids optimization on /wiki/tag/method ? #8242

Open
Tracked by #10665
jywarren opened this issue Aug 4, 2020 · 11 comments
Open
Tracked by #10665

qids optimization on /wiki/tag/method ? #8242

jywarren opened this issue Aug 4, 2020 · 11 comments
Labels
hall-of-fame help wanted requires help by anyone willing to contribute refactorization Ruby

Comments

@jywarren
Copy link
Member

jywarren commented Aug 4, 2020

Hi @Tlazypanda -- i found this line which results in a query for 1600 records in our production db, for pages like https://publiclab.org/wiki/tag/method

@qids = Node.questions.where(status: 1)
.collect(&:nid)
if @qids.empty?
@notes = nodes
@questions = []
else
@notes = nodes.where('node.nid NOT IN (?)', @qids) if @node_type == 'note'
@questions = nodes.where('node.nid IN (?)', @qids) if @node_type == 'questions'
end

I think it's worth hunting through the blame history to see what this is all about: https://github.com/publiclab/plots2/blame/92834ead6c53d6d9431bb99024d4044b282518be/app/controllers/tag_controller.rb#L131-L139

I think there must be a better way to do it, and/or a way to cache it. I think what it's trying to do is to filter out questions (a node of type = 'note' but with a tag starting with question:_____) from the results. But it's a big query and will only get larger as the db grows. Surely we can fix this!

What do you think? How does it look on Skylight?

@jywarren jywarren added the help wanted requires help by anyone willing to contribute label Aug 4, 2020
@jywarren jywarren added this to the Performance Summer 2020 milestone Aug 4, 2020
@Tlazypanda
Copy link
Collaborator

Hey @jywarren Sure, I will look into this 👍

@Tlazypanda
Copy link
Collaborator

Tlazypanda commented Aug 7, 2020

Hey @jywarren @cesswairimu @sagarpreet-chadha I went through the issue and initially I didn't exactly understand why we had this specific way of querying here but now after some inspection I figured its because the definition of question in our models is not defined and it represents all notes having atleast one question: tag which should be more suited by all notes having question: tags for all normal tags present.

So we need to query from the nodes all the nodes which have at least one tag with like question: I tried something but its not working can you give me some clarity as to how we can query this so far I am thinking -

having('COUNT(term_data.name LIKE ?', 'question:%)>0') but I am not sure how I can access all the tags from a given node and then check (are there any available methods to get all tags or anything that might help?) Also is it possible to write this query in raw sql or it is mandatory to use the rails orm?

Thanks ✌️

@sagarpreet-chadha
Copy link
Contributor

Hey @Tlazypanda ,
Yes we can write in sql also, but we prefer to write in rails ORM so that new contributors having rails knowledge but not SQL can easily contribute.
Also I think each node has_many tags, can you check this in the Node model file. Thanks 🎉

@ebarry
Copy link
Member

ebarry commented Aug 10, 2020

@Tlazypanda thank you for going through this investigation, i wanted to ask if a plain question tag creates a question the same way a question:foo powertag does?

@Tlazypanda
Copy link
Collaborator

Hey @ebarry according to the code for fetching the questions we match it against all nodes having atleast one tag starting with question: so I don't think the question tag should create a question the same way a question:foo does since it's missing : but @jywarren could answer this better 😅 since I am still learning my way around the architecture of the project 😂

@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2020

Hi, just noting this again as a good one to optimize. I think we can instead of using qids as a filter, we can just port in the standard way to filter by like question: as @Tlazypanda suggested.

@qids = Node.questions.where(status: 1)
.collect(&:nid)
if @qids.empty?
@notes = nodes
@questions = []
else
@notes = nodes.where('node.nid NOT IN (?)', @qids) if @node_type == 'note'
@questions = nodes.where('node.nid IN (?)', @qids) if @node_type == 'questions'
end

Here's how we should do it:

plots2/app/models/node.rb

Lines 948 to 951 in cbb807b

Node.where(type: 'note')
.joins(:tag)
.where('term_data.name LIKE ?', 'question:%')
.group('node.nid')

@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2020

So it might be like:

if @node_type == 'note'
  @notes = nodes
     .joins(:tag) 
     .where('term_data.name LIKE ?', 'question:%') # here i'm not sure -- we need to /exclude/ questions... so...
     .group('node.nid')
elsif @node_type == 'questions' 
   @questions = nodes
     .joins(:tag) 
     .where('term_data.name LIKE ?', 'question:%') 
     .group('node.nid')
end 

The tough part here is the query for notes that excludes questions. We may need a custom query, something like this???

https://stackoverflow.com/a/711671

SELECT apps.ApplicationName, apps.isavailable 
FROM dbo.Applications apps
WHERE apps.ApplicationName = @AppName
    AND NOT EXISTS 
( SELECT * 
  FROM Holidays 
  WHERE ApplicationId = apps.ApplicationId
     AND CONVERT(VARCHAR,getdate(),101) = CONVERT(VARCHAR,holidaydate,101)
)

It may need some experimentation to get this right!!!

@jywarren
Copy link
Member Author

jywarren commented Dec 7, 2020

I think this is our top target for optimization now! It affects all tag pages, i believe.

@daemon1024
Copy link
Member

If all we want to do is filter the nodes without question we can use something like

if @node_type == 'note'
  @notes = nodes
     .joins(:tag) 
     .where.not('term_data.name LIKE ?', 'question:%') # Since we need to exclude the question...
     .group('node.nid')

Referencing the docs here

cc @jywarren

@TildaDares TildaDares mentioned this issue Jan 18, 2022
4 tasks
@anirudhprabhakaran3
Copy link
Member

Hi! Can I look into this?

@govindgoel
Copy link
Member

@anirudhprabhakaran3 yeah go ahead!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hall-of-fame help wanted requires help by anyone willing to contribute refactorization Ruby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants