Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e: relax dynamic demotion first detection round requirement #1053

Merged
merged 1 commit into from Aug 22, 2023

Conversation

askervin
Copy link
Contributor

Strict requirement makes the test flaky. Give two more rounds time for detecting first memory moves.

@codecov-commenter
Copy link

Codecov Report

Merging #1053 (9f9cd7d) into master (e38b14b) will not change coverage.
Report is 2 commits behind head on master.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1053   +/-   ##
=======================================
  Coverage   31.73%   31.73%           
=======================================
  Files          64       64           
  Lines        9816     9816           
=======================================
  Hits         3115     3115           
  Misses       6410     6410           
  Partials      291      291           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -36,7 +36,7 @@ pages_per_second_per_process="$(awk '

# After how many rounds (seconds) first migrations should be visible.
first_migrations_visible="$(awk '
/PageScanInterval:/{gsub(/[^0-9]/, "", $2); print $2+3}
/PageScanInterval:/{gsub(/[^0-9]/, "", $2); print $2+5}
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to make this 10, in my testing 5 (round 15) was on the limit

Suggested change
/PageScanInterval:/{gsub(/[^0-9]/, "", $2); print $2+5}
/PageScanInterval:/{gsub(/[^0-9]/, "", $2); print $2+10}

Copy link
Contributor

Choose a reason for hiding this comment

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

Or at least +6 to have some margin for "fluctiations"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. Let's increase the margin still.

Strict requirement makes the test flaky. Give two more rounds time for
detecting first memory moves.
@askervin askervin force-pushed the 5QV_e2e_relax_dynamic_demotion branch from 9f9cd7d to 6699d6b Compare August 22, 2023 07:51
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

LGTM

@marquiz marquiz merged commit 0cd36b0 into intel:master Aug 22, 2023
6 checks passed
@marquiz marquiz mentioned this pull request Jan 8, 2024
18 tasks
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.

None yet

3 participants