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

osd: Log audit #16281

Merged
merged 1 commit into from Jul 27, 2017

Conversation

Projects
None yet
3 participants
@badone
Contributor

badone commented Jul 12, 2017

Review current log messages for consistency, accuracy and necessesity as
part of usability initiative. First in a series.

Signed-off-by: Brad Hubbard bhubbard@redhat.com

@badone

This comment has been minimized.

Contributor

badone commented Jul 18, 2017

@jdurgin Forgot to mention this... take a look?

@jdurgin

This comment has been minimized.

Member

jdurgin commented Jul 19, 2017

@jcsp wrote up some helpful guidelines for these: #16292 - looking at these in that light

@@ -992,7 +992,7 @@ void ECBackend::handle_sub_read(
hinfo = get_hash_info(i->first);
if (!hinfo) {
r = -EIO;
get_parent()->clog_error() << __func__ << ": No hinfo for " << i->first;
get_parent()->clog_error() << "No hinfo for " << i->first;

This comment has been minimized.

@jdurgin

jdurgin Jul 19, 2017

Member

hinfo doesn't mean much to a user - perhaps something like "Corruption detected: object i->first is missing hash_info"

@@ -1006,8 +1006,7 @@ void ECBackend::handle_sub_read(
j->get<1>(),
bl, j->get<2>());
if (r < 0) {
get_parent()->clog_error() << __func__
<< ": Error " << r
get_parent()->clog_error() << "Error " << r
<< " reading "

This comment has been minimized.

@jdurgin

jdurgin Jul 19, 2017

Member

"reading object "?

@@ -1231,7 +1230,7 @@ void ECBackend::handle_sub_read_reply(
err = rop.complete[iter->first].errors.begin()->second;
rop.complete[iter->first].r = err;
} else {
get_parent()->clog_error() << __func__ << ": Error(s) ignored for "
get_parent()->clog_error() << "Error(s) ignored for "

This comment has been minimized.

@jdurgin

jdurgin Jul 19, 2017

Member

Since we're continuing without needing admin intervention, should be warn level - it'll be caught during the next deep scrub anyway

@badone

This comment has been minimized.

Contributor

badone commented Jul 20, 2017

@jdurgin Good suggestions, thanks.

@badone

This comment has been minimized.

Contributor

badone commented Jul 21, 2017

Jenkins retest this please

1 similar comment
@badone

This comment has been minimized.

Contributor

badone commented Jul 25, 2017

Jenkins retest this please

osd: Log audit
Review current log messages for consistency, accuracy and necessesity as
part of usability initiative. First in a series.

Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
@badone

This comment has been minimized.

Contributor

badone commented Jul 26, 2017

@jdurgin conflicts fixed. Not sure about the run-cli-tests failure since it passes locally?

@jdurgin jdurgin added the needs-qa label Jul 26, 2017

@jdurgin jdurgin added this to the luminous milestone Jul 26, 2017

@badone

This comment has been minimized.

Contributor

badone commented Jul 26, 2017

Jenkins retest this please

@liewegas liewegas merged commit 41bcf2f into ceph:master Jul 27, 2017

4 checks passed

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
make check (arm64) make check succeeded
Details

@badone badone deleted the badone:wip-PG-cluster-log-audit branch Jul 27, 2017

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