[bgpcfg]: Batch bgp updates #6006
Conversation
vtysh -f command is slow. It is sometimes takes about 3 seconds. When we need to run many vtysh -f commands that slows down the system. Batch vtysh -f updates.
|
@qiluo-msft , can you review this? @pavel-shirshov , do you have the performance number before and after? |
|
@lguohan I added performance change to the description |
| return | ||
| else: | ||
| log_warn("Can't read daemon status from FRR: %s" % str(err)) | ||
| time.sleep(0.1) # sleep 100 ms |
There was a problem hiding this comment.
time.sleep(0.1) # sleep 100 ms [](start = 12, length = 31)
What happens if no sleep?
There was a problem hiding this comment.
Higher cpu usage. I think it is better to wait for 0.1 than to burn cpu by requests.
| @@ -90,14 +90,13 @@ def check_routemap_in_file(filename, route_map_name): | |||
| found_entry = False | |||
| found_seq_no = None | |||
| if route_map_re.match(line): | |||
| found_seq_no = None | |||
There was a problem hiding this comment.
this should go to the other pr.
| if self.changes.strip() == "": | ||
| return True | ||
| rc_write = self.frr.write(self.changes) | ||
| rc_restart = self.frr.restart_peer_groups(self.peer_groups_to_restart) |
There was a problem hiding this comment.
i am not sure if it is always correct to restart peer group after we make any change. for example, if we shutdown a bgp session, which peer group to restart?
There was a problem hiding this comment.
self.peer_groups_to_restart would be empty in 'shutdown a bgp session" situation and restart_peer_groups will do nothing in this case. I update self.peer_groups_to_restart in two cases now: BBR and allow_prefix.
| ret_code, out, err = run_command(command) | ||
| if ret_code != 0: | ||
| err_tuple = tmp_filename, ret_code, out, err | ||
| log_err("ConfigMgr::commit(): can't push configuration from file='%s', rc='%d', stdout='%s', stderr='%s'" % err_tuple) |
There was a problem hiding this comment.
do we want to keep this tmp file, or just put the tmpfile content into syslog?
There was a problem hiding this comment.
The tmp file content it is full config on the start. Kind of a lot. Do we want to see it in the syslog?
|
if we are doing this config bgp shutdown all, which managers are we triggering? I am not able to identify. |
|
@lguohan |
This reverts commit 85e6ce2.
|
retest vs please |
* [bgpcfgd]: Batch bgp updates. vtysh -f command is slow. It is sometimes takes about 3 seconds. When we need to run many vtysh -f commands that slows down the system. Batch vtysh -f updates. * Use correct file to import run_command
The issue was a typo introduced in sonic-net#6006. In that change, the BGP allow list configuration manager was updated to use a method of common ConfigMgr for restarting peer groups. However, the method name 'restart_peers' was used instead of the correct 'restart_peer_groups'. This change updated the managers_allow_list.py to use correct method 'restart_peer_groups' for restarting peer groups. Signed-off-by: Xin Wang <xiwang5@microsoft.com>
The issue was a typo introduced in #6006. In that change, the BGP allow list configuration manager was updated to use a method of common ConfigMgr for restarting peer groups. However, the method name 'restart_peers' was used instead of the correct 'restart_peer_groups'. This change updated the managers_allow_list.py to use correct method 'restart_peer_groups' for restarting peer groups. Signed-off-by: Xin Wang <xiwang5@microsoft.com>
The issue was a typo introduced in #6006. In that change, the BGP allow list configuration manager was updated to use a method of common ConfigMgr for restarting peer groups. However, the method name 'restart_peers' was used instead of the correct 'restart_peer_groups'. This change updated the managers_allow_list.py to use correct method 'restart_peer_groups' for restarting peer groups. Signed-off-by: Xin Wang <xiwang5@microsoft.com>
* [bgpcfgd]: Batch bgp updates. vtysh -f command is slow. It is sometimes takes about 3 seconds. When we need to run many vtysh -f commands that slows down the system. Batch vtysh -f updates. * Use correct file to import run_command
…ic-net#6088) The issue was a typo introduced in sonic-net#6006. In that change, the BGP allow list configuration manager was updated to use a method of common ConfigMgr for restarting peer groups. However, the method name 'restart_peers' was used instead of the correct 'restart_peer_groups'. This change updated the managers_allow_list.py to use correct method 'restart_peer_groups' for restarting peer groups. Signed-off-by: Xin Wang <xiwang5@microsoft.com>
- Why I did it
vtysh -f command is slow. It sometimes takes about 3 seconds.
When we need to run many vtysh -f commands that slows down the system.
With this fix bgcfgd configuration time reduced from 10.58 sec to 2.80 sec on my dut.
- How I did it
- How to verify it
Build an image and run on your dut. All tests must be working
- Which release branch to backport (provide reason below if selected)
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)