[ 
https://issues.apache.org/jira/browse/METRON-1657?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16538651#comment-16538651
 ] 

ASF GitHub Bot commented on METRON-1657:
----------------------------------------

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

    https://github.com/apache/metron/pull/1099#discussion_r201356953
  
    --- Diff: 
metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/topology/ParserTopologyBuilder.java
 ---
    @@ -91,14 +101,14 @@ public Config getTopologyConfig() {
        */
       public static ParserTopology build(String zookeeperUrl,
                                           Optional<String> brokerUrl,
    -                                      String sensorType,
    -                                      ValueSupplier<Integer> 
spoutParallelismSupplier,
    -                                      ValueSupplier<Integer> 
spoutNumTasksSupplier,
    +                                      List<String> sensorTypes,
    +                                      ValueSupplier<List> 
spoutParallelismSupplier,
    --- End diff --
    
    I actually forgot to ask about this in the main thing.  It's because of the 
use of the Class argument in ValueSupplier breaks the typing involved 
elsewhere. It's the same reason it's ValueSupplier<Map> instead of 
ValueSupplier<Map<Foo, Bar>> elsewhere.
    
    Having said that, it seems like it's entirely unnecessary to have the class 
argument at all and we could just actually type everything everywhere and just 
remove the argument entirely. I tried this out briefly and it seemed fine, but 
I didn't spin it up through full dev since it seemed optional to do.


> Parser aggregation in storm
> ---------------------------
>
>                 Key: METRON-1657
>                 URL: https://issues.apache.org/jira/browse/METRON-1657
>             Project: Metron
>          Issue Type: Bug
>            Reporter: Justin Leet
>            Assignee: Justin Leet
>            Priority: Major
>
> Currently our parsing solution requires one storm topology per sensor. It has 
> been complained that this may be wasteful of resources and that, rather than 
> one storm topology per sensor, it would be advantageous to have multiple 
> sensors in the same topology. The benefit to this is that it would require 
> fewer storm slots.
> The issue with this is that whenever we've aggregated functionality like this 
> before, we've run into issues appropriately being able to scale storm (e.g. 
> batch vs random access indexing in the same topology).  The main point in 
> addressing this is to recommend that parsers with similar velocities and 
> complexity are grouped together.
> Particularly for a first cut, leave the configuration mostly as-is, while 
> allowing for comma separated lists of sensors in start_parser_topology.sh 
> (e.g. bro,yaf creates a aggregated parser consisting of those two). 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to