MINIFICPP-2398 Only commit once per time_slice in EventDrivenSchedulingAgent #1815
Conversation
|
Thanks @lordgamez for noticing some issues with the PR, I converted into draft while I iron out those things (it still got some problems as the failing docker tests seems relevant to the changes) |
martinzink
force-pushed
the
MINIFICPP-2398
branch
from
ca6dc73
to
d27d0ce
Compare
martinzink
force-pushed
the
MINIFICPP-2398
branch
from
d27d0ce
to
1b23b67
Compare
|
Since we are in the discussion about deprecating the whole JNI extension, I think we safely ignore the clang tidy issues coming from that |
| metrics_->addLastOnTriggerRuntime(std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start)); | ||
| start = std::chrono::steady_clock::now(); | ||
| session->commit(); | ||
| metrics_->addLastSessionCommitRuntime(std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start)); |
There was a problem hiding this comment.
The session runtime metric seems to be missing after the changes
There was a problem hiding this comment.
I've moved that logic into the (imho more fitting place) ProcessSession::commit
https://github.com/apache/nifi-minifi-cpp/pull/1815/files#diff-d55088efe0f1ad2a600dba414bfee749c5fb634ab44418146623f892bd6756a4R807-R922
| process_session->setMetrics(processor->getMetrics()); | ||
|
|
||
|
|
||
| try { |
There was a problem hiding this comment.
as I understand SchedulingAgent::trigger catches any exception from Processor::onTrigger and wraps it in an expected, but we do not rethrow that, so the error-case for this try-catch is when process_session->commit() throws, previously SchedulingAgent::trigger wrapped both the Processor::onTrigger and commit/rollback exceptions and EventDrivenSchedulingAgent simply discarded it
gsl::finally should happen in case of an exception, but in this case I don't think we want to call commit when we get an exception here, so I would just move process_session->commit() after the while-loop, also we should make sure to log any exceptions from both commit and rollback
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically main)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.