Github user knusbaum commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1199#discussion_r55871683
  
    --- Diff: storm-core/src/jvm/org/apache/storm/trident/TridentTopology.java 
---
    @@ -431,6 +459,52 @@ public StormTopology build() {
             
             return builder.buildTopology();
         }
    +
    +    private static Map<String, Number> mergeDefaultResources(Map<String, 
Number> res, Map defaultConfig) {
    +        Map<String, Number> ret = new HashMap<String, Number>();
    +
    +        Number  onHeapDefault = 
(Number)defaultConfig.get(Config.TOPOLOGY_COMPONENT_RESOURCES_ONHEAP_MEMORY_MB);
    +        Number offHeapDefault = 
(Number)defaultConfig.get(Config.TOPOLOGY_COMPONENT_RESOURCES_OFFHEAP_MEMORY_MB);
    +        Number cpuLoadDefault = 
(Number)defaultConfig.get(Config.TOPOLOGY_COMPONENT_CPU_PCORE_PERCENT);
    +
    +        if(res == null) {
    +            ret.put(Config.TOPOLOGY_COMPONENT_RESOURCES_ONHEAP_MEMORY_MB, 
onHeapDefault);
    +            ret.put(Config.TOPOLOGY_COMPONENT_RESOURCES_OFFHEAP_MEMORY_MB, 
offHeapDefault);
    +            ret.put(Config.TOPOLOGY_COMPONENT_CPU_PCORE_PERCENT, 
cpuLoadDefault);
    +            return ret;
    +        }
    +
    +        Number  onHeap = 
res.get(Config.TOPOLOGY_COMPONENT_RESOURCES_ONHEAP_MEMORY_MB);
    +        Number offHeap = 
res.get(Config.TOPOLOGY_COMPONENT_RESOURCES_OFFHEAP_MEMORY_MB);
    +        Number cpuLoad = 
res.get(Config.TOPOLOGY_COMPONENT_CPU_PCORE_PERCENT);
    +
    +        if(onHeap == null) {
    +            onHeap = onHeapDefault;
    +        }
    +        else {
    +            onHeap = Math.max(onHeap.doubleValue(), 
onHeapDefault.doubleValue());
    --- End diff --
    
    Since each component can be the sum of a bunch of operations, the simplest 
thing to do is take the max.
    
    What we don't want to happen is for the user to set low resources on one 
operation, and have that be set for the compound. If *any* component isn't set, 
we want to use the default. 
    
    Right now the code doesn't check that. It just sums them up and takes the 
max for simplicity's sake. We could do something to check if every node had set 
a low value and if the sum of those is less than the default, keep it, but that 
doesn't seem like it has great benefits for the added complexity. The defaults 
are very low as it is, and I'm not sure how many people are setting their 
components with lower than default resource values.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to