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

document more config options #25456

Merged
merged 11 commits into from Dec 18, 2018

Conversation

Projects
None yet
5 participants
@liewegas
Copy link
Member

liewegas commented Dec 9, 2018

https://tracker.ceph.com/issues/36515

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

@liewegas liewegas force-pushed the liewegas:wip-doc-more-options branch from 2236467 to bfdc1f5 Dec 10, 2018

.add_see_also("log_stderr_prefix"),

Option("mon_cluster_log_to_syslog", Option::TYPE_STR, Option::LEVEL_ADVANCED)
.set_default("default=false")
.set_description(""),
.set_description("Make monitor send clsuter log messages to syslog"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo:

Suggested change
.set_description("Make monitor send clsuter log messages to syslog"),
.set_description("Make monitor send cluster log messages to syslog"),
Option("compressor_zlib_isal", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
.set_default(false)
.set_description(""),
.set_description("Use Intel ISA-L accellerated zlib implementation if available"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo:

Suggested change
.set_description("Use Intel ISA-L accellerated zlib implementation if available"),
.set_description("Use Intel ISA-L accelerated zlib implementation if available"),
@@ -231,6 +231,12 @@
on the intended OSDs). You can reenable the old warning by setting
``mon_warn_on_misplaced`` to ``true``.

* The ``mon_osd_pool_ec_fast_reac`` option has been renamed

This comment has been minimized.

Copy link
@tchaikov

tchaikov Dec 10, 2018

Contributor

s/mon_osd_pool_ec_fast_reac/mon_osd_pool_ec_fast_read/

@liewegas liewegas force-pushed the liewegas:wip-doc-more-options branch 2 times, most recently from 8fde82e to a09c4e7 Dec 10, 2018

.set_default(false)
.set_description(""),
.set_description("Indce a daemon crash/exit if sender skips a message sequence number"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo:

Suggested change
.set_description("Indce a daemon crash/exit if sender skips a message sequence number"),
.set_description("Induce a daemon crash/exit if sender skips a message sequence number"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 13, 2018

Contributor

That typo still should be fixed :)


Option("ms_inject_delay_max", Option::TYPE_FLOAT, Option::LEVEL_DEV)
.set_default(1)
.set_description(""),
.set_description("Max delay ot inject"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo?

Suggested change
.set_description("Max delay ot inject"),
.set_description("Max delay on inject"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 13, 2018

Contributor

Dito here.

@@ -231,6 +231,12 @@
on the intended OSDs). You can reenable the old warning by setting
``mon_warn_on_misplaced`` to ``true``.

* The ``mon_osd_pool_ec_fast_read`` option has been renamed
``osd_pool_default_ec_fast_read`` to be more consitent with other

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo:

Suggested change
``osd_pool_default_ec_fast_read`` to be more consitent with other
``osd_pool_default_ec_fast_read`` to be more consistent with other

Option("mon_accept_timeout_factor", Option::TYPE_FLOAT, Option::LEVEL_ADVANCED)
.set_default(2.0)
.set_description(""),
.set_description("multiple of mon_lease for follower mons to accept proposed state changes before caling a new election")

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo:

Suggested change
.set_description("multiple of mon_lease for follower mons to accept proposed state changes before caling a new election")
.set_description("multiple of mon_lease for follower mons to accept proposed state changes before calling a new election")

Option("mon_osd_report_timeout", Option::TYPE_INT, Option::LEVEL_ADVANCED)
.set_default(900)
.set_description(""),
.set_description("time before OSDs who do not report to the mons are marked down (seconds)"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Just wondering: since we're writing "OSDs" in all caps, shouldn't we do the same for "MONs" consistently?


Option("mon_force_standby_active", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
.set_default(true)
.set_description(""),
.set_description("allow use of MDS daemons in standby-replay as replacments"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo:

Suggested change
.set_description("allow use of MDS daemons in standby-replay as replacments"),
.set_description("allow use of MDS daemons in standby-replay as replacements"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 13, 2018

Contributor

And here.

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 14, 2018

Contributor

Just one typo remaining :)

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Dec 10, 2018

@liewegas liewegas force-pushed the liewegas:wip-doc-more-options branch from a09c4e7 to 5c4b080 Dec 10, 2018


Option("mon_daemon_bytes", Option::TYPE_SIZE, Option::LEVEL_ADVANCED)
.set_default(400ul << 20)
.set_description("max bytes of outstanding mon messages mon will read offf the network"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo:

Suggested change
.set_description("max bytes of outstanding mon messages mon will read offf the network"),
.set_description("max bytes of outstanding mon messages mon will read off the network"),
.set_default(false)
.set_description(""),
.set_description("enable exxtra debugging during mon sync"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo:

Suggested change
.set_description("enable exxtra debugging during mon sync"),
.set_description("enable extra debugging during mon sync"),

Option("mon_inject_sync_get_chunk_delay", Option::TYPE_FLOAT, Option::LEVEL_DEV)
.set_default(0)
.set_description(""),
.set_description("inject dleay during sync (seconds)"),

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 10, 2018

Contributor

Typo:

Suggested change
.set_description("inject dleay during sync (seconds)"),
.set_description("inject delay during sync (seconds)"),
@LenzGr

This comment has been minimized.

Copy link
Contributor

LenzGr commented Dec 10, 2018

Maybe.. OSD is an acronym while mon is just short for monitor.

Good point. Thanks for adding these missing help texts? What's the reasoning behind changing the capital letters at the beginning of each help text to lowercase? This makes the help text look a bit odd in the Dashboard UI. For consistency, I'd suggest to always start a help text with a capital letter.

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Dec 10, 2018

@s0nea

This comment has been minimized.

Copy link
Member

s0nea commented Dec 10, 2018

Great and helpful improvement for the config options! Are you also going to add more information about which section belongs to a config option (I guess that's what the service column is for) - http://tracker.ceph.com/issues/36515 ?

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Dec 10, 2018

@liewegas liewegas force-pushed the liewegas:wip-doc-more-options branch 2 times, most recently from 01e33fe to 9418abd Dec 10, 2018

@tchaikov

This comment has been minimized.

Copy link
Contributor

tchaikov commented Dec 11, 2018

updated rocksdb submodule by accident?

@liewegas liewegas force-pushed the liewegas:wip-doc-more-options branch 3 times, most recently from 93bb25e to 2b55a99 Dec 11, 2018

liewegas added some commits Dec 9, 2018

common/options: document monitor log options
Signed-off-by: Sage Weil <sage@redhat.com>
common/options: compression options
Signed-off-by: Sage Weil <sage@redhat.com>
common/options: document misc
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas force-pushed the liewegas:wip-doc-more-options branch from 2b55a99 to 0a4eee2 Dec 13, 2018

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Dec 13, 2018

@LenzGr fixed the typos... looks ok to you?

@LenzGr

This comment has been minimized.

Copy link
Contributor

LenzGr commented Dec 13, 2018

@liewegas there still seem to be a few unfixed ones - would you mind resolving the remaining ones?

@liewegas liewegas force-pushed the liewegas:wip-doc-more-options branch from 0a4eee2 to 171619f Dec 13, 2018

liewegas added some commits Dec 9, 2018

common/options: messenger options
Signed-off-by: Sage Weil <sage@redhat.com>
common/options: document mon options
Signed-off-by: Sage Weil <sage@redhat.com>
mon: mon_osd_pool_ec_fast_read -> osd_pool_default_ec_fast_read
More consistent name!

Signed-off-by: Sage Weil <sage@redhat.com>
mon: remove dead option mon_pg_min_inactive
Signed-off-by: Sage Weil <sage@redhat.com>
common/options: mon options
Signed-off-by: Sage Weil <sage@redhat.com>
common/options: kill old mon_max_pgmap_epochs option
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas force-pushed the liewegas:wip-doc-more-options branch from 171619f to 4d2da64 Dec 13, 2018

liewegas added some commits Dec 10, 2018

common/options: more mon options
Signed-off-by: Sage Weil <sage@redhat.com>
common/options: set mon or mgr service on these options
Signed-off-by: Sage Weil <sage@redhat.com>
@LenzGr

LenzGr approved these changes Dec 14, 2018

Copy link
Contributor

LenzGr left a comment

LGTM, except for that final typo I reported - thanks!

@liewegas liewegas force-pushed the liewegas:wip-doc-more-options branch from 4d2da64 to bd993c1 Dec 14, 2018

@liewegas liewegas added the needs-qa label Dec 14, 2018

@s0nea

This comment has been minimized.

Copy link
Member

s0nea commented Dec 17, 2018

I'm sorry for the late question, but I saw some config options e.g. https://github.com/ceph/ceph/blob/master/src/common/options.cc#L507 "tagged" with service "common" or "rgw" although http://docs.ceph.com/docs/master/rados/configuration/ceph-conf/#configuration-sections says the possible sections are "global", "mon", "mgr", "osd", "mds" and "client".
I'm wondering whether the implementation or the documentation is correct (maybe common applies to global, but rgw?) or did I just mix up things?

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Dec 17, 2018

The tags and config sections aren't directly related. The tags are intended to tell you who pays attention to this config and are more specific (e.g., 'rgw').

@s0nea

This comment has been minimized.

Copy link
Member

s0nea commented Dec 17, 2018

@liewegas

The tags and config sections aren't directly related. The tags are intended to tell you who pays attention to this config and are more specific (e.g., 'rgw').

Ah, okay, thanks for the clarification!

Would there be another way to figure out which section applies to a config option? - I think it would be a great improvement if we could filter the relevant sections in the config options editor (dashboard). We're showing all sections for every config option at the moment and the user can set the value for a section even if it doesn't belong to the config option.

@liewegas liewegas merged commit bd993c1 into ceph:master Dec 18, 2018

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

liewegas added a commit that referenced this pull request Dec 18, 2018

Merge PR #25456 into master
* refs/pull/25456/head:
	common/options: set mon or mgr service on these options
	common/options: more mon options
	common/options: kill old mon_max_pgmap_epochs option
	common/options: mon options
	mon: remove dead option mon_pg_min_inactive
	mon: mon_osd_pool_ec_fast_read -> osd_pool_default_ec_fast_read
	common/options: document mon options
	common/options: messenger options
	common/options: document misc
	common/options: compression options
	common/options: document monitor log options

Reviewed-by: Lenz Grimmer <lgrimmer@suse.com>

Option("heartbeat_file", Option::TYPE_STR, Option::LEVEL_ADVANCED)
.set_default("")
.set_description(""),
.set_default("File to touch on successful internal heartbeat")

This comment has been minimized.

Copy link
@cbodley

cbodley Dec 18, 2018

Contributor

this should be set_description()? a file named 'File to touch on successful internal heartbeat' keeps getting recreated in my build directory :D fixed in #25617

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.