Re: [pbs-devel] [PATCH proxmox-backup 2/2] traffic-control: add debug log when we found a matching rule

[Dominik Csapak <d.csapak@proxmox.com>]

On 2/4/22 11:05, Thomas Lamprecht wrote:
> On 04.02.22 10:12, Dominik Csapak wrote:
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> optional, at least one user in the forum has a problem with traffic
>> control, this could help debug that in the future...
> 
> Above needs to be in the commit message and actually linking to the relevant
> forum thread.
> 
> in general sure, but I dislike the direction of the approach, as its again
> moving in the same direction as e.g., pmxcfs, a single boolean flag for all
> or nothing, which in practice will soon mean that's rather useless as its
> spamming so much stuff that relevant things get drowned even for experienced
> users.
> 
> More fine grained approach it both, the verbosity and the topic axis would
> be much nicer, especially the latter as then a user could only enable
> traffic-control related logs.
> 
> But just mentioning as this is a major pain point in pmxcfs that I get "hurt"
> by frequently..

makes total sense. did you already imagine any way to enable this?
could we simply have some 'sections' (like tc,connections,etc.)
and enable them like this:

PROXMOX_DEBUG=tc=debug,conn=info,foo=none

or should we avoid the environment variable at all, and put it in
the node config?

> 
>>   src/cached_traffic_control.rs | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/cached_traffic_control.rs b/src/cached_traffic_control.rs
>> index 2f077d36..cd13bc1b 100644
>> --- a/src/cached_traffic_control.rs
>> +++ b/src/cached_traffic_control.rs
>> @@ -342,6 +342,7 @@ impl TrafficControlCache {
>>               Some((rule, _)) => {
>>                   match self.limiter_map.get(&rule.config.name) {
>>                       Some((read_limiter, write_limiter)) => {
>> +                        log::debug!("found traffic control rule for {:?} : {}", peer_ip, &rule.config.name);
>>                           (&rule.config.name, read_limiter.clone(), write_limiter.clone())
>>                       }
>>                       None => ("", None, None), // should never happen
>