You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/12/12 11:54:33 UTC
[GitHub] [nifi-minifi-cpp] fgerlits opened a new pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
fgerlits opened a new pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958
https://issues.apache.org/jira/browse/MINIFICPP-1432
* Add a mockable Clock field to `NetworkPrioritizerService`
* Fix and clean up the tests
* Fix some bugs in `NetworkPrioritizerService`:
- `max_payload_` was not set in `onEnable()`
- only payloads strictly smaller than `max_payload_` were allowed
- use `steady_clock` instead of `system_clock`
---
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:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] 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.
- [x] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [x] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### 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.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542421731
##########
File path: libminifi/include/utils/TimeUtil.h
##########
@@ -53,6 +53,29 @@ inline uint64_t getTimeNano() {
return std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
}
+/**
+ * Mockable clock classes
+ */
+class Clock {
+ public:
+ virtual ~Clock() = default;
+ virtual std::chrono::milliseconds timeSinceEpoch() const = 0;
Review comment:
This could be type erased with `std::function<std::chrono::milliseconds()>` instead of a class hierarchy for simplicity.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542465018
##########
File path: libminifi/include/controllers/NetworkPrioritizerService.h
##########
@@ -121,6 +126,7 @@ class NetworkPrioritizerService : public core::controller::ControllerService, pu
bool verify_interfaces_;
private:
+ std::shared_ptr<utils::timeutils::Clock> clock_;
Review comment:
info(not ment to argue either for or against) : if we want to reference a member of a `shared_ptr` pointee, and not accidentally mess up, we can share the lifetime of the pointee while pointing to the member, so the "container" is kept alive even if we are only using its member
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542420260
##########
File path: libminifi/include/controllers/NetworkPrioritizerService.h
##########
@@ -121,6 +126,7 @@ class NetworkPrioritizerService : public core::controller::ControllerService, pu
bool verify_interfaces_;
private:
+ std::shared_ptr<utils::timeutils::Clock> clock_;
Review comment:
in the tests the clock is being advanced externally
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542434970
##########
File path: libminifi/include/controllers/NetworkPrioritizerService.h
##########
@@ -121,6 +126,7 @@ class NetworkPrioritizerService : public core::controller::ControllerService, pu
bool verify_interfaces_;
private:
+ std::shared_ptr<utils::timeutils::Clock> clock_;
Review comment:
The service and the unit test. If only the service owned it, the unit test would need to have a naked pointer (or reference) to the clock, and rely on the order of the operations to make sure it remains valid -- I have considered this, but did not like it. Since the `clock_` field is only created once, and never copied, there isn't much of an overhead.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542401133
##########
File path: libminifi/test/unit/NetworkPrioritizerServiceTests.cpp
##########
@@ -41,129 +47,105 @@ TEST_CASE("TestPrioritizerOneInterface", "[test1]") {
}
TEST_CASE("TestPrioritizerOneInterfaceMaxPayload", "[test2]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ auto controller = createNetworkPrioritizerService("TestService");
controller->initialize();
controller->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0,eth1");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 B");
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxPayload, "1 B");
+ controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 kB");
+ controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxPayload, "10 B");
controller->onEnable();
- // can't because we've triggered the max payload
- REQUIRE("" == controller->getInterface(5).getInterface());
+
+ REQUIRE("eth0" == controller->getInterface(5).getInterface());
+ REQUIRE("" == controller->getInterface(20).getInterface()); // larger than max payload
+ REQUIRE("eth0" == controller->getInterface(5).getInterface());
}
TEST_CASE("TestPrioritizerOneInterfaceMaxThroughput", "[test3]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ auto clock = std::make_shared<utils::ManualClock>();
+ auto controller = createNetworkPrioritizerService("TestService", clock);
controller->initialize();
controller->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0,eth1");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
controller->onEnable();
- // can't because we've triggered the max payload
REQUIRE("eth0" == controller->getInterface(5).getInterface());
REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("" == controller->getInterface(5).getInterface());
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
+ REQUIRE("" == controller->getInterface(5).getInterface()); // max throughput reached
+ clock->advance(std::chrono::milliseconds{10}); // wait for more tokens to be generated
+ REQUIRE("eth0" == controller->getInterface(5).getInterface()); // now we can send again
}
TEST_CASE("TestPriorotizerMultipleInterfaces", "[test4]") {
- LogTestController::getInstance().setTrace<minifi::controllers::NetworkPrioritizerService>();
-
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- auto controller2 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService2");
- auto controller3 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService3");
+ auto clock = std::make_shared<utils::ManualClock>();
+ auto parent_controller = createNetworkPrioritizerService("TestService", clock);
std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
- controller->initialize();
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ parent_controller->initialize();
+ parent_controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->initialize();
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller3->onEnable();
-
- controller2->initialize();
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller2->onEnable();
- std::vector<std::shared_ptr<core::controller::ControllerService> > services;
- services.push_back(controller2);
- services.push_back(controller3);
- controller->setLinkedControllerServices(services);
- controller->onEnable();
- // can't because we've triggered the max payload
- REQUIRE("eth1" == controller->getInterface(5).getInterface());
- REQUIRE("eth1" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
-}
+ auto controller0 = createNetworkPrioritizerService("TestService_eth0", clock);
+ controller0->initialize();
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
+ controller0->onEnable();
-TEST_CASE("TestPriorotizerMultipleInterfacesNeverSwitch", "[test5]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- auto controller2 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService2");
- auto controller3 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService3");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
- controller->initialize();
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ auto controller1 = createNetworkPrioritizerService("TestService_eth1", clock);
+ controller1->initialize();
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
+ controller1->onEnable();
- controller3->initialize();
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 kB");
- controller3->onEnable();
-
- controller2->initialize();
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller2->onEnable();
std::vector<std::shared_ptr<core::controller::ControllerService> > services;
- services.push_back(controller3);
- services.push_back(controller2);
- controller->setLinkedControllerServices(services);
- controller->onEnable();
- // can't because we've triggered the max payload
- for (int i = 0; i < 50; i++) {
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ services.push_back(controller0);
+ services.push_back(controller1);
+ parent_controller->setLinkedControllerServices(services);
+ parent_controller->onEnable();
+
+ SECTION("Switch to second interface when the first is saturated") {
+ REQUIRE("eth0" == parent_controller->getInterface(5).getInterface());
+ REQUIRE("eth0" == parent_controller->getInterface(5).getInterface());
+ // triggered the max throughput on eth0, switching to eth1
+ REQUIRE("eth1" == parent_controller->getInterface(5).getInterface());
+ REQUIRE("eth1" == parent_controller->getInterface(5).getInterface());
}
-}
+ SECTION("Can keep sending on eth0 if we wait between packets") {
+ for (int i = 0; i < 100; i++) {
+ REQUIRE("eth0" == parent_controller->getInterface(10).getInterface());
Review comment:
> `getInterface(size)` returns a network interface that we should use to send the next size number of bytes?
Yes.
> what controls how long we should wait (5ms in this case) until the same interface is available?
It is a (version of the) [token bucket algorithm](https://en.wikipedia.org/wiki/Token_bucket): conceptually, the bucket starts out with `b` tokens and `t` tokens are added to the bucket each millisecond, and each send operation removes `s` times `size` tokens from the bucket, if there are enough tokens available, otherwise it fails (and does not remove any tokens).
[In practice, the bucket is filled in `getInterface()` according to the number of elapsed milliseconds since the last call to `getInterface()`, and the way the `b`, `t` and `s` parameters are set is a bit strange (`t` is always 2, `b` and `s` depend on `MaxThroughput`, but differently depending on whenter `MaxThroughput` is less or more than 1 kB).]
Since I don't think anybody actually uses this class at the moment, I did not clean it up; I only fixed the unit test so that it is no longer dependent on timing, and fixed a few obvious bugs in the service code.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542401133
##########
File path: libminifi/test/unit/NetworkPrioritizerServiceTests.cpp
##########
@@ -41,129 +47,105 @@ TEST_CASE("TestPrioritizerOneInterface", "[test1]") {
}
TEST_CASE("TestPrioritizerOneInterfaceMaxPayload", "[test2]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ auto controller = createNetworkPrioritizerService("TestService");
controller->initialize();
controller->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0,eth1");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 B");
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxPayload, "1 B");
+ controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 kB");
+ controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxPayload, "10 B");
controller->onEnable();
- // can't because we've triggered the max payload
- REQUIRE("" == controller->getInterface(5).getInterface());
+
+ REQUIRE("eth0" == controller->getInterface(5).getInterface());
+ REQUIRE("" == controller->getInterface(20).getInterface()); // larger than max payload
+ REQUIRE("eth0" == controller->getInterface(5).getInterface());
}
TEST_CASE("TestPrioritizerOneInterfaceMaxThroughput", "[test3]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ auto clock = std::make_shared<utils::ManualClock>();
+ auto controller = createNetworkPrioritizerService("TestService", clock);
controller->initialize();
controller->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0,eth1");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
controller->onEnable();
- // can't because we've triggered the max payload
REQUIRE("eth0" == controller->getInterface(5).getInterface());
REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("" == controller->getInterface(5).getInterface());
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
+ REQUIRE("" == controller->getInterface(5).getInterface()); // max throughput reached
+ clock->advance(std::chrono::milliseconds{10}); // wait for more tokens to be generated
+ REQUIRE("eth0" == controller->getInterface(5).getInterface()); // now we can send again
}
TEST_CASE("TestPriorotizerMultipleInterfaces", "[test4]") {
- LogTestController::getInstance().setTrace<minifi::controllers::NetworkPrioritizerService>();
-
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- auto controller2 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService2");
- auto controller3 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService3");
+ auto clock = std::make_shared<utils::ManualClock>();
+ auto parent_controller = createNetworkPrioritizerService("TestService", clock);
std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
- controller->initialize();
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ parent_controller->initialize();
+ parent_controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->initialize();
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller3->onEnable();
-
- controller2->initialize();
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller2->onEnable();
- std::vector<std::shared_ptr<core::controller::ControllerService> > services;
- services.push_back(controller2);
- services.push_back(controller3);
- controller->setLinkedControllerServices(services);
- controller->onEnable();
- // can't because we've triggered the max payload
- REQUIRE("eth1" == controller->getInterface(5).getInterface());
- REQUIRE("eth1" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
-}
+ auto controller0 = createNetworkPrioritizerService("TestService_eth0", clock);
+ controller0->initialize();
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
+ controller0->onEnable();
-TEST_CASE("TestPriorotizerMultipleInterfacesNeverSwitch", "[test5]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- auto controller2 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService2");
- auto controller3 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService3");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
- controller->initialize();
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ auto controller1 = createNetworkPrioritizerService("TestService_eth1", clock);
+ controller1->initialize();
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
+ controller1->onEnable();
- controller3->initialize();
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 kB");
- controller3->onEnable();
-
- controller2->initialize();
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller2->onEnable();
std::vector<std::shared_ptr<core::controller::ControllerService> > services;
- services.push_back(controller3);
- services.push_back(controller2);
- controller->setLinkedControllerServices(services);
- controller->onEnable();
- // can't because we've triggered the max payload
- for (int i = 0; i < 50; i++) {
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ services.push_back(controller0);
+ services.push_back(controller1);
+ parent_controller->setLinkedControllerServices(services);
+ parent_controller->onEnable();
+
+ SECTION("Switch to second interface when the first is saturated") {
+ REQUIRE("eth0" == parent_controller->getInterface(5).getInterface());
+ REQUIRE("eth0" == parent_controller->getInterface(5).getInterface());
+ // triggered the max throughput on eth0, switching to eth1
+ REQUIRE("eth1" == parent_controller->getInterface(5).getInterface());
+ REQUIRE("eth1" == parent_controller->getInterface(5).getInterface());
}
-}
+ SECTION("Can keep sending on eth0 if we wait between packets") {
+ for (int i = 0; i < 100; i++) {
+ REQUIRE("eth0" == parent_controller->getInterface(10).getInterface());
Review comment:
> `getInterface(size)` returns a network interface that we should use to send the next size number of bytes?
Yes.
> what controls how long we should wait (5ms in this case) until the same interface is available?
It is a (version of the) [token bucket algorithm](https://en.wikipedia.org/wiki/Token_bucket): conceptually, the bucket starts out with `b` tokens and `t` tokens are added to the bucket each millisecond, and each send operation removes `s` times `size` tokens from the bucket, if there are enough tokens available, otherwise it fails (and does not remove any tokens).
[In practice, the bucket is filled in `getInterface()` according to the number of elapsed milliseconds since the last call to `getInterface()`, and the way the `b`, `t` and `s` parameters are set is a bit strange (`t` is always 2, `b` and `s` depend on `MaxThroughput`, but differently depending on whether `MaxThroughput` is less or more than 1 kB).]
Since I don't think anybody actually uses this class at the moment, I did not clean it up; I only fixed the unit test so that it is no longer dependent on timing, and fixed a few obvious bugs in the service code.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542436009
##########
File path: libminifi/include/utils/TimeUtil.h
##########
@@ -53,6 +53,29 @@ inline uint64_t getTimeNano() {
return std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
}
+/**
+ * Mockable clock classes
+ */
+class Clock {
+ public:
+ virtual ~Clock() = default;
+ virtual std::chrono::milliseconds timeSinceEpoch() const = 0;
Review comment:
Yes, that would be another way of doing it, but I'm not sure it would be simpler. Do you think it would be so much simpler that it's worth rewriting this?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542420260
##########
File path: libminifi/include/controllers/NetworkPrioritizerService.h
##########
@@ -121,6 +126,7 @@ class NetworkPrioritizerService : public core::controller::ControllerService, pu
bool verify_interfaces_;
private:
+ std::shared_ptr<utils::timeutils::Clock> clock_;
Review comment:
in the tests the clock is being advanced externally (if this is the only reason, we should create a TestAccessor, and switch to `unique_ptr`)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542405642
##########
File path: libminifi/test/unit/NetworkPrioritizerServiceTests.cpp
##########
@@ -41,129 +47,105 @@ TEST_CASE("TestPrioritizerOneInterface", "[test1]") {
}
TEST_CASE("TestPrioritizerOneInterfaceMaxPayload", "[test2]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ auto controller = createNetworkPrioritizerService("TestService");
controller->initialize();
controller->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0,eth1");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 B");
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxPayload, "1 B");
+ controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 kB");
+ controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxPayload, "10 B");
controller->onEnable();
- // can't because we've triggered the max payload
- REQUIRE("" == controller->getInterface(5).getInterface());
+
+ REQUIRE("eth0" == controller->getInterface(5).getInterface());
+ REQUIRE("" == controller->getInterface(20).getInterface()); // larger than max payload
+ REQUIRE("eth0" == controller->getInterface(5).getInterface());
}
TEST_CASE("TestPrioritizerOneInterfaceMaxThroughput", "[test3]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ auto clock = std::make_shared<utils::ManualClock>();
+ auto controller = createNetworkPrioritizerService("TestService", clock);
controller->initialize();
controller->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0,eth1");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
controller->onEnable();
- // can't because we've triggered the max payload
REQUIRE("eth0" == controller->getInterface(5).getInterface());
REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("" == controller->getInterface(5).getInterface());
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
+ REQUIRE("" == controller->getInterface(5).getInterface()); // max throughput reached
+ clock->advance(std::chrono::milliseconds{10}); // wait for more tokens to be generated
+ REQUIRE("eth0" == controller->getInterface(5).getInterface()); // now we can send again
}
TEST_CASE("TestPriorotizerMultipleInterfaces", "[test4]") {
- LogTestController::getInstance().setTrace<minifi::controllers::NetworkPrioritizerService>();
-
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- auto controller2 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService2");
- auto controller3 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService3");
+ auto clock = std::make_shared<utils::ManualClock>();
+ auto parent_controller = createNetworkPrioritizerService("TestService", clock);
std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
- controller->initialize();
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ parent_controller->initialize();
+ parent_controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->initialize();
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller3->onEnable();
-
- controller2->initialize();
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller2->onEnable();
- std::vector<std::shared_ptr<core::controller::ControllerService> > services;
- services.push_back(controller2);
- services.push_back(controller3);
- controller->setLinkedControllerServices(services);
- controller->onEnable();
- // can't because we've triggered the max payload
- REQUIRE("eth1" == controller->getInterface(5).getInterface());
- REQUIRE("eth1" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
-}
+ auto controller0 = createNetworkPrioritizerService("TestService_eth0", clock);
+ controller0->initialize();
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
+ controller0->onEnable();
-TEST_CASE("TestPriorotizerMultipleInterfacesNeverSwitch", "[test5]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- auto controller2 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService2");
- auto controller3 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService3");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
- controller->initialize();
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ auto controller1 = createNetworkPrioritizerService("TestService_eth1", clock);
+ controller1->initialize();
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
+ controller1->onEnable();
- controller3->initialize();
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 kB");
- controller3->onEnable();
-
- controller2->initialize();
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller2->onEnable();
std::vector<std::shared_ptr<core::controller::ControllerService> > services;
- services.push_back(controller3);
- services.push_back(controller2);
- controller->setLinkedControllerServices(services);
- controller->onEnable();
- // can't because we've triggered the max payload
- for (int i = 0; i < 50; i++) {
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ services.push_back(controller0);
+ services.push_back(controller1);
+ parent_controller->setLinkedControllerServices(services);
+ parent_controller->onEnable();
+
+ SECTION("Switch to second interface when the first is saturated") {
+ REQUIRE("eth0" == parent_controller->getInterface(5).getInterface());
+ REQUIRE("eth0" == parent_controller->getInterface(5).getInterface());
+ // triggered the max throughput on eth0, switching to eth1
+ REQUIRE("eth1" == parent_controller->getInterface(5).getInterface());
+ REQUIRE("eth1" == parent_controller->getInterface(5).getInterface());
}
-}
+ SECTION("Can keep sending on eth0 if we wait between packets") {
+ for (int i = 0; i < 100; i++) {
+ REQUIRE("eth0" == parent_controller->getInterface(10).getInterface());
Review comment:
thank you for clarifying!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542418714
##########
File path: libminifi/include/controllers/NetworkPrioritizerService.h
##########
@@ -121,6 +126,7 @@ class NetworkPrioritizerService : public core::controller::ControllerService, pu
bool verify_interfaces_;
private:
+ std::shared_ptr<utils::timeutils::Clock> clock_;
Review comment:
Why shared_ptr, what are the other owners?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm closed pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests, version 1
Posted by GitBox <gi...@apache.org>.
szaszm closed pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542423177
##########
File path: libminifi/include/utils/TimeUtil.h
##########
@@ -53,6 +53,29 @@ inline uint64_t getTimeNano() {
return std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
}
+/**
+ * Mockable clock classes
+ */
+class Clock {
+ public:
+ virtual ~Clock() = default;
+ virtual std::chrono::milliseconds timeSinceEpoch() const = 0;
Review comment:
also: I wouldn't introduce a `SystemClock` class until it is actually needed
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542433430
##########
File path: libminifi/include/utils/TimeUtil.h
##########
@@ -53,6 +53,29 @@ inline uint64_t getTimeNano() {
return std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
}
+/**
+ * Mockable clock classes
+ */
+class Clock {
+ public:
+ virtual ~Clock() = default;
+ virtual std::chrono::milliseconds timeSinceEpoch() const = 0;
Review comment:
regarding the `std::function<...>`, I would keep the `Clock` class, as IMO it better describes its usage
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542450733
##########
File path: libminifi/include/utils/TimeUtil.h
##########
@@ -53,6 +53,29 @@ inline uint64_t getTimeNano() {
return std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
}
+/**
+ * Mockable clock classes
+ */
+class Clock {
+ public:
+ virtual ~Clock() = default;
+ virtual std::chrono::milliseconds timeSinceEpoch() const = 0;
Review comment:
Currently I see the `Clock` class hierarchy as just related functions in disguise. This change would allow us to get rid of the classes and replace them with free functions (e.g. `get_time`, `get_steady_clock_time` and a lambda with a captured local for the manual case), which is closer to the truth IMO.
I think functions are simpler than classes (one operation vs multiple operations + data), so for my code, I would opt for that approach, but I'm totally fine with you not sharing my viewpoint and keeping the code as is.
For the readability argument, I'm arguing that the usage we describe is retrieving the time, and that can be described by naming the functions and the type-erased member something along the lines of `get_time_`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #958: MINIFICPP-1432 Remove the timing-sensitivity of NetworkPrioritizerServiceTests
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #958:
URL: https://github.com/apache/nifi-minifi-cpp/pull/958#discussion_r542346927
##########
File path: libminifi/test/unit/NetworkPrioritizerServiceTests.cpp
##########
@@ -41,129 +47,105 @@ TEST_CASE("TestPrioritizerOneInterface", "[test1]") {
}
TEST_CASE("TestPrioritizerOneInterfaceMaxPayload", "[test2]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ auto controller = createNetworkPrioritizerService("TestService");
controller->initialize();
controller->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0,eth1");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 B");
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxPayload, "1 B");
+ controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 kB");
+ controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxPayload, "10 B");
controller->onEnable();
- // can't because we've triggered the max payload
- REQUIRE("" == controller->getInterface(5).getInterface());
+
+ REQUIRE("eth0" == controller->getInterface(5).getInterface());
+ REQUIRE("" == controller->getInterface(20).getInterface()); // larger than max payload
+ REQUIRE("eth0" == controller->getInterface(5).getInterface());
}
TEST_CASE("TestPrioritizerOneInterfaceMaxThroughput", "[test3]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ auto clock = std::make_shared<utils::ManualClock>();
+ auto controller = createNetworkPrioritizerService("TestService", clock);
controller->initialize();
controller->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0,eth1");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
controller->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
controller->onEnable();
- // can't because we've triggered the max payload
REQUIRE("eth0" == controller->getInterface(5).getInterface());
REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("" == controller->getInterface(5).getInterface());
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
+ REQUIRE("" == controller->getInterface(5).getInterface()); // max throughput reached
+ clock->advance(std::chrono::milliseconds{10}); // wait for more tokens to be generated
+ REQUIRE("eth0" == controller->getInterface(5).getInterface()); // now we can send again
}
TEST_CASE("TestPriorotizerMultipleInterfaces", "[test4]") {
- LogTestController::getInstance().setTrace<minifi::controllers::NetworkPrioritizerService>();
-
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- auto controller2 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService2");
- auto controller3 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService3");
+ auto clock = std::make_shared<utils::ManualClock>();
+ auto parent_controller = createNetworkPrioritizerService("TestService", clock);
std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
- controller->initialize();
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ parent_controller->initialize();
+ parent_controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->initialize();
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller3->onEnable();
-
- controller2->initialize();
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller2->onEnable();
- std::vector<std::shared_ptr<core::controller::ControllerService> > services;
- services.push_back(controller2);
- services.push_back(controller3);
- controller->setLinkedControllerServices(services);
- controller->onEnable();
- // can't because we've triggered the max payload
- REQUIRE("eth1" == controller->getInterface(5).getInterface());
- REQUIRE("eth1" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
-}
+ auto controller0 = createNetworkPrioritizerService("TestService_eth0", clock);
+ controller0->initialize();
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ controller0->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
+ controller0->onEnable();
-TEST_CASE("TestPriorotizerMultipleInterfacesNeverSwitch", "[test5]") {
- auto controller = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService");
- auto controller2 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService2");
- auto controller3 = std::make_shared<minifi::controllers::NetworkPrioritizerService>("TestService3");
- std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
- controller->initialize();
- controller->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ auto controller1 = createNetworkPrioritizerService("TestService_eth1", clock);
+ controller1->initialize();
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
+ controller1->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
+ controller1->onEnable();
- controller3->initialize();
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth0");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller3->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "1 kB");
- controller3->onEnable();
-
- controller2->initialize();
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::NetworkControllers, "eth1");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::VerifyInterfaces, "false");
- controller2->setProperty(minifi::controllers::NetworkPrioritizerService::MaxThroughput, "10 B");
- controller2->onEnable();
std::vector<std::shared_ptr<core::controller::ControllerService> > services;
- services.push_back(controller3);
- services.push_back(controller2);
- controller->setLinkedControllerServices(services);
- controller->onEnable();
- // can't because we've triggered the max payload
- for (int i = 0; i < 50; i++) {
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- REQUIRE("eth0" == controller->getInterface(5).getInterface());
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ services.push_back(controller0);
+ services.push_back(controller1);
+ parent_controller->setLinkedControllerServices(services);
+ parent_controller->onEnable();
+
+ SECTION("Switch to second interface when the first is saturated") {
+ REQUIRE("eth0" == parent_controller->getInterface(5).getInterface());
+ REQUIRE("eth0" == parent_controller->getInterface(5).getInterface());
+ // triggered the max throughput on eth0, switching to eth1
+ REQUIRE("eth1" == parent_controller->getInterface(5).getInterface());
+ REQUIRE("eth1" == parent_controller->getInterface(5).getInterface());
}
-}
+ SECTION("Can keep sending on eth0 if we wait between packets") {
+ for (int i = 0; i < 100; i++) {
+ REQUIRE("eth0" == parent_controller->getInterface(10).getInterface());
Review comment:
this is a little unclear to me, so `getInterface(size)` returns a network interface that we should use to send the next `size` number of bytes? what controls how long we should wait (5ms in this case) until the same interface is available?
----------------------------------------------------------------
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:
users@infra.apache.org