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

Solr typeahead #1537

Merged
merged 44 commits into from
Nov 16, 2017
Merged

Solr typeahead #1537

merged 44 commits into from
Nov 16, 2017

Conversation

jywarren
Copy link
Member

@jywarren jywarren commented Jul 13, 2017

May need a toggle, however, as in #1463

Fixes #1654

  • all tests pass -- rake test:all
  • code is in uniquely-named feature branch, and has been rebased on top of latest master (especially if you've been asked to make additional changes)
  • pull request is descriptively named with #number reference back to original issue

@PublicLabBot
Copy link

PublicLabBot commented Jul 13, 2017

3 Messages
📖 @jywarren Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.
📖 This pull request doesn’t link to a issue number. Please refer to the issue it fixes (if any) in the body of your PR, in the format: Fixes #123.
📖 It looks like you haven’t marked all the checkboxes. Help us review and accept your suggested changes by going through the steps one by one. If it is still a ‘Work in progresss’, please include ‘[WIP]’ in the title.

Generated by 🚫 Danger

@jywarren
Copy link
Member Author

@jywarren
Copy link
Member Author

@icarito - not sure, i added a new field here, do we need to update the schema when we do that?

@jywarren
Copy link
Member Author

May still need a toggle to switch between solr and non solr, and tests for this service.

@jywarren
Copy link
Member Author

OK -- so now, this is working (probably, once my last commit runs) but it takes a timeout to determine if Solr is working... which is too long.

Here, we only need to know if a Solr service is available and we can toggle between -- but we need to know /fast/. Ideas, @icarito ?

@jywarren
Copy link
Member Author

Here's the toggle area:

https://github.com/publiclab/plots2/pull/1537/files#diff-1bc1c213d7a40db44441cd43fd8cd96cR35

It depends on an attempt to run a Node.search in the SolrToggle library we built in /lib/ -- but that has to time out before determining which way to go. That'll take forever and also occupy a zillion threads if we put it in production.

@icarito
Copy link
Member

icarito commented Jul 17, 2017

Is there a possibility to just instead of making a pre-query, try the actual query and fallback only if it fails?

@icarito
Copy link
Member

icarito commented Aug 3, 2017

The big thing (...) is trying to find a faster way than a timeout to determine if Solr is present (from in a rails app). I think that may be a sunspot question and could be searched on Stack overflow and/or posted on sunspot's issue tracker.

Looking for ideas.

@jywarren
Copy link
Member Author

OK, no longer seeing the timeout, but resolving a failure and some errors. Added a check to ensure solr is not available when the test failure occurs.

@jywarren
Copy link
Member Author

Our plan is to simply monitor publiclab.org/searches/test instead of having a fallback on production.

@icarito
Copy link
Member

icarito commented Aug 24, 2017 via email

@jywarren
Copy link
Member Author

Well, solrAvailable is evaluating to true, but not returning the blog nodes. Can we reproduce this in an interactive environment to see why?

@icarito
Copy link
Member

icarito commented Aug 24, 2017 via email

@jywarren
Copy link
Member Author

Yes, lets. We should be able to use /searches/test?q=blog and other queries to ensure Solr is actually working. for now, i'll also output a blank solr query to the log just to see.

@jywarren
Copy link
Member Author

They should actually both be for Solr, and I'm wondering if the extra parameters makes it return nothing... we'll see -- if this passes, could you push it to unstable? Thanks!

@icarito
Copy link
Member

icarito commented Nov 15, 2017

I see what you mean! It might very well be, here's the logged query for both cases. I'll push to unstable when it passes.

solr_1  | 1549882 INFO  (qtp1112280004-15) [   x:default] o.a.s.c.S.Request [default] webapp=/solr path=/select params={q=about&defType=edismax&qf=title_text+body_text+comments_text&fl=*+score&start=0&fq=type:Node&fq=status_s:1&fq=type_s:note&sort=updated_at_s+desc&rows=5&wt=ruby} hits=2003 status=0 QTime=3 

solr_1  | 1582962 INFO  (qtp1112280004-19) [   x:default] o.a.s.c.S.Request [default] webapp=/solr path=/select params={q=test&defType=edismax&qf=title_text+body_text+comments_text&fl=*+score&start=0&fq=type:Node&rows=30&wt=ruby} hits=1960 status=0 QTime=1

@jywarren
Copy link
Member Author

This is odd, and I'm restarting the build -- it looks to be stuck on rake db:setup -- may just be an intermittent Travis issue?

@jywarren
Copy link
Member Author

Promising! It returned 2 results instead of 3 for a couple tests. I think that means the Solr query is working now; I adjusted the test assertion to match. Next I can re-enable some of the extra Solr parameters to see which caused the issue... hopefully there are, for example, pagination defaults we can lean on.

@icarito
Copy link
Member

icarito commented Nov 15, 2017

Pushed to unstable!

@jywarren
Copy link
Member Author

Ug! Unstable still doesn't show results. @icarito, could you paste in the most recent solr queries from the logs and we can try to see why no results are returned?

@jywarren
Copy link
Member Author

Huh, interesting! One where query works but not the other?

@jywarren
Copy link
Member Author

OK, so the only line that makes the tests fail is with :type, "note" -- that's odd.

Oh, we don't index that!

https://github.com/publiclab/plots2/blob/master/app/models/node.rb#L24-L39

Changing that to see.

@jywarren
Copy link
Member Author

Oh wait, that's weird - we do index that, at least in this branch. Never mind, I'm not sure what's going on here...

@jywarren
Copy link
Member Author

@icarito -- finished this; pushing it to unstable too.

@icarito
Copy link
Member

icarito commented Nov 16, 2017 via email

@jywarren
Copy link
Member Author

testing it in unstable now. If this passes, can you merge it and push it to stable? Thanks!

@jywarren
Copy link
Member Author

It works in unstable:
screenshot 2017-11-16 at 2 37 15 pm

@jywarren jywarren merged commit eb2e7b6 into master Nov 16, 2017
@jywarren
Copy link
Member Author

🍔 🔨 🍬 🔥 🎆

@icarito
Copy link
Member

icarito commented Nov 16, 2017

Reindexing production now in advance of deploying this.

@icarito
Copy link
Member

icarito commented Nov 16, 2017

Reindexing stable too.

@icarito
Copy link
Member

icarito commented Nov 16, 2017

Deployed to production!

@icarito
Copy link
Member

icarito commented Nov 16, 2017

https://publiclab.org/api/typeahead/all?srchString=precocious no results, not sure why

jywarren added a commit that referenced this pull request Nov 16, 2017
* Solr typeahead (#1537)

* Solr typeahead

May need a toggle, however.

* Update typeahead_service.rb

Not yet using limit...

* Update typeahead_service.rb

* Update node.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* service reworking

* toggle and test; very slow

* note tweak

* tweaks

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update and rename test/unit/typeahead_service_test.rb to test/solr/typeahead_service_test.rb

* Update typeahead_service_test.rb

* Create typeahead_service_test.rb

* Update Gemfile

* Delete typeahead_service_test.rb

* Update typeahead_service_test.rb

* Re-disable solr in test mode

* Re-add regular test of typeahead and confirm toggle fallback

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update rusers.yml

* Update typeahead_service.rb

* Re-adding sunspot.yml overwriting

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Testing disabling extra Solr fields

* Update typeahead_service_test.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead.rb

* Update restful_typeahead.js

* new gemfile
SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
* Solr typeahead

May need a toggle, however.

* Update typeahead_service.rb

Not yet using limit...

* Update typeahead_service.rb

* Update node.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* service reworking

* toggle and test; very slow

* note tweak

* tweaks

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update and rename test/unit/typeahead_service_test.rb to test/solr/typeahead_service_test.rb

* Update typeahead_service_test.rb

* Create typeahead_service_test.rb

* Update Gemfile

* Delete typeahead_service_test.rb

* Update typeahead_service_test.rb

* Re-disable solr in test mode

* Re-add regular test of typeahead and confirm toggle fallback

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update rusers.yml

* Update typeahead_service.rb

* Re-adding sunspot.yml overwriting

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Testing disabling extra Solr fields

* Update typeahead_service_test.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead.rb

* Update restful_typeahead.js
SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
* Solr typeahead (publiclab#1537)

* Solr typeahead

May need a toggle, however.

* Update typeahead_service.rb

Not yet using limit...

* Update typeahead_service.rb

* Update node.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* service reworking

* toggle and test; very slow

* note tweak

* tweaks

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update and rename test/unit/typeahead_service_test.rb to test/solr/typeahead_service_test.rb

* Update typeahead_service_test.rb

* Create typeahead_service_test.rb

* Update Gemfile

* Delete typeahead_service_test.rb

* Update typeahead_service_test.rb

* Re-disable solr in test mode

* Re-add regular test of typeahead and confirm toggle fallback

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update rusers.yml

* Update typeahead_service.rb

* Re-adding sunspot.yml overwriting

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Update typeahead_service_test.rb

* Testing disabling extra Solr fields

* Update typeahead_service_test.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead_service.rb

* Update typeahead.rb

* Update restful_typeahead.js

* new gemfile
@emilyashley emilyashley deleted the solr-typeahead branch January 15, 2020 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants