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

luminous: mds: log evicted clients to clog/dbg #25858

Merged
merged 1 commit into from Feb 26, 2019

Conversation

Projects
None yet
4 participants
@ashishkumsingh
Copy link
Contributor

commented Jan 9, 2019

@batrick batrick added the cephfs label Jan 11, 2019

@batrick batrick added this to the luminous milestone Jan 11, 2019

@@ -3059,11 +3059,21 @@ bool MDSRank::evict_client(int64_t session_id,
return false;
}

auto& addr = session->info.inst.addr;
{
CachedStackStringStream _ss;

This comment has been minimized.

Copy link
@batrick

batrick Jan 11, 2019

Member

Please use std::stringstream instead.

@smithfarm

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2019

@ashishkumsingh Failed make check, @batrick suggested a fix, please take a look.

@ashishkumsingh ashishkumsingh force-pushed the ashishkumsingh:wip-37823-luminous branch 2 times, most recently from cd9a6e5 to d4f8858 Jan 16, 2019

@smithfarm
Copy link
Contributor

left a comment

Please add a "Conflicts" section to the commit message mentioning the manual changes you made.

@smithfarm

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2019

Build failure is:

/home/jenkins-build/build/workspace/ceph-pull-requests/src/mds/MDSRank.cc: In member function ‘bool MDSRank::evict_client(int64_t, bool, bool, std::stringstream&, Context*)’:
/home/jenkins-build/build/workspace/ceph-pull-requests/src/mds/MDSRank.cc:3072:20: error: ‘std::stringstream {aka class std::__cxx11::basic_stringstream<char>}’ has no member named ‘get_stream’
     auto& ss = _ss.get_stream();
                    ^
src/mds/CMakeFiles/mds.dir/build.make:110: recipe for target 'src/mds/CMakeFiles/mds.dir/MDSRank.cc.o' failed

(Putting it here because the Jenkins console log will disappear within a very short time)

ss << "Evicting " << (blacklist ? "(and blacklisting) " : "")
<< "client session " << session_id << " (" << addr << ")";
dout(1) << ss.strv() << dendl;
clog->info() << ss.strv();

This comment has been minimized.

Copy link
@smithfarm

smithfarm Jan 17, 2019

Contributor

I'm not well-versed in std::stringstream, but it doesn't look too complicated. Something like this:

{
  std::stringstream ss;
  ss << "Evicting " << (blacklist ? "(and blacklisting) " : "")
     << "client session " << session_id << " (" << addr << ")";
  dout(1) << ss.str() << dendl;
  clog->info() << ss.str();
}

@batrick Does that look right?

This comment has been minimized.

Copy link
@batrick

This comment has been minimized.

Copy link
@ashishkumsingh

ashishkumsingh Jan 22, 2019

Author Contributor

@smithfarm @batrick thank you for the help. I have made the changes and commit.

@ashishkumsingh ashishkumsingh force-pushed the ashishkumsingh:wip-37823-luminous branch from d4f8858 to 3726435 Jan 22, 2019

mds: log evicted clients to clog/dbg
Fixes: http://tracker.ceph.com/issues/37639
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
(cherry picked from commit 7a81421)

Conflicts:
	src/mds/MDSRank.cc
	  - Used 'std::stringstream' instead of 'CachedStackStringStream'
	    and repllaced the 'ss.strv()' to 'ss.str()'.

@ashishkumsingh ashishkumsingh force-pushed the ashishkumsingh:wip-37823-luminous branch from 3726435 to ce61c84 Jan 22, 2019

LGTM

@yuriw

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2019

@yuriw yuriw merged commit 1ca8cbb into ceph:luminous Feb 26, 2019

4 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.