Implement Search After based on refactored scroll-chan#52
Open
axel-angel wants to merge 5 commits into
Open
Conversation
Contributor
Author
|
@mpenet Can you take a look and opinion on this? |
axel-angel
force-pushed
the
feature-search-after-scroll
branch
from
7adf2df to
8cd2f2f
Compare
Contributor
Author
|
@mpenet I've revised and made new tests. Can you take a look for merging this? thank you |
rborer
reviewed
Apr 6, 2020
Also ensure we cleanup the testing index when the tests finished
axel-angel
force-pushed
the
feature-search-after-scroll
branch
from
8cd2f2f to
9ebd2cb
Compare
Owner
|
Hi, |
Contributor
Author
|
Hi Max, no problem. Keep me updated or ping me if you need. Thanks. |
Collaborator
|
@mpenet FYI we've been running with this patch in prod since a few days & it's working fine. |
Collaborator
|
👋 😉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We noticed some performance issue because we're solely using ES scroll to retrieve pages of results of our queries for "real-time" requests, however it's strongly discouraged in the doc. Here we opt to implement Search After which is very close to scroll-chan, the difference is the necessity to have a proper ordering through :sort
Note that this PR refactored and simplified scroll-chan plus we fixed a possible bug: we always pick the last scroll, acquired from the last page, which wasn't the case before. As described in the doc, we should always use the lastest scroll.
Other note, We noticed that chan->seq doesn't properly close the chan when the seq isn't entirely consumed. We also simplified it using simple clojure functions. This issue was already here and no idea how to fix it yet :/ Any idea welcomed.