Skip to content

Add an index setting to limit the maximum number of slices allowed in a scroll request. #18782

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

Merged
merged 1 commit into from
Jun 10, 2016
Merged

Add an index setting to limit the maximum number of slices allowed in a scroll request. #18782

merged 1 commit into from
Jun 10, 2016

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Jun 8, 2016

This is a follow up for #18237 (comment)

/**
* The maximum number of slices allowed in a scroll request
*/
public static final Setting<Integer> MAX_SLICES_PER_SCROLL = Setting.intSetting("index.max_slices_per_scroll", 10000, 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should default it to the number of shards :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be very conservative here by default

@s1monw
Copy link
Contributor

s1monw commented Jun 8, 2016

left some comments! thanks for doing this!

@jimczi
Copy link
Contributor Author

jimczi commented Jun 8, 2016

@s1monw I pushed another commit to address your comments. The default maximum number of slices is now equal to the number of shards in the requested index.

@s1monw
Copy link
Contributor

s1monw commented Jun 9, 2016

@s1monw I pushed another commit to address your comments. The default maximum number of slices is now equal to the number of shards in the requested index.

this might actually not work 😞 if we go through an alias we may have way more slices than shards in a single index. and then we fail half way. So I think we need something different (as you had)? Maybe 1024 ? @jpountz WDTY - sorry for not thinking about it earlier

@jimczi
Copy link
Contributor Author

jimczi commented Jun 9, 2016

Don't _shrink my scroll ! ;)
Yep aliases are dangerous. The simple fact that each slice is an independent request is dangerous.
You may also argue that the index could be deleted and re-created between two sliced scroll.
+1 for 1024 and I'll add a note in the docs about aliases and index creation

@jpountz
Copy link
Contributor

jpountz commented Jun 9, 2016

+1 to a fixed limit, I think it is easier to reason about

@jimczi
Copy link
Contributor Author

jimczi commented Jun 10, 2016

@jpountz @s1monw I changed the default to 1024.

@s1monw
Copy link
Contributor

s1monw commented Jun 10, 2016

LGTM

@jimczi jimczi merged commit 39a7bec into elastic:master Jun 10, 2016
@jimczi jimczi deleted the max_slices_per_scroll branch June 10, 2016 07:49
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Scroll labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories v5.0.0-alpha4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants