markap14 commented on code in PR #11730:
URL: https://github.com/apache/nifi/pull/11730#discussion_r4330075964
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java:
##########
@@ -1504,6 +1504,11 @@ public void addConnection(final Connection connection) {
} finally {
writeLock.unlock();
}
+
+ scheduler.notifySchedulingEvent(connection.getSource());
Review Comment:
[Claude Opus 5.5] Thanks for catching this. Changing a connection's
destination doesn't notify the scheduler, so a running automatic Processor that
becomes the new destination won't register a wake-up listener on that queue. I
looked at the effect, though, and I don't think it's worth adding code for.
The Processor still picks up the FlowFiles. Its readiness checks read the
live list of incoming connections, so the moved queue is included straight
away. The only thing missing is the instant wake-up: when a FlowFile arrives on
that queue, an idle worker isn't woken. It finds the FlowFile on its next
fallback check, which happens at most one second later. Under steady load
there's no delay, because busy workers don't wait between invocations. The
listeners are registered again the next time the Processor is restarted or any
of its connections are added or removed.
It also only applies when a connection is moved onto a Processor that's
already running with automatic scheduling. The old destination has to be
stopped before the move. Given that the worst case is up to a second of extra
delay in that one situation, I'd rather keep the code as it is.
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExtractText.java:
##########
@@ -60,6 +61,8 @@
@SideEffectFree
@SupportsBatching
@InputRequirement(Requirement.INPUT_REQUIRED)
+// A maximum-sized content buffer is allocated eagerly for every
ProcessContext concurrency slot.
+@AllowsAutoScheduling(false)
Review Comment:
[Claude Opus 5.5] Thanks for checking these. I don't think opting them out
is worth it, because the buffers they allocate per concurrency slot are never
used by the concurrent tasks.
Both Processors do create `ByteBufferPool(context.getMaxConcurrentTasks(),
bufferSize)`. But the only user of that pool is the single
`DatagramChannelDispatcher` thread, which takes one buffer when it starts and
keeps it until it stops. The `onTrigger` tasks never touch the pool. With
automatic scheduling, the extra cost is 11 unused buffers of the Receive Buffer
Size: 65,507 bytes by default, and a UDP datagram can't be larger than that.
That comes to about 700 KB per Processor at the default limit of 12 concurrent
tasks.
`ExtractText` and `ExtractGrok` are different. Their buffer is the Max
Buffer Size, which is often much larger, and every concurrent task actually
uses one.
The unused UDP buffers are also wasted under timer-driven scheduling
whenever Concurrent Tasks is above 1. If we want to fix that, I'd rather size
the pool to a single buffer in a separate change than turn off automatic
scheduling for these Processors.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
