[GitHub] [pulsar] grssam commented on pull request #18812: [fix][broker] Revert #14829 Filter the virtual NIC with relative path

[GitBox <gi...@apache.org>]

grssam commented on PR #18812:
URL: https://github.com/apache/pulsar/pull/18812#issuecomment-1342108446

   @mattisonchao Hello. As I understand the core issue -- 
   
   > the property `loadBalancerOverrideBrokerNicSpeedGbps` was introduced because certain VM (and also true for k8s world) the max nic speed reported in the `/sys/class/net/%s/speed` was incorrect, so the property helped define the max speed limit. Then it was later found out that using all of the nics present inside the `/sys/class/net/%s` do not correctly represent the actual rx and tx of the broker and thus the `/virtual/` ones were removed.
   
   These assumptions are actually correct most of the times. For instance, with the change introduced in this PR, now both of the `eth0` and `lo` would be counted towards current rx and tx of the broker host.. which is also incorrect, as the `lo` represents internal local rx/tx between processes and 127.0.0.1  and thus, only `eth0`'s rx and tx should be counted towards network io.
   
   My suggestion to fix this would be following:
   
   * Keep relying on `loadBalancerOverrideBrokerNicSpeedGbps` config for max rx/tx of the broker.
   * Keep filtering out `/virtual/` nics.
   * Introduce another config `loadBalancerOverrideBrokerWhitelistedNics` as a comma separated list of nics to be considered for calculating actual rx/tx.
   
   that way, in our example : 
   ```
   pulsar@pulsar-broker-6:~$ ls -larth /sys/class/net/
   total 0
   drwxr-xr-x 29 root root 0 Oct 31 18:08 ..
   drwxr-xr-x  2 root root 0 Oct 31 18:08 .
   lrwxrwxrwx  1 root root 0 Oct 31 18:08 lo -> ../../devices/virtual/net/lo
   lrwxrwxrwx  1 root root 0 Oct 31 18:08 eth0 -> ../../devices/virtual/net/eth0
   ```
   
   we will set the config `loadBalancerOverrideBrokerWhitelistedNics` to `eth0` (even this name is not fixed across debian versions for us) and the config `loadBalancerOverrideBrokerNicSpeedGbps` to the correct software (k8s) imposed network limits.
   
   Please let me know if this sounds good?


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org