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

common/ceph_context.cc: Use CEPH_DEV to reduce logfile noise #10384

Merged
merged 1 commit into from Apr 21, 2017

Conversation

Projects
None yet
3 participants
@wjwithagen
Contributor

wjwithagen commented Jul 21, 2016

Logfiles contain a lot of warnings about features set to '*'
Setting the CEPH_DEV option allows suppression of these warnings.
Can be set in vstart.sh for example.

Signed-off-by: Willem Jan Withagen wjw@digiware.nl

@@ -254,9 +254,11 @@ class CephContextObs : public md_config_obs_t {
conf->enable_experimental_unrecoverable_data_corrupting_features,
cct->_experimental_features);
ceph_spin_unlock(&cct->_feature_lock);
#if defined(NDEBUG)

This comment has been minimized.

@tchaikov

tchaikov Jul 27, 2016

Contributor

NDEBUG is defined for non-DEBUG builds, so if we want to respect this macro, better of putting

#ifndef NDEBUG

and i believe this warning is intentional, and helpful to our end users. it makes sure they are aware of the risk of using experimental features.

This comment has been minimized.

@wjwithagen

wjwithagen Aug 23, 2016

Contributor

@tchaikov
So we 100% agree.

  • it is for the end users
  • it is NonDebug

So if I want to suppress this during developement, because it does not conviene any information to developers other than logfile pollution, it think it needs to be included in a:

#ifdef NDEBUG
#endif
@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jul 28, 2016

retest this please

@wjwithagen wjwithagen changed the title from Only alert for dangerous features if running with NDEBUG to [DNM] Only alert for dangerous features if running with NDEBUG Nov 21, 2016

@wjwithagen wjwithagen changed the title from [DNM] Only alert for dangerous features if running with NDEBUG to Only alert for dangerous features if CEPH_DEV is not defined Nov 21, 2016

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Nov 21, 2016

@tchaikov
Rewrote this to start using CEPH_DEV, which I intend to use at a few more locations to reduce logfile noise.

@ghost

This comment has been minimized.

ghost commented Nov 21, 2016

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Nov 21, 2016

@dachary
Yup, :( Always happens when you do thing while you're already with one foot out of the door.
Will fix.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Nov 21, 2016

@dachary
I would like to do something similar to build/bin/ceph and the warning
DEVMODEMSG = *** DEVELOPER MODE: setting PATH, PYTHONPATH and LD_LIBRARY_PATH ***
Because that is also a rather serious spammer in logfiles, and I think developers will be able to figure this out by them selves??
Something like os.environ.has_key(CEPH_DEV)

Is that a plan?

@ghost ghost added the cleanup label Nov 21, 2016

@ghost

This comment has been minimized.

ghost commented Nov 21, 2016

@wjwithagen I'm not sure, it depends on how developers feel about such a change. I don't mind the spam but I have a high tolerance for it. Maybe starting a thread on ceph-devel is the way to go ?

@liewegas

This comment has been minimized.

Member

liewegas commented Mar 28, 2017

What about instead setting an environment variable (CEPH_DEV=1 or CEPH_DEV_UNSAFE=1)? That way vstart.sh can just set it and all the annoying warnings will go away, but production deployments (regardless of how the binary was built) will still be noisy. I'm not sure I like the idea of compiling out the warnings entirely, even for debug builds, as debug builds can also get deployed for various reasons.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Mar 28, 2017

@liewegas
That would be possible too, But then I think we need to commit the other PR first.
Once we have that, I'll also take a whack at ceph.py since that nags too about being run as developer, and that seriously clogs up all script testing logs.
Instead of death by chocolate, it is death by logs. ;-)

@wjwithagen wjwithagen changed the title from Only alert for dangerous features if CEPH_DEV is not defined to common/ceph_context.cc: Use CEPH_DEV to reduce logfile noise Apr 2, 2017

common/ceph_context.cc: Use CEPH_DEV to reduce logfile noise
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>

@wjwithagen wjwithagen requested a review from tchaikov Apr 3, 2017

@liewegas liewegas added the needs-qa label Apr 20, 2017

@tchaikov tchaikov merged commit 2b12429 into ceph:master Apr 21, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@ghost ghost referenced this pull request Jan 8, 2015

Merged

tests: resolve ceph-helpers races #3215

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