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

mds: track global rstat progagation #32126

Closed
wants to merge 8 commits into from
Closed

mds: track global rstat progagation #32126

wants to merge 8 commits into from

Conversation

ukernel
Copy link
Contributor

@ukernel ukernel commented Dec 9, 2019

Fixes: https://tracker.ceph.com/issues/24725

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • 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 backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@ukernel ukernel added cephfs Ceph File System feature labels Dec 9, 2019
@ukernel ukernel force-pushed the wip-24725 branch 3 times, most recently from d150211 to dedb15c Compare December 9, 2019 14:46
@ukernel ukernel mentioned this pull request Dec 9, 2019
Closed
3 tasks
@ukernel ukernel force-pushed the wip-24725 branch 4 times, most recently from e64d466 to d2c9cfa Compare December 18, 2019 13:29
@ukernel ukernel force-pushed the wip-24725 branch 2 times, most recently from dc774e9 to 69eaac7 Compare December 25, 2019 16:55
@batrick
Copy link
Member

batrick commented Jan 6, 2020

Supersedes #22781.

@ukernel
Copy link
Contributor Author

ukernel commented Jan 16, 2020

test case added

@ukernel
Copy link
Contributor Author

ukernel commented Jan 16, 2020

http://qa-proxy.ceph.com/teuthology/zyan-2020-01-16_12:32:06-multimds-wip-24725-distro-basic-smithi/4673845/teuthology.log

@ukernel ukernel mentioned this pull request Mar 31, 2020
Closed
3 tasks
fullerdj
fullerdj reviewed May 18, 2020
View reviewed changes
src/mds/CDir.cc
return;

if (pf->rstat.dirty_from.is_zero())
dout(1) << __func__ << " zero dirty_from " << *this << dendl;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we probably don't want debug level 1 here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"rstat dirty and dirty_from is zero" is unexpected. should only happens when upgrade from old mds

@ukernel ukernel force-pushed the wip-24725 branch 2 times, most recently from f9c3f8d to 848d5b4 Compare July 2, 2020 09:51
batrick
batrick requested changes Jul 9, 2020
View reviewed changes
Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

Is this also fixing https://tracker.ceph.com/issues/36171 ? If so, please update the tracker/commits.

qa/tasks/cephfs/test_multimds_misc.py Outdated Show resolved Hide resolved
src/mds/MDSRank.cc
return -EINVAL;
}

CInode *root = mdcache->get_root();
Copy link
Member

Choose a reason for hiding this comment

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

Why not have this command accept any path?

Copy link
Contributor Author

@ukernel ukernel Jul 14, 2020 •
edited

Choose a reason for hiding this comment

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

because this implementation can only tracks if rstats are propagated to root.

src/mds/MDSRank.cc
Comment on lines +3601 to +3608
f->open_object_section("root rstats");
f->dump_unsigned("rbytes", rstat.rbytes);
f->dump_unsigned("rfiles", rstat.rfiles);
f->dump_unsigned("rsubdirs", rstat.rsubdirs);
f->dump_unsigned("rsnaps", rstat.rsnaps);
f->dump_stream("rctime") << rstat.rctime;
f->dump_stream("flushed_to") << locker->get_rstat_flushed_to();
f->dump_stream("mds_time") << ceph_clock_now();
f->close_section();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
f->open_object_section("root rstats");
f->dump_unsigned("rbytes", rstat.rbytes);
f->dump_unsigned("rfiles", rstat.rfiles);
f->dump_unsigned("rsubdirs", rstat.rsubdirs);
f->dump_unsigned("rsnaps", rstat.rsnaps);
f->dump_stream("rctime") << rstat.rctime;
f->dump_stream("flushed_to") << locker->get_rstat_flushed_to();
f->dump_stream("mds_time") << ceph_clock_now();
f->close_section();
f->open_object_section("root rstats");
f->dump_object("rstats", rstat);
f->dump_string("path", path);
f->dump_stream("flushed_to") << locker->get_rstat_flushed_to();
f->dump_stream("mds_time") << ceph_clock_now();
f->close_section();

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will include 'dirty_from' field of rstat, which is meaningless for root inode

qa/tasks/cephfs/test_multimds_misc.py Show resolved Hide resolved
qa/tasks/cephfs/test_multimds_misc.py Outdated Show resolved Hide resolved
@ukernel ukernel force-pushed the wip-24725 branch 2 times, most recently from cb495bf to b0059be Compare July 20, 2020 08:44
@ukernel
Copy link
Contributor Author

ukernel commented Jul 21, 2020

Is this also fixing https://tracker.ceph.com/issues/36171 ? If so, please update the tracker/commits.

no. it doesn't

@ukernel
mds: avoid calling mdlog->flush() multiple times in tick
74b5d34 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: introduce CDir::mark_dirty_{fragstat,rstat}
67ec535 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: propagate dirty rstat to base inodes
427d8a7 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: add dirty_from to nest_info_t
68bda4c 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: track dirty rstat propagation (single mds)
c101996 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: gather status of rstats progagation from multiple mds
f0cefdd 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
mds: add command that gets status of rstats progagation
d61b679 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
qa/cephfs: add test for rstats progagation
aeafdc8 
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@ukernel
Copy link
Contributor Author

ukernel commented Aug 3, 2020

jenkins retest this please

@vshankar vshankar self-requested a review August 13, 2020 16:29
@stale
Copy link

stale bot commented Oct 12, 2020

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Oct 12, 2020
@vshankar vshankar closed this Oct 12, 2020
@vshankar
Copy link
Contributor

(closed since @ukernel feels that this is still buggy and is not worth investing time on)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fullerdj fullerdj fullerdj left review comments

@batrick batrick batrick requested changes

@vshankar vshankar Awaiting requested review from vshankar

Assignees
No one assigned
Labels
cephfs Ceph File System feature needs-review stale
Projects
None yet
Milestone
No milestone
5 participants
@ukernel @batrick @vshankar @fullerdj @gregsfortytwo