[GitHub] [nifi] thenatog 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>]

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


##########
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:
   I'm wary of adding tests which used thread sleeps. They are frequently unreliable when run on Github Actions infrastructure. The obvious problem is that the it's hard to test a control rate feature without using a delay like this.. Anyway, thought it was worth mentioning, as we're trying to reduce the number of flakey tests we have.



-- 
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