[Bro-Dev] [JIRA] (BIT-1319) topic/jsiwek/broker

Jon Siwek (JIRA) jira at bro-tracker.atlassian.net
Tue Mar 3 11:06:00 PST 2015


    [ https://bro-tracker.atlassian.net/browse/BIT-1319?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=19906#comment-19906 ] 

Jon Siwek commented on BIT-1319:
--------------------------------

{quote}
cmake/RequireCXX11.cmake says: TODO: don't seem to be any great/easy ways to get a clang version string. Isn't that as easy as: clang --version | grep ^clang | cut -d ' ' -f 3 ?
{quote}

On mac:

$ clang --version
Apple LLVM version 6.0 (clang-600.0.56) (based on LLVM 3.5svn)

The suggested grep/cut doesn't work on that.  I'm not that excited about parsing a version string whose format differs across platforms or maybe even across time just to give a nice error message than a compilation failure.  It's also not the common case: new versions of CMake will be able to directly supply compiler version information and I used that instead if it's available.

I just want to avoid telling someone with a valid compiler that it won't work when it actually will.

{quote}
in the Bro docs for the Broker interface, I think it would be helpful to revert the order of the consumer/producer examples to show producer/consumer instead. In particular for the Store example, it took me a bit to realize some missing context is really in the 2nd script.
{quote}

Ok, I'll take a look.  I organized it as listen() side comes before the connect() side just to avoid adding the little extra complexity to the doc (or tests) to explain that the connect() side will do automatic reconnect attempts and emit warnings when it fails, but I wasn't thinking in terms of producer/consumer.

{quote}
Store::create_clone(“name”): I'm not quite sure how to interpret this in terms of which peer this goes out to: is it cloning any store of that name, independent of the peer? What if two peers both happen to have a store with that name? Should the function explicitly specify the peer instead?
{quote}

Master data store names are meant to be unique, so the first peer who told us it has a store by that name wins.  Any subsequent peers that try to register a store with the same name will fail and the error will show up in reporter.log.  Maybe it's a bit clumsy to handle those types of error programmatically; it is technically possible, but I figure most of the time that will be the type of programmer-error you debug and fix once the first time you run new code.

Don't think it would be hard to change it to Store::create_clone(“name”, peer) in order to require the master counterpart be located on the given peer, but maybe that just gives another chance for programmer-error to slip in by specifying the wrong peer/endpoint?

{quote}
two tests don't terminate for me (the 2nd one I have to kill, presumably because it doesn't use btest-bg-wait)
[  0%] comm.clone_store ... failed
[ 33%] comm.master_store ...^C
{quote}

Thanks, I'll take a look.

{quote}
I was wondering about namespaces for the broker parts, both script-land and C++. I'm kind of inclined to just call it Broker, and maybe BrokerComm and BrokerStore in script-land. That way it's clear what it's about. The script framework would then also become broker.
{quote}

I don't have much preference.  I went with the generic "comm" in case it ended up that the interface was good, but the implementation was bad, then you could come up with yet another library to silently replace broker as if it never happened :).  But maybe it's just clearer to have "broker" in the namespaces for users to make the right associations at the moment.

So I'll switch to your naming suggestion unless there's other ideas.

{quote}
The script API for the Store looks a bit cumbersome to use, because of the async interface through when. Could we add sync versions of the various functions that just go to the local cache? Or does that not work architecturally with how the communication between Bro/Broker/CAF is structured?
{quote}

Blocking versions can be added, but some caution is still probably needed when using them because even though it goes to the local cache, queries are still processed via a queue of all other data store operations and I don't think there's a good way to tell what the current load is.  So I think you could unknowingly block for longer than you'd want if the store is severely overloaded.

I also think the "when" interface is a bit cumbersome, but maybe also "good habit".  Let me know if you want the blocking version of the data store queries.

{quote}
I also wondered about this: Comm::refine_to_string(Comm::vector_lookup(res$result, 0))); That also looks somewhat cumbersome, but I haven't further traced down yet what exactly is happening there.
{quote}

Looks like a data store query returned a vector of strings and this is retrieving the first element of that.  i.e. it's translating broker data types to bro data types.  The two aren't similar enough to simply "cast" res$result to a ``vector of string`` (if we had a way to cast, or made one).  e.g. broker vectors don't necessarily contain homogenous data types.

> topic/jsiwek/broker
> -------------------
>
>                 Key: BIT-1319
>                 URL: https://bro-tracker.atlassian.net/browse/BIT-1319
>             Project: Bro Issue Tracker
>          Issue Type: New Feature
>          Components: Bro
>            Reporter: Jon Siwek
>            Assignee: Jon Siwek
>             Fix For: 2.4
>
>
> The "topic/jsiwek/broker" branch is in the bro and cmake repos to add the initial support for Broker.
> Notes/Disclaimers/Caveats:
> - Bro has a --enable-broker configure flag.
> - requires actor-framework "develop" branch.  When version 0.13 is out, I will put that as a requirement in the README and have CMake check for that.
> - no C bindings yet
> - no Python bindings yet
> - other than checking compilation that the new unit tests pass on Linux/FreeBSD/Mac, I've not done must extensive of testing, profiling, optimization etc.



--
This message was sent by Atlassian JIRA
(v6.4-OD-15-055#64014)



More information about the bro-dev mailing list