Skip to content

refactor: optimize pretest for mult-DUT devices#17493

Merged
yejianquan merged 1 commit intosonic-net:masterfrom
cyw233:optimize-pretest-for-multi-dut
Apr 3, 2025
Merged

refactor: optimize pretest for mult-DUT devices#17493
yejianquan merged 1 commit intosonic-net:masterfrom
cyw233:optimize-pretest-for-multi-dut

Conversation

@cyw233
Copy link
Contributor

@cyw233 cyw233 commented Mar 13, 2025

Description of PR

Optimize test_pretest.py by Python multithreading to reduce the running time.

Summary:
Fixes # (issue) Microsoft ADO 30056122

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

Approach

What is the motivation for this PR?

We noticed that some test cases in test_pretest.py can be run in parallel with Python multithreading to reduce the running time.

After this PR, I will create an internal PR to optimize the test_pretest.py test cases that only exists in our internal repo.

How did you do it?

How did you verify/test it?

I ran the updated code and can confirm it's working well.

  • For dual ToR topo, the running time will be decreased from ~21 min to ~13 min (Elastictest link before the change, Elastictest link after the change)
  • For T2 topo, the running time be decreased from ~46 min to ~18 min (Elastictest link before the change, Elastictest link after the change)

Any platform specific information?

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

Documentation

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@cyw233 cyw233 force-pushed the optimize-pretest-for-multi-dut branch from bcf2b5b to eb235f4 Compare March 13, 2025 03:28
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
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.

do you have data how much faster this PR runs on dualtor/t2 testbed?

verify_features_state, dut), "Not all service states are valid!")
logger.info("The states of features in 'CONFIG_DB' are all valid.")

with SafeThreadPoolExecutor(max_workers=8) as executor:
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we customize the work count based on the duthosts count?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, will verify and update you later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @lolyu, I did some verification and can confirm we have some good improvements on the running time. I have updated the PR description for more details. Please check, thanks!

@cyw233 cyw233 force-pushed the optimize-pretest-for-multi-dut branch from eb235f4 to 96d3cb1 Compare March 15, 2025 03:00
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@cyw233 cyw233 force-pushed the optimize-pretest-for-multi-dut branch from 96d3cb1 to a78af2b Compare March 15, 2025 03:12
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
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.

LGTM

@cyw233 cyw233 closed this Mar 21, 2025
@cyw233 cyw233 reopened this Mar 21, 2025
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@cyw233 cyw233 force-pushed the optimize-pretest-for-multi-dut branch from a78af2b to 2864bda Compare March 21, 2025 11:11
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yejianquan
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@cyw233 cyw233 force-pushed the optimize-pretest-for-multi-dut branch from 2864bda to be9e973 Compare April 1, 2025 23:01
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@yejianquan yejianquan left a comment

Choose a reason for hiding this comment

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

LGTM

@yejianquan yejianquan merged commit c458e29 into sonic-net:master Apr 3, 2025
18 checks passed
yejianquan added a commit to Azure/sonic-mgmt.msft that referenced this pull request Jun 4, 2025
… tsa pretest (#344)" (#359)

This reverts commit 7551587.

I thought we cherry picked the multithreading run of pretest PR
(sonic-net/sonic-mgmt#17493) into msft.202405
but we never did, so the PR
sonic-net/sonic-mgmt#18699 should never be
cherry picked into msft.202405 as it's only a fix when test_pretest.py
runs with multithreading.

In msft.202405 branch, we are running test_pretest.py (e.g.
test_disable_startup_tsa_tsb_service()) in a normal for loop:
https://github.com/Azure/sonic-mgmt.msft/blob/202405/tests/test_pretest.py#L385,
so we should use duthost instead of dut
opcoder0 pushed a commit to opcoder0/sonic-mgmt that referenced this pull request Dec 8, 2025
Description of PR
Optimize test_pretest.py by Python multithreading to reduce the running time.

Summary:
Fixes # (issue) Microsoft ADO 30056122

Approach
What is the motivation for this PR?
We noticed that some test cases in test_pretest.py can be run in parallel with Python multithreading to reduce the running time.

After this PR, I will create an internal PR to optimize the test_pretest.py test cases that only exists in our internal repo.

How did you do it?
How did you verify/test it?
I ran the updated code and can confirm it's working well.

For dual ToR topo, the running time will be decreased from ~21 min to ~13 min (Elastictest link before the change, Elastictest link after the change)
For T2 topo, the running time be decreased from ~46 min to ~18 min (Elastictest link before the change, Elastictest link after the change)

co-authorized by: [email protected]

Signed-off-by: opcoder0 <[email protected]>
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Dec 21, 2025
Description of PR
Optimize test_pretest.py by Python multithreading to reduce the running time.

Summary:
Fixes # (issue) Microsoft ADO 30056122

Approach
What is the motivation for this PR?
We noticed that some test cases in test_pretest.py can be run in parallel with Python multithreading to reduce the running time.

After this PR, I will create an internal PR to optimize the test_pretest.py test cases that only exists in our internal repo.

How did you do it?
How did you verify/test it?
I ran the updated code and can confirm it's working well.

For dual ToR topo, the running time will be decreased from ~21 min to ~13 min (Elastictest link before the change, Elastictest link after the change)
For T2 topo, the running time be decreased from ~46 min to ~18 min (Elastictest link before the change, Elastictest link after the change)

co-authorized by: [email protected]

Signed-off-by: Guy Shemesh <[email protected]>
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Jan 26, 2026
Description of PR
Optimize test_pretest.py by Python multithreading to reduce the running time.

Summary:
Fixes # (issue) Microsoft ADO 30056122

Approach
What is the motivation for this PR?
We noticed that some test cases in test_pretest.py can be run in parallel with Python multithreading to reduce the running time.

After this PR, I will create an internal PR to optimize the test_pretest.py test cases that only exists in our internal repo.

How did you do it?
How did you verify/test it?
I ran the updated code and can confirm it's working well.

For dual ToR topo, the running time will be decreased from ~21 min to ~13 min (Elastictest link before the change, Elastictest link after the change)
For T2 topo, the running time be decreased from ~46 min to ~18 min (Elastictest link before the change, Elastictest link after the change)

co-authorized by: [email protected]

Signed-off-by: Guy Shemesh <[email protected]>
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