add support for more than 40 results

If your search results in more than 40 results you will get:

Too many matches!

The current version of Atlas does not support a result set greater than 40. This is due to some performance issues in doing multiple parallel connections in javascript. Future versions will hopefully manage to overcome this issue.

sample (big relay groups):

https://atlas.torproject.org/#search/contact:39B9820C

https://onionoo.torproject.org/summary?contact=39B9820C

added complete-before-abandoning-globe component::metrics/relay search owner::irl parent::18893 priority::medium resolution::fixed severity::normal status::closed type::defect labels

Trac:
Username: tyseom
Cc: N/A to nusenu@openmailbox.org

Suggestion: Display the first 40 results and indicate in a warning that there are really more than 40 results that cannot all be displayed. In theory and without looking at the code, this should be a simple fix.

If possible, let's fix this before dumping Globe.

Trac:
Sponsor: N/A to N/A
Keywords: N/A deleted, complete-before-abandoning-globe added
Reviewer: N/A to N/A
Severity: N/A to Normal

Please review my branch task-15395. I tested it locally, but it's certainly possibly that I overlooked an edge case or two.

Trac:
Status: new to needs_review

<% if (relays.length >= 40) {%>

Should this actually ever happen? I mean, == 40 is fine, but at most 40 relays will be returned so if more than 40 are returned something has gone horribly wrong.

<% if (i > 40) return true; %>

Again here, if this code path is ever followed, then there is an error occuring in your changes in results.js and at best we're disguising the error.

(These comments are not actually bugs, more just a preference for not having more code than is necessary to achieve the goal and attempting to minimise complexity - my local testing has not thrown up any issues with this patch).

Looking at #17938 (moved), we can use this as a temporary fix bringing Atlas to the same level of functionality as Globe but we really should implement pagination later.

Trac:
Cc: nusenu@openmailbox.org to nusenu@openmailbox.org, irl@fsfe.org

I checked out that branch. The only issue I have with it is that it was considering too many results also the situation where you had exactly 40 relays, since it is truncating the list of relays and assuming that if there are 40 then it's due to truncation and not that this is the value returned by onionoo.

To overcome this I wrote a patch to forward the exact error code for this case into the view. You can find it at this branch: https://github.com/hellais/atlas/commit/e2b19792c8d67077af0db1613fee0cdd5067e042 that is based on top of yours.

So looking back at actually properly fixing this and not just making a workaround, I realised that the true issue lied in the fact that a full page-re-render was being done every time a relay was being looked up.

This obviously lead to terrible performances.

By delaying the rendering of the page of results until all the relays have been looked up there is a noticeable performance benefit: https://github.com/hellais/atlas/commit/4da8ad27333efb72eddcc69bad5fb42c15b740bc.

I think there is probably a better way of doing this by leveraging by doing some two-way data binding in backbone (in angular this is quite trivial to do, but in backbone there a slight bit more work to do it properly: see: http://stackoverflow.com/questions/12604834/two-way-data-binding-in-backbone-js)

The performance is still not ideal above 500 results on my machine.

A search for country:gb is OK but country:de locks up my browser for a while.

I'll merge together the two solutions (fixing the over-rendering and passing along an error if the limit is exceeded) and test, then come back with a final patch for review.

Trac:
Owner: phw to irl
Status: needs_review to accepted

 Allow returning up to 500 search results and error on empty search

 * Added the ability for errors to be passed along when rendering result
   sets, to display an error or warning message while still allowing
   a result set to be rendered.
 * Results are only rendered once all the requests have succeeded (or
   failed), not every time there is a response. This greatly improves
   the performance and now up to 500 results can be rendered reasonably.
 * Limits the result set to a maximum of 500 results, introducing a new
   error message when this number is exceeded. (Fixes: #15395)
 * Introduces a new error message when an empty string is passed and
   fixed the rendering of that error message by displaying it. (Fixes:
   #19372)
 * Thanks to karsten and hellais for their help on this patch. (:

https://github.com/irl/atlas/commit/b2ac2eeed6afbf984124b52d79582b98b82f70d7

I think this solves everything.

Example in action: http://irl.github.io/atlas/#search/country:de

Tested locally quite extensively, and accidentally also fixed #19372 (moved) as a side-effect.

Trac:
Status: accepted to needs_review

The patch looks very reasonable, though I'd like to make one request: can you please change the number 500 back to 40 for now? Here's why: For n results, Atlas makes n+1 requests to the Onionoo server which returns n+1 responses each containing 1 result. That's just crazy. In theory, it should only make 1 request containing n results. We'll have to change the queries that Atlas makes and very likely the way how Atlas processes responses. I'm happy to discuss those changes, but we could easily do that after applying this patch. Except for those change from 40 to 500 this patch looks ready to go. Cool stuff, thanks a lot!

karsten: Would you be happy with 50? This would be the same limit Globe has been using.

I agree that we should be doing just 1 request for the search, and I'd be happy for that bug to be filed after this patch is applied. (:

Patch with 50 as the limit: https://github.com/irl/atlas/commit/feecc4b16ce7427cf9de5b92d986335196576aba

Sure, 50 is as good as 40. Thanks!

The new ticket would have a summary like "Make single request for Onionoo details document" and would say in the description "Rather than making 1 request for an Onionoo summary document and then n requests for full Onionoo details documents, let's make a single request for 1 Onionoo details document. Exact parameters are subject to further discussion but would include limit=51 (50 results max and 1 to see if there are even more), fields=... (all fields we'd like to display in the results table), search=... (whatever we pass to the summary request right now), and maybe more." Let's discuss more details once this patch is merged.

Ah, I didn't say this explicitly above, but the new patch (feecc4b) looks good, to me. I'd say merge. phw, what do you think? Happy to merge and deploy if you're okay with that.

Trac:
Cc: nusenu@openmailbox.org, irl@fsfe.org to nusenu@openmailbox.org, irl@fsfe.org, phw

Trac:
Parent: N/A to #18893 (moved)

Thanks karsten, hellais, and irl! feecc4b is now merged and deployed.

Karsten, I took the liberty of creating the ticket you requested. It's #19452 (moved).

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

mentioned in issue #17938 (moved)

mentioned in issue #19452 (moved)

mentioned in issue #23871 (moved)