szaszm commented on a change in pull request #735: MINIFICPP-1158 - Event
driven processors can starve each other
URL: https://github.com/apache/nifi-minifi-cpp/pull/735#discussion_r378946607
##########
File path: libminifi/src/EventDrivenSchedulingAgent.cpp
##########
@@ -34,26 +31,21 @@ namespace minifi {
uint64_t EventDrivenSchedulingAgent::run(const
std::shared_ptr<core::Processor> &processor, const
std::shared_ptr<core::ProcessContext> &processContext,
const
std::shared_ptr<core::ProcessSessionFactory> &sessionFactory) {
- while (this->running_) {
- bool shouldYield = this->onTrigger(processor, processContext,
sessionFactory);
-
- if (processor->isYield()) {
- // Honor the yield
- return processor->getYieldTime();
- } else if (shouldYield && this->bored_yield_duration_ > 0) {
- // No work to do or need to apply back pressure
- return this->bored_yield_duration_;
- }
-
- // Block until work is available
-
- processor->waitForWork(1000);
-
- if (!processor->isWorkAvailable()) {
- return 1000;
+ if (this->running_ && processor->isRunning()) {
+ auto start_time = std::chrono::steady_clock::now();
+ // trigger processor until it has work to do, but no more than half a sec
+ while (std::chrono::steady_clock::now() - start_time <
std::chrono::milliseconds(500)) {
+ bool shouldYield = this->onTrigger(processor, processContext,
sessionFactory);
+ if (processor->isYield()) {
+ // Honor the yield
+ return processor->getYieldTime();
+ } else if (shouldYield) {
+ // No work to do or need to apply back pressure
+ return (this->bored_yield_duration_ > 0) ? this->bored_yield_duration_
: 10; // No work left to do, stand by
+ }
}
}
- return 0;
+ return 10; // Let's check back for work in 10ms or when a thread is
available to execute
Review comment:
Could you explain the reason behind the changes? It's not obvious from
either the code or the PR title why this is necessary.
The meaning of the return value is not obvious. Judging from the last
comment, it's a duration in milliseconds. I know the lack of comment is not
your fault, but I'd appreciate if you could comment its meaning somewhere near
the function.
There are too many hardcoded durations (10ms, 500ms). Could you explain why
these were chosen?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
