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

    https://github.com/apache/storm/pull/1199#discussion_r55755494
  
    --- Diff: storm-core/src/jvm/org/apache/storm/trident/Stream.java ---
    @@ -124,6 +124,31 @@ public Stream parallelismHint(int hint) {
         }
     
         /**
    +     * Sets the CPU Load resource for the current node
    +     */
    +    public Stream setCPULoad(Number load) {
    --- End diff --
    
    I can look at adding a method for including a component -> resources map 
when creating a trident topology. 
    
    As for the others:
    There's not really a clean way to add this to TridentTopology, and we 
wanted some way to specify resources similar to RAS's API. Stream is the 
cleanest place for it, and it works just like `parallelismHint(...)` does.
    
    Having one method, 'nodeConfig(...)' with a `NodeConfig` object would 
reduce the number of new methods in Stream, like you said. I don't really see 
that as looking any better for the end-user, though. Consider this:
    ```java
    TridentTopology topo = new TridentTopology();
    TridentState wordCounts = 
        topology.newStream("spout1", spout)
            .setCPULoad(50)
            .setMemoryLoad(1024)
            .each(new Fields("sentence"), new Split(), new Fields("word"))
            .setCPULoad(100)
            .setMemoryLoad(256)
            .groupBy(new Fields("word"))
            .persistentAggregate(new MemorymapState.Factory(), new Count(), new 
Fields("count"))
            .parallelismHint(6);
    ```
    
    Would become this:
    ```java
    TridentTopology topo = new TridentTopology();
    TridentState wordCounts = 
        topology.newStream("spout1", spout)
            .nodeConfig(new NodeConfig()
                            .setCPULoad(50)
                            .setMemoryLoad(1024))
            .each(new Fields("sentence"), new Split(), new Fields("word"))
            .nodeConfig(new NodeConfig()
                            .setCPULoad(100)
                            .setMemoryLoad(256))
            .groupBy(new Fields("word"))
            .persistentAggregate(new MemorymapState.Factory(), new Count(), new 
Fields("count"))
            .parallelismHint(6);
    ```
    
    I may be missing the benefit here, but as far as I can tell, users 
shouldn't really need to be aware of Nodes at all. They're supposed to be 
internal to Trident.


---
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