Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 111
FALCON-2112 Set property value to set map memory for replication and retention#264
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
9d0072278b98d544e168c759e8894f9b32cdd93d271180b15e59b3b5c7a7941d603fa9File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -37,8 +37,28 @@ | ||
| <name>oozie.launcher.oozie.libpath</name> | ||
| <value>${wf:conf("falcon.libpath")}</value> | ||
| </property> | ||
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. If you are setting the launcher memory for others, better be consistent and set it here too. ContributorAuthor 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. Ack. I will put launcher memory here too. | ||
| <property> | ||
| <name>oozie.launcher.yarn.app.mapreduce.am.resource.mb</name> | ||
| <value>${amMemory}</value> | ||
| </property> | ||
| <property> | ||
| <name>oozie.launcher.yarn.app.mapreduce.am.command-opts</name> | ||
| <value>${amCommandOpts}</value> | ||
| </property> | ||
| <property> | ||
| <name>oozie.launcher.mapreduce.map.memory.mb</name> | ||
| <value>${mapMemory}</value> | ||
| </property> | ||
| <property> | ||
| <name>oozie.launcher.mapreduce.map.java.opts</name> | ||
| <value>${mapJavaOpts}</value> | ||
| </property> | ||
| </configuration> | ||
| <main-class>org.apache.falcon.retention.FeedEvictor</main-class> | ||
| <arg>-Dmapreduce.map.memory.mb={amMemory}</arg> | ||
| <arg>-Dmapreduce.map.memory.mb={amCommandOpts}</arg> | ||
| <arg>-Dmapreduce.map.memory.mb={mapMemory}</arg> | ||
| <arg>-Dmapreduce.map.memory.mb={mapJavaOpts}</arg> | ||
| <arg>-feedBasePath</arg> | ||
| <arg>${feedDataPath}</arg> | ||
| <arg>-falconFeedStorageType</arg> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -48,6 +48,10 @@ public abstract class FeedReplicationWorkflowBuilder extends OozieOrchestrationW | ||
| protected static final String REPLICATION_ACTION_NAME = "replication"; | ||
| private static final String MR_MAX_MAPS = "maxMaps"; | ||
| private static final String MR_MAP_BANDWIDTH = "mapBandwidth"; | ||
| private static final String MR_AM_MEMORY = "amMemory"; | ||
| private static final String MR_AM_COMMAND_OPTS = "amCommandOpts"; | ||
| private static final String MR_MAP_MEMORY = "mapMemory"; | ||
| private static final String MR_MAP_JAVA_OPTS = "mapJavaOpts"; | ||
| private static final String REPLICATION_JOB_COUNTER = "job.counter"; | ||
| private static final String TDE_ENCRYPTION_ENABLED = "tdeEncryptionEnabled"; | ||
| @@ -96,6 +100,18 @@ protected Properties getWorkflowProperties(Feed feed) throws FalconException { | ||
| if (props.getProperty(MR_MAP_BANDWIDTH) == null) { // set default if user has not overridden | ||
| props.put(MR_MAP_BANDWIDTH, getDefaultMapBandwidth()); | ||
| } | ||
| if (props.getProperty(MR_AM_MEMORY) == null) { // set default app master memory if user has not overridden | ||
| props.put(MR_AM_MEMORY, getDefaultAmMemory()); | ||
| } | ||
| if (props.getProperty(MR_AM_COMMAND_OPTS) == null) { // set default app maseter opts if user has not overridden | ||
| props.put(MR_AM_COMMAND_OPTS, getDefaultAmCommandOpts()); | ||
| } | ||
| if (props.getProperty(MR_MAP_MEMORY) == null) { // set default map memory if user has not overridden | ||
| props.put(MR_MAP_MEMORY, getDefaultMapMemory()); | ||
| } | ||
| if (props.getProperty(MR_MAP_JAVA_OPTS) == null) { // set default map java opts if user has not overridden | ||
| props.put(MR_MAP_JAVA_OPTS, getDefaultMapJavaOpts()); | ||
| } | ||
| return props; | ||
| } | ||
| @@ -145,6 +161,24 @@ private String getDefaultMapBandwidth() { | ||
| return RuntimeProperties.get().getProperty("falcon.replication.workflow.mapbandwidth", "100"); | ||
| } | ||
| private String getDefaultAmMemory() { | ||
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. It would be better to move these method to OozieOrchestrationWorkflowBuilder and use it from there in FeedRetentionWorkflowBuilder and FeedReplicationWorkflowBuilder rather than doing same thing twice. ContributorAuthor 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. These settings are not meant for other builders like ProcessWorfklowBuilder, might give wrong conscience of being available for other builders. Thats why it has been put into these two builders specifically. In case if we want to achieve the same for process builder too then we can move it into OrchestrationWorkflowBuilder. 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. Still its better to move it to OrchestrationWorkflowBuilder to avoid repetition of code.We can override the method in ProcessWorfklowBuilder and throw an exception method not supported if donot want to use them there. ContributorAuthor 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. Its just not ProcessWorkflowBuilder , there are data import, export and hive builders. 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. As discussed please remove the method and use it inline in both the places. | ||
| return RuntimeProperties.get().getProperty("falcon.feed.workflow.yarn.app.mapreduce.am.resource.mb", "512"); | ||
| } | ||
| private String getDefaultAmCommandOpts() { | ||
| return RuntimeProperties.get().getProperty("falcon.feed.workflow.yarn.app.mapreduce.am.command-opts", | ||
| "-Xmx400m -XX:MaxMetaspaceSize=64m"); | ||
| } | ||
| private String getDefaultMapMemory() { | ||
| return RuntimeProperties.get().getProperty("falcon.feed.workflow.mapreduce.map.memory.mb", "512"); | ||
| } | ||
| private String getDefaultMapJavaOpts() { | ||
| return RuntimeProperties.get().getProperty("falcon.feed.workflow.mapreduce.map.java.opts", | ||
| "-Xmx400m -XX:MaxMetaspaceSize=64m"); | ||
| } | ||
| private boolean isTDEEnabled() { | ||
| String tdeEncryptionEnabled = FeedHelper.getPropertyValue(entity, TDE_ENCRYPTION_ENABLED); | ||
| return "true" .equalsIgnoreCase(tdeEncryptionEnabled); | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
nit: just to keep it consistent either start it from the same line or move every property name to a separate line.
Right now mapBandwith is in one line and rest of setting is in form of paragraph.
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.
Ack,will make the changes accordingly.