Task #2107
closedRefactor management system
100%
Description
Refactor management system so that:
- Each Interest is dispatched only once.
- Authentication is centralized and consistent.
- Reply encryption can be turned on when necessary.
- Basic mechanisms can be reused inside and outside NFD.
Files
Updated by Junxiao Shi about 10 years ago
Eliminate double-dispatch describes the design intention of NFD Management system, which can achieve the first three goals.
Since basic mechanisms are increasingly reused in other projects, and some are available from the library, it's also desirable to make the underlying Face compatible with both nfd::Face
and ndn::Face
, so that code can be reused.
I'll present the basic idea sometime later.
Updated by Junxiao Shi about 10 years ago
@Steve, don't rush on implementation.
There are design issues to solve: goal 4 is incompatible with the design in Eliminate double-dispatch.
Updated by Junxiao Shi about 10 years ago
- Blocks Task #2144: Delete NotificationStream added
Updated by Junxiao Shi about 10 years ago
- Target version set to v0.3
At 20141112 conference call, Steve agrees to implement this in v0.3, if the design issue can be resolved.
Updated by Junxiao Shi about 10 years ago
- Related to Feature #2182: InMemoryStorage for management added
Updated by Junxiao Shi about 10 years ago
- Blocks Feature #2184: faces/enable-local-control: CachingPolicy added
Updated by Junxiao Shi about 10 years ago
- Blocked by Task #2200: Design dispatch mechanism for Management added
Updated by Junxiao Shi almost 10 years ago
- Estimated time changed from 12.00 h to 30.00 h
This is a large and complex Task.
To ensure success, I recommend the assignee to write all the headers first and submit them for review, and work on the implementation after header review is passed.
I'm increasing the time estimation because I realize that there's also major problems in the unit testing of NFD Management.
There are large amounts of code duplication in the test suite. The test suite needs to be consolidated so that they are easier to maintain.
Updated by Anonymous almost 10 years ago
- Status changed from New to In Progress
Updated by Anonymous almost 10 years ago
- Status changed from In Progress to Feedback
- % Done changed from 0 to 10
Header changes submitted.
Updated by Anonymous almost 10 years ago
- Status changed from Feedback to In Progress
Updated by Anonymous almost 10 years ago
I have a lot of other things going on right now (especially with the run up to the retreat). I do not plan to resume work on this task until after the retreat.
I would prefer to refactor the unit tests as a separate task with a later version target (a v0.3 minor release?) so that I can get through the other v0.3 management tasks. I haven't had a chance to look over the tasks and no refactoring plan has been proposed.
Updated by Junxiao Shi almost 10 years ago
I would prefer to refactor the unit tests as a separate task with a later version target (a v0.3 minor release?) so that I can get through the other v0.3 management tasks. I haven't had a chance to look over the tasks and no refactoring plan has been proposed.
Refactoring management system involves changes to class interfaces, which inevitably requires changes to unit tests. Therefore, unit testing refactoring has to be in this Task as well.
I will take more care on test cases in code reviews.
Test cases must be well designed, and code duplication should be avoided.
Updated by Anonymous almost 10 years ago
What's the recommended way to test sending Interests and receiving Data in ndn-cxx? Specifically, I want to test ManagementDispatcher::addControlCommand
and making sure the handler gets called.
I'm looking at DummyClientFace
, but I don't think it does what I want.
Updated by Alex Afanasyev almost 10 years ago
DummyClientFace will do everything what you need. You can emulate reception of external interests/data using DummyClientFace::receive methods.
For simple example, you can check https://github.com/named-data/ndn-cxx/blob/master/tests/unit-tests/test-face.cpp.
For complex example, check https://github.com/named-data/NFD/blob/master/tests/daemon/fw/topology-tester.hpp
Updated by Anonymous almost 10 years ago
I've looked over the examples, but I'm still stuck. For example, the dispatcher needs to call Face.setInterestFilter
to register prefixes. However, I think this needs the io service to run for a bit, otherwise the filter table will be empty when I call receive
in the test case. I know NFD has LimitedIo
for this sort of thing, but is there something equivalent in cxx? (...or am I overcomplicating things?)
Updated by Junxiao Shi almost 10 years ago
I think this needs the io service to run for a bit, otherwise the filter table will be empty when I call
receive
in the test case. I know NFD hasLimitedIo
for this sort of thing, but is there something equivalent in cxx?
io.poll()
Updated by Yanbiao Li over 9 years ago
- Assignee changed from Anonymous to Yanbiao Li
Updated by Alex Afanasyev over 9 years ago
I saw http://gerrit.named-data.net/#/c/2048. Why is it part of ndn-cxx library? Are we expecting this management dispatcher to be used anywhere else except NFD?
Updated by Yanbiao Li over 9 years ago
I guess this is one of the design objectives.
"Basic mechanisms can be reused inside and outside NFD."
Updated by Junxiao Shi over 9 years ago
Are we expecting this management dispatcher to be used anywhere else except NFD?
The intention is to allow using ManagementDispatcher
in any application that adopts a management protocol similar to NFD Management.
As pointed out in #2200 note-30 item 1, this goal isn't fulfilled in current design.
I'm thinking about a solution.
Updated by Junxiao Shi over 9 years ago
- Blocks Task #2857: Refactor RibManager to use ManagementDispatcher added
Updated by Junxiao Shi over 9 years ago
- Blocks deleted (Feature #2184: faces/enable-local-control: CachingPolicy)
Updated by Junxiao Shi over 9 years ago
- Blocks Feature #3029: Publish RouterName in ForwarderStatus added
Updated by Junxiao Shi over 9 years ago
- Target version changed from v0.3 to v0.4
At 20150710 conference call, @Yanbiao reveals that Dispatcher is mostly finished, but he has no idea on how to proceed with the rest of refactoring.
I also don't have sufficient understanding on how to port Managers to use the Dispatcher.
It's requested that @Steve should explain the existing management system.
Updated by Anonymous over 9 years ago
@Yanbiao, please feel free to send me an email to setup a Skype/hangout time. I'm pretty available tomorrow. Otherwise, we can try sometime during the week. (Anyone else interested is welcome to attend, too...not sure this is worth taking time from an NFD call.)
Updated by Yanbiao Li over 9 years ago
@Yanbiao, please feel free to send me an email to setup a Skype/hangout time. I'm pretty available tomorrow. Otherwise, we can try sometime during the week. (Anyone else interested is welcome to attend, too...not sure this is worth taking time from an NFD call.)
Thanks Steve, I has sent an email to you.
Updated by Junxiao Shi over 9 years ago
One problem in dispatcher implementation is: how to emit error logs.
Background:
- The dispatcher lives in ndn-cxx library which does not have an logging facility.
- The dispatcher may be used in various applications that have different logging systems: NFD has
NFD_LOG_ERROR
macro, NLSR has log4cxx. - Some functions are executed from
io.run()
so they shouldn't throw exceptions unless a fatal error (+ program termination) is desired.
Option 1: use log4cxx
- benefit: powerful logging facility, used by many projects
- benefit: reusable among all ndn-cxx components
- drawback: extra dependency
Option 2: emit an logError
signal from Dispatcher
- benefit: works without dependency
- drawback: log lines are generated even if no logging facility is connected (note: dispatcher emits
logError
only in error conditions; it's not for debug logs)
Option 3: have a global logError
signal
- benefit: works without dependency
- benefit: reusable among all ndn-cxx components
- drawback: log lines are generated even if no logging facility is connected (no problem with dispatcher, but other components can easily abuse this)
Updated by Junxiao Shi over 9 years ago
20150727 conference call decides:
- Put a
// #define NDN_CXX_MGMT_DISPATCHER_ENABLE_LOGGING
at the top of dispatcher.hpp. Dispatcher
can write error logs intostd::clog
.- All logging commands should be wrapped in
#ifdef NDN_CXX_MGMT_DISPATCHER_ENABLE_LOGGING
#endif // NDN_CXX_MGMT_DISPATCHER_ENABLE_LOGGING
.
Updated by Junxiao Shi over 9 years ago
Reply to #2200 note-76 item 2:
When publishing face status, we should avoid the old data be fetched if there is a newer version. Shall we do this by setting NO CACHY policy or setting a freshnessPeriod?
In this Task, a FreshnessPeriod consistent with older version should be set.
Setting NO_CACHE
would cause StatusDataset with multiple segments to stop working, because there's no internal cache in Dispatcher
, and responding to subsequent segments relies on the ContentStore.
#2182 will add an internal cache, and then NO_CACHE
will be set.
Updated by Junxiao Shi over 9 years ago
From Alex:
I have briefly checked FibManager, FaceManager, and StrategyChoiceManager test cases. I do not see how they are affected by the refactoring of the management. What is mostly tested there is the effects how manager reacts on the incoming commands: this behavior is not changing.
@Junxiao: what exactly you see the problem with fib-manager.t.cpp, face-manager.t.cpp, strategy-choice-manager.t.cpp? They are not ideal, but I cannot say they completely wrong and unreadable. Most test cases have self-explanatory names; actual checks I would agree are not very straightforward, but they should do the job.
I didn't say the current test cases are wrong. They just have too much duplication and are hard to maintain.
If test cases are not broken in this task, I could agree to split test case improvements into a separate task, but that needs to be done sometime.
From note-9:
There are large amounts of code duplication in the test suite. The test suite needs to be consolidated so that they are easier to maintain.
Updated by Junxiao Shi over 9 years ago
- Blocks Task #3081: Transition ForwarderStatus single packet to GeneralStatus dataset added
Updated by Junxiao Shi over 9 years ago
At 20150821 conference call, Alex disagrees with deferring unit testing improvements, because existing test cases will be broken when changing into the new dispatcher.
There are two possible options:
- Delete all test cases, and re-add them.
- Modify existing test cases BEFORE changing dispatcher, so that they feed Interests into the
InternalFace
instead ofXManager
. When changing into the new dispatcher, test cases should not be broken.
Yanbiao is to explore the second option.
Updated by Junxiao Shi about 9 years ago
- Blocked by deleted (Task #2200: Design dispatch mechanism for Management)
Updated by Junxiao Shi about 9 years ago
- Related to Task #2200: Design dispatch mechanism for Management added
Updated by Junxiao Shi about 9 years ago
I notice a small issue with Dispatcher
design: StatusDatasetContext
isn't marked noncopyable
. I'll upload another Change to fix this problem, as well as changing some function-by-value to function-by-const-reference.
Updated by Junxiao Shi about 9 years ago
- File 20150908190620.tgz 20150908190620.tgz added
Updated by Alex Afanasyev about 9 years ago
There is a problem with ForwarderStatusManager and specifically ForwarderStatus dataset.
ForwarderStatus is encoded as
21 (Content) (size: 75)
128 (APP_TAG_1) (size: 20) [[0.3.4-commit-c9b26a3]]
...
152 (APP_TAG_1) (size: 1) [[%00]]
and then appended to the context, resulting in two nested Content blocks in the data packet:
21 (Content) (size: 77)
21 (Content) (size: 75)
128 (APP_TAG_1) (size: 20) [[0.3.4-commit-c9b26a3]]
...
152 (APP_TAG_1) (size: 1) [[%00]]
Updated by Alex Afanasyev about 9 years ago
- File 20150909014411.tgz 20150909014411.tgz added
Integration tests are passing now.
Updated by Alex Afanasyev about 9 years ago
- Status changed from In Progress to Code review
- % Done changed from 10 to 100
Updated by Alex Afanasyev about 9 years ago
- Status changed from Code review to New
- % Done changed from 100 to 80
The code has been merged, but we need to update developer's guide to close this issue.
Updated by Yanbiao Li about 9 years ago
Is this urgent? Since the remote registration also needs to update the Developer's Guide. Can I do their documentation together?
Updated by Junxiao Shi about 9 years ago
- Status changed from New to Feedback
Don't put this back to New if something is already merged and not reverted.
Updated by Junxiao Shi about 9 years ago
I have written the documentation of Dispatcher
in #2200 note-91.
Yanbiao should write documentation for NFD's usage of Dispatcher
as part of this issue; I can't write this part because I'm unfamiliar with Managers.
Updated by Yanbiao Li about 9 years ago
Where to put this documentation? NFD developer guider?
Updated by Junxiao Shi about 9 years ago
Answer to note-47:
Dispatcher
itself is already documented in ndn-cxx Application Developer Guide.
NFD Management and its usage of Dispatcher
should be documented in NFD Developer Guide.
Contact Alex for repository permission.
Updated by Junxiao Shi about 9 years ago
- Status changed from Feedback to Closed
- % Done changed from 80 to 100
NFD Developer Guide has been updated in nfd-docs:commit:08f2271a3ff61acc2737a6449efeddcf9609b9d8.
I also fixed a small mistake in nfd-docs:commit:bed706e5852191e75342e6f30b80a541a6a830b6.