[
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)
_______________________________________________
bro-dev mailing list
bro-dev@bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
