Skip to content

Improve facts gathering while creating SonicHost instance#21442

Merged
StormLiangMS merged 2 commits intosonic-net:masterfrom
wangxin:improve-sonichost-init
Mar 26, 2026
Merged

Improve facts gathering while creating SonicHost instance#21442
StormLiangMS merged 2 commits intosonic-net:masterfrom
wangxin:improve-sonichost-init

Conversation

@wangxin
Copy link
Copy Markdown
Collaborator

@wangxin wangxin commented Nov 26, 2025

Description of PR

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202205
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505

Approach

What is the motivation for this PR?

The SonicHost init method needs to run multiple commands on the SONiC host to gather some basic facts. Execution of each command has some overhead. To improve the performance, multi threads were introduced to run the commands in parallel. It looks like an over kill. In some scenarios, it will cause multiple threads in multiple threads and could make things even more complicated. What's more, getting versions still need to run commands on SONiC host multiple times.

How did you do it?

To simplify the procedure of gathering facts and ensure performance at the same time, this change introduced a new customized ansible module sonic_basic_facts.py. This module will be executed on SONiC host and use various methods to directly gather facts:

  • call sonic_py_common library
  • read from config_db using DB connector
  • read files The time requred to run the sonic_basic_facts module is similiar as running a single command on the SONiC device. By this method, the code for gathering facts is significantly simplified with even better performance. When there is no cache, run a script may need 10 seconds less.

The init method of MultiAsicSonicHost is also updated to use gathered facts instead of running the config_facts module which is a little bit slow and produces too much log.

How did you verify/test it?

Tested on KVM testbed.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

@wangxin wangxin requested a review from lolyu November 26, 2025 06:28
@wangxin wangxin requested a review from yxieca as a code owner November 26, 2025 06:28
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin wangxin closed this Nov 26, 2025
@wangxin wangxin reopened this Nov 26, 2025
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Collaborator

@lolyu lolyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the running time usingsonic_basic_facts?

@wangxin
Copy link
Copy Markdown
Collaborator Author

wangxin commented Nov 27, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin
Copy link
Copy Markdown
Collaborator Author

wangxin commented Nov 27, 2025

what's the running time usingsonic_basic_facts?

I didn't measure the running time of sonic_basic_facts. Generally it is similar as running a simple command on DUT.
Overall, when there is no cache, a script execution time can be 20 seconds less. When there is already cache, the time saving is just around 2 seconds.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin wangxin force-pushed the improve-sonichost-init branch from b316cc9 to 6bc1457 Compare December 1, 2025 02:43
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin
Copy link
Copy Markdown
Collaborator Author

wangxin commented Dec 18, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

StormLiangMS
StormLiangMS previously approved these changes Jan 13, 2026
Copy link
Copy Markdown
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and one minor question.

@StormLiangMS
Copy link
Copy Markdown
Collaborator

One more question, do we get this enhancement tested on a multi asic devices?

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin
Copy link
Copy Markdown
Collaborator Author

wangxin commented Jan 13, 2026

One more question, do we get this enhancement tested on a multi asic devices?

Yes. Multi-ASIC is covered in PR testing.

@StormLiangMS
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin
Copy link
Copy Markdown
Collaborator Author

wangxin commented Jan 20, 2026

/azpw run

@mssonicbld
Copy link
Copy Markdown
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin
Copy link
Copy Markdown
Collaborator Author

wangxin commented Feb 10, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin wangxin force-pushed the improve-sonichost-init branch from 21f2e1e to 9cc147a Compare March 6, 2026 03:14
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin
Copy link
Copy Markdown
Collaborator Author

wangxin commented Mar 12, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

wangxin added 2 commits March 24, 2026 16:15
The SonicHost __init__ method needs to run multiple commands on the SONiC host to gather some basic facts. Execution of each command has some overhead. To improve the performance, multi threads were introduced to run the commands in parallel. It looks like a over kill. In some scenarios, it will cause multiple threads in multiple threads and could make things even more complicated. What's more, getting versions still need to run commands on SONiC host multiple times.

To simplify the procedure of gathering facts and ensure performance at the same time, this change introduced a new customized ansible module `sonic_basic_facts.py`. This module will be executed on SONiC host and use various methods to directly gather facts:
* call sonic_py_common library
* read from config_db using DB connector
* read files
The time requred to run the `sonic_basic_facts` module is similiar as running a single command on the SONiC device.
By this method, the code for gathering facts is significantly simplified with even better performance. When there is no cache, run a script may need 10 seconds less.

The __init__ method of MultiAsicSonicHost is also updated to use gathered facts instead of running the config_facts module which is a little bit slow and produces too much log.

Signed-off-by: Xin Wang <xiwang5@microsoft.com>
Signed-off-by: Xin Wang <xiwang5@microsoft.com>
@wangxinbot wangxinbot force-pushed the improve-sonichost-init branch from 9cc147a to 843f73c Compare March 24, 2026 08:21
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@StormLiangMS StormLiangMS merged commit 31b53be into sonic-net:master Mar 26, 2026
18 checks passed
@dypet dypet mentioned this pull request Mar 26, 2026
12 tasks
ravaliyel pushed a commit to ravaliyel/sonic-mgmt that referenced this pull request Mar 27, 2026
…21442)

* Improve facts gathering while creating SonicHost instance

The SonicHost __init__ method needs to run multiple commands on the SONiC host to gather some basic facts. Execution of each command has some overhead. To improve the performance, multi threads were introduced to run the commands in parallel. It looks like a over kill. In some scenarios, it will cause multiple threads in multiple threads and could make things even more complicated. What's more, getting versions still need to run commands on SONiC host multiple times.

To simplify the procedure of gathering facts and ensure performance at the same time, this change introduced a new customized ansible module `sonic_basic_facts.py`. This module will be executed on SONiC host and use various methods to directly gather facts:
* call sonic_py_common library
* read from config_db using DB connector
* read files
The time requred to run the `sonic_basic_facts` module is similiar as running a single command on the SONiC device.
By this method, the code for gathering facts is significantly simplified with even better performance. When there is no cache, run a script may need 10 seconds less.

The __init__ method of MultiAsicSonicHost is also updated to use gathered facts instead of running the config_facts module which is a little bit slow and produces too much log.

Signed-off-by: Xin Wang <xiwang5@microsoft.com>

* Improve robustness of getting router_subtype key

Signed-off-by: Xin Wang <xiwang5@microsoft.com>

---------

Signed-off-by: Xin Wang <xiwang5@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants