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.

Sign up for GitHub

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

Jump to bottom

qa: lower mds_session_metadata_threshold for tests #53873

Merged
merged 2 commits into from Oct 9, 2023
Merged

qa: lower mds_session_metadata_threshold for tests #53873

merged 2 commits into from Oct 9, 2023

Conversation

vshankar
Copy link
Contributor

@vshankar vshankar commented Oct 7, 2023 •
edited by rishabh-d-dave

... and increase the number of files that are created so as to hit the threshold.

Fixes: http://tracker.ceph.com/issues/62873

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@vshankar vshankar added the cephfs Ceph File System label Oct 7, 2023
@vshankar vshankar requested a review from a team October 7, 2023 16:48
@github-actions github-actions bot added the tests label Oct 7, 2023
@vshankar
Copy link
Contributor Author

vshankar commented Oct 7, 2023

https://pulpito.ceph.com/vshankar-2023-10-07_16:15:40-fs-wip-vshankar-testing-20230926.081818-testing-default-smithi/

@vshankar
Copy link
Contributor Author

vshankar commented Oct 9, 2023

https://pulpito.ceph.com/vshankar-2023-10-09_04:32:30-fs-wip-vshankar-testing-20230926.081818-testing-default-smithi/

@vshankar
Copy link
Contributor Author

vshankar commented Oct 9, 2023

jenkins retest this please

rishabh-d-dave
rishabh-d-dave approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave left a comment

Choose a reason for hiding this comment

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

For first commit message,it'll be useful to explain why are increasing and decreasing these certain numbers.

For second commit message, s/md_thresh_evicted/mdthresh_evicted is part of commit title as well as body. It'll be fine if we move it entirely to the body.

mchangir
mchangir approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@mchangir mchangir left a comment

Choose a reason for hiding this comment

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

LGTM

@vshankar
Copy link
Contributor Author

vshankar commented Oct 9, 2023

For first commit message,it'll be useful to explain why are increasing and decreasing these certain numbers.

Fair enough.

For second commit message, s/md_thresh_evicted/mdthresh_evicted is part of commit title as well as body. It'll be fine if we move it entirely to the body.

Not really - only the commit title.

@vshankar
qa: lower mds_session_metadata_threshold for tests
92200d9 
... and increase the number of files that are created so as to
hit the threshold with a high probability.

Fixes: http://tracker.ceph.com/issues/62873
Signed-off-by: Venky Shankar <vshankar@redhat.com>
@vshankar
qa: typo fix when checking for perf counter - s/md_thresh_evicted/mdt…
5856a1e 
…hresh_evicted

Signed-off-by: Venky Shankar <vshankar@redhat.com>
@vshankar
Copy link
Contributor Author

vshankar commented Oct 9, 2023

Added some code commentary and pushed.

leonid-s-usov
leonid-s-usov approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@leonid-s-usov leonid-s-usov left a comment

Choose a reason for hiding this comment

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

I'm still not happy with 100000 as a reasonable number of files for a test, and in general with the hardcoded magic numbers which are hard to relate to (why is 5000 low? compared to what?) but given your suggestion to clean up I'll approve the change.

@vshankar
Copy link
Contributor Author

vshankar commented Oct 9, 2023

I'm still not happy with 100000 as a reasonable number of files for a test, and in general with the hardcoded magic numbers which are hard to relate to (why is 5000 low? compared to what?) but given your suggestion to clean up I'll approve the change.

Yeh - those magic numbers are hard to maintain. Let's plan a cleanup.

@vshankar
Copy link
Contributor Author

vshankar commented Oct 9, 2023

https://pulpito.ceph.com/vshankar-2023-10-09_11:09:04-fs-wip-vshankar-testing-20230926.081818-testing-default-smithi/

@vshankar vshankar merged commit 1568321 into ceph:main Oct 9, 2023
10 of 11 checks passed
@rishabh-d-dave rishabh-d-dave mentioned this pull request Nov 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leonid-s-usov leonid-s-usov leonid-s-usov approved these changes

@rishabh-d-dave rishabh-d-dave rishabh-d-dave approved these changes

@mchangir mchangir mchangir approved these changes

Assignees
No one assigned
Labels
cephfs Ceph File System tests
Projects
None yet
Milestone
No milestone
4 participants
@vshankar @leonid-s-usov @rishabh-d-dave @mchangir