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

ceph: perfcounter priorities and daemonperf updates to use them #14793

Merged
merged 28 commits into from May 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3a634ec
os/bluestore: rename nicks to be <= 4 chars
liewegas Mar 28, 2017
fd30646
osd: rename nicks to be <= 4 chars
liewegas Mar 28, 2017
25dcc96
osdc/Objecter: rename nicks to be <= 4 chars
liewegas Mar 28, 2017
5b6e791
common/perf_counters: require nicks be <= 4 chars
liewegas Mar 28, 2017
6216f21
common/perf_counters: allow priorities to be assigned to perf counters
liewegas Mar 28, 2017
2b1410b
osd: clean up whitespace in create_logger()
liewegas Mar 28, 2017
9db636c
osd: set perf counter priorities
liewegas Apr 4, 2017
cc441b4
mds: add perf counter nicks, priorities
liewegas Apr 4, 2017
fe109b4
osdc/Objecter: set perf counter priorities
liewegas Apr 4, 2017
5b743f3
common/perf_counters: replace suppress_nicks with prio adjustment
liewegas Apr 4, 2017
334f175
ceph.in: move daemon commands to their own function
dmick Apr 5, 2017
cecdadd
ceph.in: Move daemonperf to its own function
dmick Apr 14, 2017
814aae9
pybind/ceph_daemon.py: DaemonWatcher: fit display to window size
dmick Apr 18, 2017
fc1bd7e
pybind/ceph_daemon.py: stop early termination of sleep
dmick Apr 18, 2017
9d9cd5d
pybind/ceph_daemon.py: reprint headers as soon as they disappear
dmick Apr 18, 2017
51809cc
ceph.in, pybind/ceph_daemon.py: allow stat filter patterns
dmick Apr 19, 2017
583e6f6
pybind/ceph_daemon.py: allow sections to split for 'fitting'
dmick Apr 19, 2017
b297e72
ceph.in, pybind/ceph_daemon.py: add priority filtering
dmick Apr 21, 2017
3fa8bb1
ceph.in, pybind/ceph_daemon.py: add 'daemonperf list'
dmick Apr 25, 2017
a734e34
pybind/ceph_daemon: default to prio 0 if unspecified
liewegas Apr 26, 2017
dada2c5
pybind/ceph_daemon: preserve ordering
liewegas Apr 26, 2017
0aa0975
os/bluestore: set priorities
liewegas Apr 26, 2017
94cf1b0
os/bluestore/BlueFS: set priorities
liewegas Apr 26, 2017
714bd6d
osd: clean up perf counter priorities whitespace
liewegas Apr 26, 2017
00cb8f2
ceph.in: daemonperf: add 'ls' as a synonym for 'list'
dmick Apr 27, 2017
9e3d7d9
ceph.spec.in, debian/control: add dep for python-prettytable
dmick Apr 27, 2017
5e8bdb2
pybind/ceph_daemon.py: _gettermsize fails on non-terminals
dmick Apr 27, 2017
8b0bdb0
ceph.in: drop semicolons; relax priority arg ordering
dmick Apr 29, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions ceph.spec.in
Expand Up @@ -121,6 +121,7 @@ BuildRequires: pkgconfig
BuildRequires: python
BuildRequires: python-devel
BuildRequires: python-nose
BuildRequires: python-prettytable
Copy link
Contributor

Choose a reason for hiding this comment

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

do we run ceph at build-time? "make_check" is disabled by default. maybe we can guard this line like

%if 0%{with make_check}
BuildRequires:	python-prettytable
%endif

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I assume the same thing applies as for Debian, in that the package needs to be installed for make check to work. Since the make check builds don't install the built packages to pull in prettytable via Requires, there's nothing else to do it that I know of. (if there is some sort of "make check dependencies" mechanism I'm happy to learn about it.)

Copy link
Contributor

Choose a reason for hiding this comment

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

so the consumer of this line is install-deps.sh actually. i am far from an expert of dnf or its builddep plugin. so guess this is the best way to document the "make check" dependencies at this moment.

BuildRequires: python-requests
BuildRequires: python-sphinx
BuildRequires: python-virtualenv
Expand Down Expand Up @@ -256,6 +257,7 @@ Requires: python-rados = %{epoch}:%{version}-%{release}
Requires: python-rbd = %{epoch}:%{version}-%{release}
Requires: python-cephfs = %{epoch}:%{version}-%{release}
Requires: python-rgw = %{epoch}:%{version}-%{release}
Requires: python-prettytable
Requires: python-requests
%{?systemd_requires}
%if 0%{?suse_version}
Expand Down
4 changes: 3 additions & 1 deletion debian/control
Expand Up @@ -50,6 +50,7 @@ Build-Depends: bc,
python (>= 2.7),
python-all-dev,
python-nose,
python-prettytable,
Copy link
Contributor

Choose a reason for hiding this comment

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

debian packaging does not run "make check", so we can remove python-prettytable from Build-Depends.

Copy link
Member Author

Choose a reason for hiding this comment

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

The thing is, if it's not in build-depends, make check fails, because the target host doesn't have the package to run the tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed. we debated about "make check" dependencies machinery before, iirc. but no acceptable result back then. i should learn to live with it.

python-setuptools,
python-sphinx,
python3-all-dev,
Expand Down Expand Up @@ -363,7 +364,8 @@ Depends: librbd1 (= ${binary:Version}), ${misc:Depends}, ${shlibs:Depends},
python-rbd (= ${binary:Version}),
python-rgw (= ${binary:Version}),
${python:Depends},
python-requests
python-requests,
python-prettytable
Conflicts: ceph-client-tools
Replaces: ceph-client-tools,
ceph (<< 10),
Expand Down
201 changes: 147 additions & 54 deletions src/ceph.in
Expand Up @@ -38,6 +38,15 @@ FLAG_NOFORWARD = (1 << 0)
FLAG_OBSOLETE = (1 << 1)
FLAG_DEPRECATED = (1 << 2)

# priorities from src/common/perf_counters.h
PRIO_CRITICAL = 10
PRIO_INTERESTING = 8
PRIO_USEFUL = 5
PRIO_UNINTERESTING = 2
PRIO_DEBUGONLY = 0

PRIO_DEFAULT = PRIO_USEFUL

# Make life easier on developers:
# If in src/, and .libs and pybind exist here, assume we're running
# from a Ceph source dir and tweak PYTHONPATH and LD_LIBRARY_PATH
Expand Down Expand Up @@ -266,10 +275,15 @@ ping <mon.id> Send simple presence/life test to a mon
<mon.id> may be 'mon.*' for all mons
daemon {type.id|path} <cmd>
Same as --admin-daemon, but auto-find admin socket
daemonperf {type.id | path} [<interval>] [<count>]
daemonperf {type.id | path} [stat-pats] [priority] [<interval>] [<count>]
daemonperf {type.id | path} list|ls [stat-pats] [priority]
Get selected perf stats from daemon/admin socket
Optional shell-glob comma-delim match string stat-pats
Optional selection priority (can abbreviate name):
critical, interesting, useful, noninteresting, debug
List shows a table of all available stats
Run <count> times (default forever),
once per <interval> seconds (default 1)
once per <interval> seconds (default 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

why added an extra space here?

Copy link
Member Author

Choose a reason for hiding this comment

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

to show that it's part of the preceding line more clearly

""", file=sys.stdout)


Expand Down Expand Up @@ -565,6 +579,134 @@ def ping_monitor(cluster_handle, name, timeout):
print(s)
return 0


def maybe_daemon_command(parsed_args, childargs):
"""
Check if --admin-socket, daemon, or daemonperf command
if it is, returns (boolean handled, return code if handled == True)
"""

daemon_perf = False
sockpath = None
if parsed_args.admin_socket:
sockpath = parsed_args.admin_socket
elif len(childargs) > 0 and childargs[0] in ["daemon", "daemonperf"]:
daemon_perf = (childargs[0] == "daemonperf")
# Treat "daemon <path>" or "daemon <name>" like --admin_daemon <path>
# Handle "daemonperf <path>" the same but requires no trailing args
require_args = 2 if daemon_perf else 3
if len(childargs) >= require_args:
if childargs[1].find('/') >= 0:
sockpath = childargs[1]
else:
# try resolve daemon name
try:
sockpath = ceph_conf(parsed_args, 'admin_socket',
childargs[1])
except Exception as e:
print('Can\'t get admin socket path: ' + str(e), file=sys.stderr)
return True, errno.EINVAL
# for both:
childargs = childargs[2:]
else:
print('{0} requires at least {1} arguments'.format(childargs[0], require_args),
file=sys.stderr)
return True, errno.EINVAL

if sockpath and daemon_perf:
return True, daemonperf(childargs, sockpath)
elif sockpath:
try:
raw_write(admin_socket(sockpath, childargs, parsed_args.output_format))
except Exception as e:
print('admin_socket: {0}'.format(e), file=sys.stderr)
return True, errno.EINVAL
return True, 0

return False, 0


def isnum(s):
try:
float(s)
return True
except ValueError:
return False

def daemonperf(childargs, sockpath):
"""
Handle daemonperf command; returns errno or 0

daemonperf <daemon> [priority string] [statpats] [interval] [count]
daemonperf <daemon> list|ls [statpats]
"""

interval = 1
count = None
statpats = None
priority = None
do_list = False

def prio_from_name(arg):

PRIOMAP = {
'critical': PRIO_CRITICAL,
'interesting': PRIO_INTERESTING,
'useful': PRIO_USEFUL,
'uninteresting': PRIO_UNINTERESTING,
'debugonly': PRIO_DEBUGONLY,
}

if arg in PRIOMAP:
return PRIOMAP[arg]
# allow abbreviation
for name, val in PRIOMAP.items():
if name.startswith(arg):
return val
return None

# consume and analyze non-numeric args
while len(childargs) and not isnum(childargs[0]):
arg = childargs.pop(0)
# 'list'?
if arg in ['list', 'ls']:
do_list = True;
continue
# prio?
prio = prio_from_name(arg)
if prio is not None:
priority = prio
continue
# statpats
statpats = arg.split(',')

if priority is None:
priority = PRIO_DEFAULT

if len(childargs) > 0:
try:
interval = float(childargs.pop(0))
if interval < 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, why not just print the error message and return errno.EINVAL?

Copy link
Member Author

Choose a reason for hiding this comment

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

because the exception can also come from float() (bad format), and this way both errors exit the same way

Copy link
Contributor

Choose a reason for hiding this comment

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

oic.

raise ValueError
except ValueError:
print('daemonperf: interval should be a positive number', file=sys.stderr)
return errno.EINVAL

if len(childargs) > 0:
arg = childargs.pop(0)
if (not isnum(arg)) or (int(arg) < 0):
print('daemonperf: count should be a positive integer', file=sys.stderr)
return errno.EINVAL
count = int(arg)

watcher = DaemonWatcher(sockpath, statpats, priority)
if do_list:
watcher.list()
else:
watcher.run(interval, count)

return 0

###
# main
###
Expand Down Expand Up @@ -610,58 +752,9 @@ def main():

format = parsed_args.output_format

daemon_perf = False
sockpath = None
if parsed_args.admin_socket:
sockpath = parsed_args.admin_socket
elif len(childargs) > 0 and childargs[0] in ["daemon", "daemonperf"]:
daemon_perf = (childargs[0] == "daemonperf")
# Treat "daemon <path>" or "daemon <name>" like --admin_daemon <path>
# Handle "daemonperf <path>" the same but requires no trailing args
require_args = 2 if daemon_perf else 3
if len(childargs) >= require_args:
if childargs[1].find('/') >= 0:
sockpath = childargs[1]
else:
# try resolve daemon name
try:
sockpath = ceph_conf(parsed_args, 'admin_socket',
childargs[1])
except Exception as e:
print('Can\'t get admin socket path: ' + str(e), file=sys.stderr)
return errno.EINVAL
# for both:
childargs = childargs[2:]
else:
print('{0} requires at least {1} arguments'.format(childargs[0], require_args),
file=sys.stderr)
return errno.EINVAL

if sockpath and daemon_perf:
interval = 1
count = None
if len(childargs) > 0:
try:
interval = float(childargs[0])
if interval < 0:
raise ValueError
except ValueError:
print('daemonperf: interval should be a positive number', file=sys.stderr)
return errno.EINVAL
if len(childargs) > 1:
if not childargs[1].isdigit():
print('daemonperf: count should be a positive integer', file=sys.stderr)
return errno.EINVAL
count = int(childargs[1])
DaemonWatcher(sockpath).run(interval, count)
return 0
elif sockpath:
try:
raw_write(admin_socket(sockpath, childargs, format))
except Exception as e:
print('admin_socket: {0}'.format(e), file=sys.stderr)
return errno.EINVAL
return 0
done, ret = maybe_daemon_command(parsed_args, childargs)
if done:
return ret

timeout = None
if parsed_args.cluster_timeout:
Expand Down
70 changes: 46 additions & 24 deletions src/common/perf_counters.cc
Expand Up @@ -378,11 +378,17 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema,
f->dump_string("description", "");
}

if (d->nick != NULL && !suppress_nicks) {
if (d->nick != NULL) {
f->dump_string("nick", d->nick);
} else {
f->dump_string("nick", "");
}
if (d->prio) {
int p = std::max(std::min(d->prio + prio_adjust,
(int)PerfCountersBuilder::PRIO_CRITICAL),
0);
f->dump_int("priority", p);
}
f->close_section();
} else {
if (d->type & PERFCOUNTER_LONGRUNAVG) {
Expand Down Expand Up @@ -453,48 +459,59 @@ PerfCountersBuilder::~PerfCountersBuilder()
m_perf_counters = NULL;
}

void PerfCountersBuilder::add_u64_counter(int idx, const char *name,
const char *description, const char *nick)
void PerfCountersBuilder::add_u64_counter(
int idx, const char *name,
const char *description, const char *nick, int prio)
{
add_impl(idx, name, description, nick, PERFCOUNTER_U64 | PERFCOUNTER_COUNTER);
add_impl(idx, name, description, nick, prio,
PERFCOUNTER_U64 | PERFCOUNTER_COUNTER);
}

void PerfCountersBuilder::add_u64(int idx, const char *name,
const char *description, const char *nick)
void PerfCountersBuilder::add_u64(
int idx, const char *name,
const char *description, const char *nick, int prio)
{
add_impl(idx, name, description, nick, PERFCOUNTER_U64);
add_impl(idx, name, description, nick, prio, PERFCOUNTER_U64);
}

void PerfCountersBuilder::add_u64_avg(int idx, const char *name,
const char *description, const char *nick)
void PerfCountersBuilder::add_u64_avg(
int idx, const char *name,
const char *description, const char *nick, int prio)
{
add_impl(idx, name, description, nick, PERFCOUNTER_U64 | PERFCOUNTER_LONGRUNAVG);
add_impl(idx, name, description, nick, prio,
PERFCOUNTER_U64 | PERFCOUNTER_LONGRUNAVG);
}

void PerfCountersBuilder::add_time(int idx, const char *name,
const char *description, const char *nick)
void PerfCountersBuilder::add_time(
int idx, const char *name,
const char *description, const char *nick, int prio)
{
add_impl(idx, name, description, nick, PERFCOUNTER_TIME);
add_impl(idx, name, description, nick, prio, PERFCOUNTER_TIME);
}

void PerfCountersBuilder::add_time_avg(int idx, const char *name,
const char *description, const char *nick)
void PerfCountersBuilder::add_time_avg(
int idx, const char *name,
const char *description, const char *nick, int prio)
{
add_impl(idx, name, description, nick, PERFCOUNTER_TIME | PERFCOUNTER_LONGRUNAVG);
add_impl(idx, name, description, nick, prio,
PERFCOUNTER_TIME | PERFCOUNTER_LONGRUNAVG);
}

void PerfCountersBuilder::add_histogram(int idx, const char *name,
PerfHistogramCommon::axis_config_d x_axis_config,
PerfHistogramCommon::axis_config_d y_axis_config,
const char *description, const char *nick)
void PerfCountersBuilder::add_histogram(
int idx, const char *name,
PerfHistogramCommon::axis_config_d x_axis_config,
PerfHistogramCommon::axis_config_d y_axis_config,
const char *description, const char *nick, int prio)
{
add_impl(idx, name, description, nick, PERFCOUNTER_U64 | PERFCOUNTER_HISTOGRAM,
add_impl(idx, name, description, nick, prio,
PERFCOUNTER_U64 | PERFCOUNTER_HISTOGRAM,
unique_ptr<PerfHistogram<>>{new PerfHistogram<>{x_axis_config, y_axis_config}});
}

void PerfCountersBuilder::add_impl(int idx, const char *name,
const char *description, const char *nick, int ty,
unique_ptr<PerfHistogram<>> histogram)
void PerfCountersBuilder::add_impl(
int idx, const char *name,
const char *description, const char *nick, int prio, int ty,
unique_ptr<PerfHistogram<>> histogram)
{
assert(idx > m_perf_counters->m_lower_bound);
assert(idx < m_perf_counters->m_upper_bound);
Expand All @@ -504,7 +521,12 @@ void PerfCountersBuilder::add_impl(int idx, const char *name,
assert(data.type == PERFCOUNTER_NONE);
data.name = name;
data.description = description;
// nick must be <= 4 chars
if (nick) {
assert(strlen(nick) <= 4);
}
data.nick = nick;
data.prio = prio;
data.type = (enum perfcounter_type_d)ty;
data.histogram = std::move(histogram);
}
Expand Down