shans / chromez

Apache License 2.0
2 stars 18 forks source link

Increase maximum number of Monorail results to 500 #96

Closed suzyh closed 7 years ago

suzyh commented 7 years ago

By default, Monorail returns 100 issues in a query. Our bug components contain more than 100 issues. Although the best thing to do here would be to implement some kind of pagination, increasing the maximum result size is a simple solution that does not require the same implementation effort.

shans commented 7 years ago

I'm OK with this in principle, but we used to have issues with monorail timing out. Have you tested that a timeout isn't something that gets hit regularly after this change?

suzyh commented 7 years ago

No, because this modifies the redirect, I cannot easily test it locally since the redirect goes to the live version, not my modified one. We could presumably increase the urlfetch deadline. What would you suggest is the best approach here?

shans commented 7 years ago

Given that Doug had similar concerns in #38, we should do pagination. From memory, the urlfetch deadline is at 10 seconds which is too long already.

The simplest approach for now would be to have cz-crbug-issues update the params' cursor value in onResponse (https://github.com/shans/chromez/blob/master/client/cz-crbug-issues.html#L28) until all the issues have been fetched (updating params will automatically kick off another AJAX request). This might need modification of the server too, but I think that the cursor is part of the query so it probably won't. onResponse should hold off on delivering the issues to the next layer until they've all been fetched (later we can add a pagination mode to the rest of the dashboard too).

See https://github.com/shans/crt/blob/master/client/bulk-index.html#L139 for an example in practice.

suzyh commented 7 years ago

Yes, pagination is the right way to do this. However, I am not going to have time to work on it beyond this small patch. As it is, the dashboard shows incomplete and misleading information, and has done for several months as no one has time to devote to implementing pagination. I'd like to request that we submit this and address the potential timeout issues later. Another option may be to exclude all Update-Quarterly issues from the results to bring the total down under 100.

shans commented 7 years ago

If there are still timeouts then they result in no data, which is worse than showing that there are some bugs out of SLA. I don't think we should do this if we can't at least prove that timeouts aren't common first.

suzyh commented 7 years ago

It doesn't show that there are some bugs out of SLA; it just misses some bugs entirely. According to the dashboard, Blink>CSS has no Update-Fortnightly bugs at all.

suzyh commented 7 years ago

If we can't agree on a cheap way to increase the number of bugs returned from Monorail, @dstockwell please modify https://gist.githubusercontent.com/dstockwell/94736914d8df1b48e496c07269ac4d5e/raw/config.json to specify "Blink>CSS": { "tag": "cz-crbug-issues", "query": {"component": "Blink>CSS", "-Update": "Quarterly"} } and update the dashboard to point to the new config.

dstoc commented 7 years ago

Should be updated, the URL doesn't change.

suzyh commented 7 years ago

Oh, thanks. In attempting to replicate what you'd done, I'd ended up with a URL pinned to a particular version of the gist, and hadn't realised that your config URL was not so pinned.