I agree with Sebastien, we do not need to tag any requests, we can just "ask" if the RPC NID is in range of a nodemap via the existing functions.
I don't read Sebastien's comment as implying that? Since the RPC handling has already looked up the nodemap I don't think it should be doing another nodemap NID lookup when NRS is processing the request again later? I think the NID->nodemap lookup is the most expensive part of having a nodemap, so it should only be done once if possible.
The default rate to apply to a "nodemap" class can be tricky. Unlike the "nid" policy, the rate will apply to a range of nodes instead of one node.
That is fine, and that rate is up to the administrator to define? I don't think we should be messing with the rates internally:
- this is confusing for the admin, if they set a rate of "5000" and then it is silently converted to "500000" if there are 1000 nodes in the nodemap
- the number of nodes in the nodemap will change over time, which would mean the TBF rate would change as clients are added and removed
I don't think this can or should be emulated with TBF NID rules, both because the NIDs in a nodemap may change over time (especially with Sebastien's dynamic nodemap code) as well as adding complexity to manage/specify complex TBF rules for many NIDs in the nodemap. Specifying the rule with the nodemap name is very clear and directly ties the rates to all nodes that the "tenant" is using in the nodemap, even if the nodes change over time.
I will rebase it later.
For your question,
To make NRS TBF so smart to detect the node map changing (adding and removing), maybe we should both name and id of node map into the key of a TBF class. Each matching will compare both name and id of the node map? In the current version and previous version, we only compare one of them (either name, or id), not both.
And the adding/removing node map will change the id, I think. Thus NRS TBF can detect the change during the matching.
However, this means more memory used for each TBF class.