[Bro-Dev] Broker::publish API
The other day when merging Johanna's code to clusterize the configuration framework, I noticed this code in there: # [Send id=val to everyone else] Broker::publish(change_topic, Config::cluster_set_option, ID, val, location); if ( Cluster::local_node_type() != Cluster::MANAGER ) Broker::relay(change_topic, change_topic, Config::cluster_set_option, ID, val, location); It took me a bit to understand that ... The goal here is that a change in a configuration value gets propagated out to all nodes in the cluster. The Broker::publish() sends it to a node's immediate neighbors, but not further. That means that for workers it goes (only) to their manager; for the manager it means, it goes to all workers. If we're not a manager, we then separately (through Broker::relay()) ask our neighbors (that's the manager) to forward the change to *their* neighbors (that's the other workers), without reraising it locally. I remember we have discussed this API before, but I wanted to bring it up again as I keep finding it confusing. I believe the code above could be simplified by using the newer Broker::publish_and_relay(), which was added to combine the two operations. Still, I'm realizing now that I don't like thinking about this in terms of separate publishing and relaying operations. It all won't become easier once we add multi-hop routing to the mix (which is in the works). And on top of all that, we also have Cluster::publish_rr, Cluster::publish_hew, Cluster::relay_rr, and Cluster::relay_hew -- another set of separate publishing & relay options. I'm wondering if we should give it another try to simply this API while we still can (i.e., before 2.6 goes out). To me, the most intuitive publish operation is "send to topic T and propagate to everybody subscribed to that topic". I'd structure the API around that, making that the main publish function for that simply: Broker::publish(topic, args); That would send to all neighbors, which then process locally and relay to their neighbors. Right now, that would propagate just across one hop but once we have multihop that'd start being broadcasted out broadly. To support the other use cases, we can then add modifiers & functions to tweak this default, e.g.: - Give publish() another argument "relay: bool &default=T" to prevent it from going beyond the immediate receiver. Or maybe instead: "relay_hops: int &default=-1" to specify the max number of hops to relay across, with -1 meaning no limit. (I recall concerns about loops being too easy to create; we could set the default here to F/0 to default to no forwarding, although conceptually I don't really like that :-) - Give publish() another argument "relay_topic: string &default="" to change the topic when relaying on the 1st hop. - Give publish() another argument "process_on_relays: bool &default=T" to change whether a relaying hop also sees the event locally. - Add a second function publish_pool() that has all the same options, but receives a pool type instead of a topic (just an enum: RR, HRW). What I'm not quite sure about is if some of these modifiers are better to leave for the receiver to specify (e.g., whether to raise events received on a given topic locally, or just forward). I think I can see that either way. Robin -- Robin Sommer * Corelight, Inc. * ro...@corelight.com * www.corelight.com ___ bro-dev mailing list bro-dev@bro.org http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
Re: [Bro-Dev] Broker::publish API
On Fri, Jul 27, 2018 at 12:40 PM Robin Sommer wrote: > I'm wondering if we should give it another try to simply this API > while we still can (i.e., before 2.6 goes out). To me, the most > intuitive publish operation is "send to topic T and propagate to > everybody subscribed to that topic". I'd structure the API around > that, making that the main publish function for that simply: > > Broker::publish(topic, args); > > That would send to all neighbors, which then process locally and relay > to their neighbors. Right now, that would propagate just across one > hop but once we have multihop that'd start being broadcasted out > broadly. Can you remind/clarify what's meant by "multihop" ? I thought: Broker already has manual multihop if you set up subscriptions on all relevant nodes on the path yourself. Bro doesn't use it right now. Broker does not yet have automatic multihop where subscriptions are globally flooded automatically. A difference between "manual multihop" and "automatic multihop" would be that in the later, some relaying nodes may not actually hold a subscription to the message they are relaying and so, in the case of Bro events, I think they would not process them locally. > - Give publish() another argument "relay: bool &default=T" to prevent > it from going beyond the immediate receiver. Or maybe instead: > "relay_hops: int &default=-1" to specify the max number of hops > to relay across, with -1 meaning no limit. Going with the generalized approach of configurable number of hops per message via "relay_hops" from the start would be better than finding out we need it later. Possibly a downside is now you need to store original hop limit in addition to current TTL in each message if you want to detect the "is 1st hop" condition for the "relay_topic" option below. > . (I recall concerns > about loops being too easy to create; we could set the default > here to F/0 to default to no forwarding, although conceptually I > don't really like that :-) It's maybe both a concern and a reality -- Bro clusters currently contain cycles (e.g. worker -> manager -> proxy -> worker) > - Give publish() another argument "relay_topic: string &default="" > to change the topic when relaying on the 1st hop. > > - Give publish() another argument "process_on_relays: bool &default=T" > to change whether a relaying hop also sees the event locally. Those seem fine to me. > - Add a second function publish_pool() that has all the same > options, but receives a pool type instead of a topic (just an > enum: RR, HRW). What's the goal of the enums instead of just publish_hrw() and publish_rr() ? Instead of the API being 2 functions, it then seems like 2 enums that are never used elsewhere + 1 function that now always branches internally. - Jon ___ bro-dev mailing list bro-dev@bro.org http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
Re: [Bro-Dev] Broker::publish API
> On Jul 27, 2018, at 1:39 PM, Robin Sommer wrote: > > I'm wondering if we should give it another try to simply this API > while we still can (i.e., before 2.6 goes out). To me, the most > intuitive publish operation is "send to topic T and propagate to > everybody subscribed to that topic". I'd structure the API around > that, making that the main publish function for that simply: > >Broker::publish(topic, args); > > That would send to all neighbors, which then process locally and relay > to their neighbors. Right now, that would propagate just across one > hop but once we have multihop that'd start being broadcasted out > broadly. This would do weird things on workers, since they connect to both the manager and proxies. Worker 1 would send to it's neighbors [manager, proxy1, proxy2] but then those 3 nodes would relay to all of the other workers. The TTL would stop the propagation, but you'd still end up sending 3 copies of the same message to each worker. I do agree that there's room for a lot of simplification, for example a worker broadcasting a message efficiently to all other workers needs to do something like this from the docs: Cluster::relay_rr(Cluster::proxy_pool, "example_key", Cluster::worker_topic, worker_to_workers, Cluster::node + " (via a proxy)"); But a lot of that could have defaults: Most use cases would want to relay through the default proxy pool Since round robin is in use, they key shouldn't matter. The round robin part itself is really an implementation detail for proxy load balancing and maybe not something that should be exposed in the API. Now that I think of it I'm not sure why one would ever use relay_hrw over relay_rr. Removing a lot of that gets close to what you are suggesting: Cluster::relay(Cluster::worker_topic, worker_to_workers, Cluster::node + " (via a proxy)"); which is I guess just Cluster::relay(topic, args) like you said. — Justin Azoff ___ bro-dev mailing list bro-dev@bro.org http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
Re: [Bro-Dev] Broker::publish API
On Fri, Jul 27, 2018 at 3:55 PM Azoff, Justin S wrote: > I do agree that there's room for a lot of simplification, for example a > worker broadcasting a message efficiently to all > other workers needs to do something like this from the docs: > > Cluster::relay_rr(Cluster::proxy_pool, "example_key", > Cluster::worker_topic, worker_to_workers, > Cluster::node + " (via a proxy)"); > > But a lot of that could have defaults: > > Most use cases would want to relay through the default proxy pool > Since round robin is in use, they key shouldn't matter. At the moment, one could write their own wrapper function around that if they find it too verbose and always want to use certain defaults? > The round robin part itself is really an implementation detail for proxy load > balancing and maybe not something that > should be exposed in the API. Now that I think of it I'm not sure why one > would ever use relay_hrw over relay_rr. Theoretically, a more favorable load distribution that's consistent over time? e.g. if you do RR of the same messaging pattern from multiple nodes, you could have waves of "randomly" overlapping loads on the relayer-node since everyone is cycling through all the proxies at their own rate when choosing the relayer. With HRW, you'd stick with the same relayer over time and only change on outages, but everyone should have chosen their relayer in a uniformly distributed fashion. - Jon ___ bro-dev mailing list bro-dev@bro.org http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
Re: [Bro-Dev] Broker::publish API
> On Jul 27, 2018, at 6:10 PM, Jon Siwek wrote: > > On Fri, Jul 27, 2018 at 3:55 PM Azoff, Justin S wrote: > >> I do agree that there's room for a lot of simplification, for example a >> worker broadcasting a message efficiently to all >> other workers needs to do something like this from the docs: >> >>Cluster::relay_rr(Cluster::proxy_pool, "example_key", >> Cluster::worker_topic, worker_to_workers, >> Cluster::node + " (via a proxy)"); >> >> But a lot of that could have defaults: >> >> Most use cases would want to relay through the default proxy pool >> Since round robin is in use, they key shouldn't matter. > > At the moment, one could write their own wrapper function around that > if they find it too verbose and always want to use certain defaults? Yeah.. The wrapper would be trivial.. Should bro include it so that the API scripts use is simpler? >> The round robin part itself is really an implementation detail for proxy >> load balancing and maybe not something that >> should be exposed in the API. Now that I think of it I'm not sure why one >> would ever use relay_hrw over relay_rr. > > Theoretically, a more favorable load distribution that's consistent > over time? e.g. if you do RR of the same messaging pattern from > multiple nodes, you could have waves of "randomly" overlapping loads > on the relayer-node since everyone is cycling through all the proxies > at their own rate when choosing the relayer. With HRW, you'd stick > with the same relayer over time and only change on outages, but > everyone should have chosen their relayer in a uniformly distributed > fashion. > > - Jon I'd expect that round robin would give the most uniform load distribution, for N proxies each proxy would see 1/N relay messages, but I guess in general round robin isn't the best load balance mechanism since it doesn't take into account the responsiveness of each proxy. With some of the information CAF provides it may be possible to also support weighted round robin. That way if a proxy node doesn't die outright but starts having issues for one reason or another, relay_rr could avoid sending it messages. — Justin Azoff ___ bro-dev mailing list bro-dev@bro.org http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
