-
Notifications
You must be signed in to change notification settings - Fork 594
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()) | ||
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 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... 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 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...? 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. 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.