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

[export saved objects] Use scroll API when exporting all #5586

Closed
wants to merge 6 commits into from

Conversation

jbudz
Copy link
Member

@jbudz jbudz commented Dec 7, 2015

Export all is throwing errors because the default max window size was changed to 10k and we are using Math.pow(2, 31) - 1. Currently the search API is used, this adds and uses a scan method.

This could be refactored to a shared method, however it does follow the existing files as is. Throwing it up now for feedback, undecided how far to take it.

Other alternatives

  1. Export all up to 10,000
  2. Configure the kibana index to allow a size of 2^31-1
  3. Make a scan API on the kibana server - held off on this for now until our conventions have solidified

Closes #5524

@spalger
Copy link
Contributor

spalger commented Dec 8, 2015

The elasticsearch-js client could really use a scanAndScrollAndMap() method... This is a lot of copy paste. Can you please share some of this across implementations, even if it's just a ui/util for now?

return scanner.scanAndMap(queryString, {
pageSize,
docCount: Number.POSITIVE_INFINITY
}, this.mapHits.bind(this));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use (hit) => this.mapHits(hit) vs function binding

@epixa
Copy link
Contributor

epixa commented Dec 9, 2015

Is this fix not relevant to 4.2? If it is, it should be backported to that as well.

@jbudz
Copy link
Member Author

jbudz commented Dec 9, 2015

Adding 4.2 label, good catch. Anything ES 2.1+

@epixa
Copy link
Contributor

epixa commented Dec 9, 2015

@jbudz This only supports es 2.1+? If so, it can't go into 4.2 (which is 2.0)

@spalger
Copy link
Contributor

spalger commented Dec 9, 2015

LGTM actually tried it, doesn't work, won't buy again

@epixa
Copy link
Contributor

epixa commented Dec 9, 2015

Ignore my last comment - this will still work on es 2.0, it just wasn't an issue with 2.0.

@spalger
Copy link
Contributor

spalger commented Dec 9, 2015

LGTM

@w33ble
Copy link
Contributor

w33ble commented Dec 9, 2015

LFunctionallyGTM

@elasticsearch-bot
Copy link

Jon Budzenski merged this into the following branches!

Branch Commits
4.3 bdf4877, 9abbd28, 466c695, d5a5c1b, 4ed2753, dba3704
4.x 7a780e4, 1c2f50a, eb821db, 7f6a661, d21cbae, c64a97d
master 142800c, 53b82fb, c60be49, d6addc5, ab0f014, b004690

jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
@jbudz jbudz closed this in 142800c Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
jbudz added a commit that referenced this pull request Dec 9, 2015
@jbudz jbudz mentioned this pull request Dec 9, 2015
@epixa epixa mentioned this pull request May 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants