-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Conversation
032222d
to
44201ec
Compare
This still needs fixing. cloner_plugin needs to operate even when there is no streamer. |
44201ec
to
0e1c4d2
Compare
since amqp-cpp is apache licensed, I think we need to include its license file in the install. Toward the bottom of the root CMakeLists.txt is where we tend to add those. |
"RabbitMQ Streams if any; Format: amqp://USER:PASSWORD@ADDRESS:PORT/QUEUE[/ROUTING_KEYS, ...]"); | ||
op("stream-loggers", bpo::value<std::vector<string>>()->composing(), | ||
"Logger Streams if any; Format: [routing_keys, ...]"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some documentation to the PR description that includes these options. Some example uses would be good also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indicate the routing_keys
are eosio::name
cloner_plugin.cpp | ||
streamer_plugin.cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be better as:
file(GLOB SRC *.cpp *.hpp streams/*.hpp)
add_executable( ${RODEOS_EXECUTABLE_NAME}
${SRC}
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do that, you really ought to add CONFIGURE_DEPENDS
. Even then it's not recommended by cmake
We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate. The CONFIGURE_DEPENDS flag may not work reliably on all generators, or if a new generator is added in the future that cannot support it, projects using it will be stuck. Even if CONFIGURE_DEPENDS works reliably, there is still a cost to perform the check on every rebuild.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We glob header files in other CMakeLists.txt and add that in. Is that the preferred method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CONFIGURE_DEPENDS
Actually, this is a cmake 3.12 feature
("q", name)("mc", messagecount)("cc", consumercount)); | ||
}); | ||
queue.onError([](const char* error_message) { | ||
throw std::runtime_error("RabbitMQ Queue error: " + std::string(error_message)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm looking at amqp-cpp correctly you should not throw from inside onError
. I don't think amqp-cpp is setup to handle that correctly. Instead just elog
the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently needed to shutdown amqp-cpp
rabbitmq_handler(boost::asio::io_service& io_service) : AMQP::LibBoostAsioHandler(io_service) {} | ||
|
||
void onError(AMQP::TcpConnection* connection, const char* message) { | ||
throw std::runtime_error("rabbitmq connection failed: " + std::string(message)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm looking at amqp-cpp correctly you should not throw from inside onError. I don't think amqp-cpp is setup to handle that correctly. Instead just elog the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently needed to shutdown amqp-cpp
|
||
if (options.count("stream-rabbits")) { | ||
auto rabbits = options.at("stream-rabbits").as<std::vector<std::string>>(); | ||
initialize_rabbits(app().get_io_service(), my->streams, rabbits); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This plugin really should have its own io_service and not use the main application thread io_service. This is fine for now. I'll create a JIRA issue to fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments.
Hi, I tested this stream function with rabbitmq, the connection drop because server side not receive any heartbeat. It only works when I config server side with heartbeat=0 to disable heartbeat. is this expected ? |
@swang-b1 hmmm so I guess the |
The documentation is deceptive.
That's true if you're using a vanilla But then the asio integration doesn't set up a timer to pump the heartbeat 😞 For now, this fault can be mitigated by doing something like uint16_t onNegotiate(AMQP::TcpConnection *connection, uint16_t interval) override {
return 0;
} in our own handler. That will disable heartbeats on the connection no matter how the server is configured. |
Cool, this will work. |
Change Description
This builds on #9018
This is a proposed Streamer Plugin to publish messages to external brokers. As a reference,
we implement two basic streams: a rabbitmq one, using amqp-cpp lib; and also a simple logger
stream, in case you want to print the filter data to your own logs.
Consensus Changes
API Changes
Documentation Additions
To publish the queues from your filter contract, add the following code that will stream data
with the
push_data
intrinsic:--plugin b1::streamer_plugin
Enables the streamer plugin in
rodeos
instance--stream-rabbits amqp://guest:guest@localhost:5672/myqueue/mykey
Adds a RabbitMQ streaming.
amqp://USER:PASSWORD@HOST:PORT/QUEUE_NAME[/KEY1,KEY2]
amqp://
prefix is fixedUSER
is the rabbitmq userPASSWORD
is the rabbitmq passwordHOST
is the address of rabbitmq hostPORT
is the rabbitmq's portQUEUE_NAME
is the name of the queue that you want to publish the messages/KEY1,KEY2
is a set of routing keys that you want to publish to your queue. the default is empty, which means will publish any routing keys to this key; you can separate keys names by commas to restrict routing keys; the types of the key names areeosio::name
, ex:eos,issue
will publish only the messages that has eithereos
orissue
as its routing key--stream-loggers "*"
Add a Logger streaming
*
it will accept any routing keys--stream-loggers "eos,issue"
Additional Implementations
If you want to add your own streaming plugin, feel free to implement the
stream_handler
class, implementing thevoid publish(const char* data, uint64_t data_size)
method. You will need to add new config params and a way to initialize it in thestreamer_plugin::plugin_initialize
.