-
Notifications
You must be signed in to change notification settings - Fork 581
upgrade yarn scheduler driver memory to ByteAmount #1728
base: master
Are you sure you want to change the base?
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // Copyright 2017 Twitter. All rights reserved. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package com.twitter.heron.common.basics; | ||
|
|
||
| public enum ByteAmountUnit { | ||
| BYTE, | ||
| MB, | ||
| GB | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ | |
| import org.apache.reef.tang.annotations.Unit; | ||
| import org.apache.reef.tang.exceptions.InjectionException; | ||
|
|
||
| import com.twitter.heron.common.basics.ByteAmount; | ||
| import com.twitter.heron.scheduler.yarn.HeronMasterDriver.ContainerAllocationHandler; | ||
| import com.twitter.heron.scheduler.yarn.HeronMasterDriver.FailedContainerHandler; | ||
| import com.twitter.heron.scheduler.yarn.HeronMasterDriver.HeronSchedulerLauncher; | ||
|
|
@@ -63,7 +64,7 @@ public class YarnLauncher implements ILauncher { | |
| private String role; | ||
| private String env; | ||
| private String queue; | ||
| private int driverMemory; | ||
| private ByteAmount driverMemory; | ||
| private ArrayList<String> libJars = new ArrayList<>(); | ||
|
|
||
| @Override | ||
|
|
@@ -75,7 +76,7 @@ public void initialize(Config config, Config runtime) { | |
| role = Context.role(config); | ||
| env = Context.environ(config); | ||
| queue = YarnContext.heronYarnQueue(config); | ||
| driverMemory = YarnContext.heronDriverMemoryMb(config); | ||
| driverMemory = YarnContext.heronDriverMemory(config); | ||
|
|
||
| try { | ||
| // In addition to jar for REEF's driver implementation, jar for packing and state manager | ||
|
|
@@ -161,7 +162,7 @@ Configuration getHMDriverConf() { | |
| .set(HeronDriverConfiguration.HTTP_PORT, 0) | ||
| .set(HeronDriverConfiguration.VERBOSE, false) | ||
| .set(YarnDriverConfiguration.QUEUE, queue) | ||
| .set(DriverConfiguration.DRIVER_MEMORY, driverMemory) | ||
| .set(DriverConfiguration.DRIVER_MEMORY, driverMemory.asMegabytes()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused here. Shouldn't
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be honest, it make me confused too. I didn't notice that...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @objmagic - cast it to int
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you can just cast here...?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mycFelix I now think I am not the right person to review this PR. You can think about Bill's review and wait for @ashvina's comment.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @objmagic - Sure~ |
||
| .build(); | ||
| } | ||
|
|
||
|
|
||
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.
I think it would be useful to add javadocs for these overloaded methods.