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

Add a softlimit to the number of open scroll contexts per node #25244

Closed
s1monw opened this issue Jun 15, 2017 · 12 comments
Closed

Add a softlimit to the number of open scroll contexts per node #25244

s1monw opened this issue Jun 15, 2017 · 12 comments
Labels
>enhancement good first issue low hanging fruit help wanted adoptme resiliency :Search/Search Search-related issues that do not fall into other categories

Comments

@s1monw
Copy link
Contributor

s1monw commented Jun 15, 2017

Today if a user creates a large number of scroll contexts without clearing them using a scroll ID we go and clean things up once a timeout is reached. This has several problems:

  • the user doesn't necessarily realize an API call to release resources is necessary
  • the node piles up open readers behind the scenes that can have a big impact on disk space and monitoring.
  • the node might go OOM due to too many open resources
  • the node might suffer from too many open files

we do protect against some of these but it would be nicer to reject scroll requests / new sticky context creation if there are too many of them. This change is rather a low hanging fruit but I suspect a big impact if a users has the related problems. A rejected scroll search would immediately be visible to the user.

@s1monw
Copy link
Contributor Author

s1monw commented Jun 15, 2017

relates to #11511

@nik9000
Copy link
Member

nik9000 commented Jun 15, 2017

Should the limit only apply to scrolls and not regular searches? Should the number of ongoing regular searches count or just scrolls?

@s1monw
Copy link
Contributor Author

s1monw commented Jun 15, 2017

Should the limit only apply to scrolls and not regular searches? Should the number of ongoing regular searches count or just scrolls?

I first thought scrolls only but we can just keep it simple and also so all of them. I already have a branch that does all.

@humblefool1997
Copy link

humblefool1997 commented Aug 7, 2017

@s1monw
I am up for it .Will Submit a patch within a week .This is my first pr.Can you suggest from where to look on.

@alexshadow007
Copy link
Contributor

@s1monw
Is SearchOperationListener::onNewScrollContext(SearchContext context) good place to add verifying for limit or not?

@s1monw
Copy link
Contributor Author

s1monw commented Oct 6, 2017

@alexshadow007 I wonder if we should rather put it directly into SearchService where we create the context?

@colings86 colings86 added the :Search/Search Search-related issues that do not fall into other categories label Apr 24, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@pushpavanthar
Copy link

@s1monw, @isaeef is this already fixed? Can I pick this up?

@s1monw
Copy link
Contributor Author

s1monw commented Jul 5, 2018

@pushpavanthar it's not fixed afaik

@RuiGuedes
Copy link

RuiGuedes commented Nov 12, 2018

Hi @pushpavanthar I would like to pick up this issue. Are you still working on it?

@pushpavanthar
Copy link

pushpavanthar commented Nov 12, 2018

@RuiGuedes No, please go ahead.

@geekpete
Copy link
Member

It might be nice to have different limits for different scroll activities, like two pools one for fast/short lived scrolls and another for longer running scrolls.
Then a way to do a scroll against one or other of the scroll types to differentiate between "regular" or short running scrolls and "special" or longer running scrolls that would take up a slot for longer.

The idea might be to have a much smaller longer running scroll max limit to keep those long open scroll contexts to a minimum number vs a larger max limit for short running scrolls.

This would also need some default if there was more than one way to scroll.
A configurable number of pools with configurable settings would also be quite nice to have.

For bonus points, it'd be awesome to be able to tie scroll limits into Security and limit resource usage based on security characteristics like role or user. I realise this might be a separate feature but might be good to keep in mind when designing the basic functionality as part of this feature.

jimczi pushed a commit that referenced this issue Dec 3, 2018
This change adds a soft limit to open scroll contexts that can be controlled with the dynamic cluster setting `search.max_open_scroll_context` (defaults to 500).
jimczi pushed a commit to jimczi/elasticsearch that referenced this issue Dec 3, 2018
This change adds a soft limit to open scroll contexts that can be controlled with the dynamic cluster setting `search.max_open_scroll_context` (defaults to 500).
jimczi added a commit that referenced this issue Dec 4, 2018
This change adds a soft limit to open scroll contexts that can be controlled with the dynamic cluster setting `search.max_open_scroll_context` (defaults to unlimited).

Relates #36009
spinscale pushed a commit that referenced this issue Dec 4, 2018
This change adds a soft limit to open scroll contexts that can be controlled with the dynamic cluster setting `search.max_open_scroll_context` (defaults to 500).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement good first issue low hanging fruit help wanted adoptme resiliency :Search/Search Search-related issues that do not fall into other categories
Projects
None yet
Development

No branches or pull requests

9 participants