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

Update default value of useMaxMemoryEstimates for Hadoop jobs #16280

Merged
merged 4 commits into from
Apr 26, 2024

Conversation

kfaraz
Copy link
Contributor

@kfaraz kfaraz commented Apr 13, 2024

Description

The useMaxMemoryEstimates parameter was introduced as a way to switch between old and new methods of estimating memory footprint while building an OnHeapIncrementalIndex. The default value of this parameter has been false (i.e. use newer more accurate estimates) for several Druid releases now, and has been found to be very stable.

The changes in this PR are meant to align the usage of this parameter in Hadoop ingestion with native ingestion. Eventually, the parameter useMaxMemoryEstimates will be completely removed from Druid.

Changes

  • Add optional tuning config for Hadoop jobs: useMaxMemoryEstimates
  • Use default value of false to align with current native batch ingestion

@kfaraz kfaraz requested a review from abhishekrb19 April 24, 2024 06:33
Copy link
Member

@kgyrtkirk kgyrtkirk left a comment

Choose a reason for hiding this comment

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

there seem to be some typo in the builder methods

Co-authored-by: Zoltan Haindrich <kirk@rxd.hu>
@kfaraz
Copy link
Contributor Author

kfaraz commented Apr 25, 2024

Thanks for catching the missing parameter, @kgyrtkirk !

@kfaraz kfaraz merged commit 4b6748b into apache:master Apr 26, 2024
87 checks passed
@kfaraz kfaraz deleted the update_default_mem_estimate branch April 26, 2024 02:37
@clintropolis
Copy link
Member

i wonder if there is actually much risk for hadoop or if we can remove this parameter even sooner

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.

3 participants