smarter ordering for queries that don't use zk first #935

merged 3 commits into from Mar 10, 2016


None yet
1 participant

ssalinas commented Mar 4, 2016

Currently, when searching anything where queryUsesZkFirst returns false, all results from zk are still initially in our list of returned items before fetching from history. So, after sorting, regardless of page, the zk results will likely be on the page and much of the persisted history will not be shown. In short, anything that hasn't been persisted yet will likely show up on every page of task search results.

This PR implements two things:

  • taskHistoryQueryUsesZkFirst option -> if true this would have every query for task history use zk items first. This will only require a single history query, and thus be much faster, with the caveat that anything not persisted will always appear before anything that has already persisted in the general order (but paging will be correct)
  • If taskHistoryQueryUsesZkFirst is false we will make similar initial queries for items from zk and from persisted history. But, after ordering them, we will check that we have fetched all relevant items from the history. If we have not (i.e. some non-persisted items in the limitStart/limitCount range are beyond the range of all persisted items we have fetched), then we will fetch additional items to ensure correct ordering of all items, but at the added cost of additional queries needed.

My overall thought on this was that we can at least give the user the choice of efficiency/speed vs ensuring correct ordering.

Only other thing I wasn't sure on was whether to make taskHistoryQueryUsesZkFirst available on the query as well to give each individual user making a query the option as well as being abel to set the system-wide default.

/cc @wsorenson @tpetr @Calvinp

@ssalinas ssalinas added the hs_staging label Mar 7, 2016

@ssalinas ssalinas added the hs_qa label Mar 8, 2016


This comment has been minimized.

Show comment
Hide comment

ssalinas Mar 10, 2016


Going to merge this into task_search since both are now in qa branch


ssalinas commented Mar 10, 2016

Going to merge this into task_search since both are now in qa branch

ssalinas added a commit that referenced this pull request Mar 10, 2016

Merge pull request #935 from HubSpot/task_search_zk_first
smarter ordering for queries that don't use zk first

@ssalinas ssalinas merged commit fefb101 into task_search Mar 10, 2016

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
continuous-integration/travis-ci/push The Travis CI build failed

@ssalinas ssalinas deleted the task_search_zk_first branch Mar 10, 2016

@ssalinas ssalinas modified the milestone: 0.4.12 Apr 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment