-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-27916: Increase tez.am.resource.memory.mb for TestIcebergCliDrver to 512MB #4907
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
Conversation
ayushtkn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| <property> | ||
| <name>tez.am.resource.memory.mb</name> | ||
| <value>256</value> | ||
| <value>512</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if that is required, as there were no issues upstream so far, but it won't harm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, should we drop hive.tez.container.size from tez and llap tez-site.xml? the same thing is set in hive-site.xml but with a lower limit - 128 vs 512
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hive.tez.container.size is for task containers in particular (it's used to create the Resource object which is provided to yarn), whereas tez.am.resource.memory.mb is for the Application Master, so basically, it would make sense to review it in (only) case of llap drivers' config, as it's confusing: LLAP is not Tez container mode
regarding duplicated stuff in hive-site.xml and tez-site.xml...yeah, it's confusing too, let me check
regarding upstream failures: yeah, I cannot recall precommit issue in particular, however there were some in downstream, so as you said: it won't harm
|
Kudos, SonarCloud Quality Gate passed!
|
|
closing this, please find comment in jira: https://issues.apache.org/jira/browse/HIVE-27916?focusedCommentId=17796287&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17796287 |
|
other confusing stuff regarding hive.tez.container.size will be handled in HIVE-27937 |
|
looks like we need this change: downstream I was able to reproduce AM OOM flakiness which disappeared with this fix |
d66f0d0 to
212093e
Compare
|
apache#4907) (Laszlo Bodor reviewed by Ayush Saxena)
apache#4907) (Laszlo Bodor reviewed by Ayush Saxena)










What changes were proposed in this pull request?
Increasing tez.am.resource.memory.mb for tez based iceberg cli driver.
Why are the changes needed?
To improve test stability.
Does this PR introduce any user-facing change?
No.
Is the change a dependency upgrade?
No.
How was this patch tested?
Precommit.