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

When using default max result window value for size, we should account for offsets #2441

Merged
merged 1 commit into from
Nov 5, 2021
Merged

When using default max result window value for size, we should account for offsets #2441

merged 1 commit into from
Nov 5, 2021

Conversation

rebeccahum
Copy link
Contributor

@rebeccahum rebeccahum commented Nov 4, 2021

Description of the Change

Offset is not accounted for when assigning the default size below for comment queries:

$number = apply_filters( 'ep_max_results_window', 10000 );
}
$formatted_args = [
'from' => 0,
'size' => $number,
];

In not doing so, we could potentially hit the ep_max_results_window if an offset is passed in. Therefore, if we are using the default value for ep_max_results_window, we should account for the sum of offset and size being equal (or less than) the maximum results window.

From https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html:

index.max_result_window The maximum value of from + size for searches to this index. Defaults to 10000. Search requests take heap memory and time proportional to from + size and this limits that memory.

Alternate Designs

N/A.

Benefits

itsover1000

Possible Drawbacks

N/A.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Changelog Entry

Fixed: When using offset and default maximum result window value for size, subtract offset from size

@felipeelia felipeelia merged commit db1c2a7 into 10up:develop Nov 5, 2021
@rebeccahum rebeccahum deleted the rebecca/account_for_offset_in_size_for_comments branch November 5, 2021 14:35
@felipeelia felipeelia added this to the 3.6.5 milestone Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants