Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Protect window operator by circuit breaker #1006

Conversation

dai-chen
Copy link
Member

@dai-chen dai-chen commented Jan 25, 2021

Issue #, if available: #1009

Description of changes: Protect window operator to avoid preload too many data into memory in worst case mentioned here: https://github.com/opendistro-for-elasticsearch/sql/blob/develop/docs/dev/AggregateWindowFunction.md#33-performance

  1. Wrap window operator's input node by resource monitor
  2. Check resource usage on every 1000 calls on next()

window-oprator-cb

Testing: Add UT to verify the behavior as expected. Load test will be done separately later.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@dai-chen dai-chen added the SQL label Jan 25, 2021
@dai-chen dai-chen self-assigned this Jan 25, 2021
@codecov
Copy link

codecov bot commented Jan 25, 2021

Codecov Report

Merging #1006 (9b74809) into develop (df61641) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #1006   +/-   ##
==========================================
  Coverage      99.87%   99.87%           
- Complexity      2403     2409    +6     
==========================================
  Files            234      234           
  Lines           5520     5527    +7     
  Branches         357      358    +1     
==========================================
+ Hits            5513     5520    +7     
  Misses             5        5           
  Partials           2        2           
Impacted Files Coverage Δ Complexity Δ
...tor/protector/ElasticsearchExecutionProtector.java 100.00% <100.00%> (ø) 20.00 <5.00> (+3.00)
...search/executor/protector/ResourceMonitorPlan.java 100.00% <100.00%> (ø) 10.00 <4.00> (+3.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df61641...9b74809. Read the comment docs.

@dai-chen dai-chen marked this pull request as ready for review January 26, 2021 22:51
@dai-chen dai-chen merged commit 614c500 into opendistro-for-elasticsearch:develop Jan 29, 2021
@dai-chen dai-chen deleted the protect-window-operator-input branch January 29, 2021 20:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants