[GitHub] [nifi] markobean commented on a diff in pull request #6506: NIFI-10243: allow ControlRate to throttle on combination of data rate or flowfile rate

[GitBox <gi...@apache.org>]

markobean commented on code in PR #6506:
URL: https://github.com/apache/nifi/pull/6506#discussion_r1014324326


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestControlRate.java:
##########
@@ -258,10 +282,245 @@ public void testNonExistingGroupAttribute() throws InterruptedException {
         runner.assertQueueEmpty();
     }
 
+    @Test
+    public void testIncreaseDataRate() throws InterruptedException {
+        final TestRunner runner = TestRunners.newTestRunner(new ControlRate());
+        runner.setProperty(ControlRate.RATE_CONTROL_CRITERIA, ControlRate.DATA_RATE);
+        runner.setProperty(ControlRate.MAX_RATE, "11 B");
+        runner.setProperty(ControlRate.TIME_PERIOD, "1 sec");
+
+        runner.enqueue("test data 1");
+        runner.enqueue("test data 2");
+        runner.enqueue("test data 3");
+        runner.enqueue("test data 4");
+        runner.enqueue("test data 5");
+        runner.enqueue("test data 6");
+
+        runner.run(7, true);
+
+        runner.assertTransferCount(ControlRate.REL_SUCCESS, 1);
+        runner.assertTransferCount(ControlRate.REL_FAILURE, 0);
+        runner.assertQueueNotEmpty();
+
+        // Increase rate after stopping processor. Previous count should remain since we are still inside time period
+        runner.setProperty(ControlRate.MAX_RATE, "33 B");
+        runner.run(7, false);
+        runner.assertTransferCount(ControlRate.REL_SUCCESS, 3);
+        runner.assertTransferCount(ControlRate.REL_FAILURE, 0);
+        runner.assertQueueNotEmpty();
+
+        // after 1 second, we should be able to send the up to 3 more flowfiles
+        Thread.sleep(ONE_SEC_PLUS);

Review Comment:
   Agree. I don't care for thread sleeps either. I'm open to suggestions if you have any. Still, the existing unit tests uses sleeps and so my addition is consistent. And, if you noticed, I reduced the total sleep time (prior to adding new unit tests) by reducing the amount of time spent sleeping. Personally, that's an issue I see frequently - sleeping much longer than necessary which has a cumulative affect on the overall build. In this case, a minimum time of 1 sec is required due to the minimum time allowed by the property. And, the sleep is only 1.01 sec.
   
   In other words, I feel it's the best that can be done given the situation and the component being tested.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org