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

Fix error while fetching mysql pid from psutil #1620

Merged
merged 6 commits into from
May 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ jobs:
env: CHECK=marathon
- stage: test
env: CHECK=mcache
# - stage: test
# env: CHECK=mysql
- stage: test
env: CHECK=mysql
- stage: test
env: CHECK=network
- stage: test
Expand Down
28 changes: 19 additions & 9 deletions mysql/datadog_checks/mysql/mysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
RATE = "rate"
COUNT = "count"
MONOTONIC = "monotonic_count"
PROC_NAME = 'mysqld'

# Vars found in "SHOW STATUS;"
STATUS_VARS = {
Expand Down Expand Up @@ -789,10 +790,10 @@ def _collect_system_metrics(self, host, db, tags):
self.warning("Error while reading mysql (pid: %s) procfs data\n%s"
% (pid, traceback.format_exc()))

def _get_server_pid(self, db):
pid = None

# Try to get pid from pid file, it can fail for permission reason
def _get_pid_file_variable(self, db):
"""
Get the `pid_file` variable
"""
pid_file = None
try:
with closing(db.cursor()) as cursor:
Expand All @@ -801,6 +802,13 @@ def _get_server_pid(self, db):
except Exception:
self.warning("Error while fetching pid_file variable of MySQL.")

return pid_file

def _get_server_pid(self, db):
pid = None

# Try to get pid from pid file, it can fail for permission reason
pid_file = self._get_pid_file_variable(db)
if pid_file is not None:
self.log.debug("pid file: %s" % str(pid_file))
try:
Expand All @@ -812,12 +820,14 @@ def _get_server_pid(self, db):

# If pid has not been found, read it from ps
if pid is None and PSUTIL_AVAILABLE:
try:
for proc in psutil.process_iter():
if proc.name() == "mysqld":
for proc in psutil.process_iter():
try:
if proc.name() == PROC_NAME:
pid = proc.pid
except Exception:
self.log.exception("Error while fetching mysql pid from psutil")
except (psutil.AccessDenied, psutil.ZombieProcess, psutil.NoSuchProcess):
continue
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we debug log the exception here to help if we run into issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure it'd help, if the codepath gets there it means a process died during the iteration. If that process was exactly mysql, the pid would be None anyways so in terms of responsibilities of the function we're good. If that wasn't mysql, we don't care at all.

except Exception:
self.log.exception("Error while fetching mysql pid from psutil")

return pid

Expand Down
1 change: 1 addition & 0 deletions mysql/requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
mock==2.0.0
pytest
psutil
1 change: 1 addition & 0 deletions mysql/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@

USER = 'dog'
PASS = 'dog'
MARIA_ROOT_PASS = 'master_root_password'
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ services:
- MARIADB_DATABASE=my_database
ports:
- "${MYSQL_PORT}:3306"
networks:
- network1

mysql-slave:
image: "${MYSQL_DOCKER_REPO}:${MYSQL_VERSION}"
Expand All @@ -31,28 +29,5 @@ services:
- MARIADB_MASTER_ROOT_PASSWORD=master_root_password
ports:
- "${MYSQL_SLAVE_PORT}:3306"
volumes:
- ${WAIT_FOR_IT_SCRIPT_PATH}:/usr/local/bin/wait-for-it.sh
networks:
- network1
# command: ["wait-for-it.sh", "mysql-master:3306", "-t", "30", "--", "/app-entrypoint.sh", "/run.sh"]
depends_on:
- "mysql-master"

mysql-setup:
image: "mysql:latest"
environment:
- MYSQL_SCRIPT_OPTIONS=${MYSQL_SCRIPT_OPTIONS}
networks:
- network1
volumes:
- ${MYSQL_SHELL_SCRIPT}:/usr/local/bin/setup_mysql.sh
- ${WAIT_FOR_IT_SCRIPT_PATH}:/usr/local/bin/wait-for-it.sh
depends_on:
- "mysql-slave"
command: ["wait-for-it.sh", "mysql-master:3306", "-t", "120", "--", "setup_mysql.sh"]


networks:
network1:
name: mysql_default
26 changes: 2 additions & 24 deletions mysql/tests/compose/mysql.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ services:
- MYSQL_ALLOW_EMPTY_PASSWORD=1
ports:
- "${MYSQL_PORT}:3306"
networks:
- network1

mysql-slave:
image: "${MYSQL_DOCKER_REPO}:${MYSQL_VERSION}"
Expand All @@ -19,27 +17,7 @@ services:
ports:
- "${MYSQL_SLAVE_PORT}:3306"
volumes:
- ${WAIT_FOR_IT_SCRIPT_PATH}:/usr/local/bin/wait-for-it.sh
networks:
- network1
command: ["wait-for-it.sh", "mysql-master:3306", "-t", "120", "--", "replication-entrypoint.sh", "mysqld"]
- ${WAIT_FOR_IT_SCRIPT_PATH}:/wait-for-it.sh
command: ["bash", "-c", "chmod +x /wait-for-it.sh && /wait-for-it.sh mysql-master:3306 -t 120 -- replication-entrypoint.sh mysqld"]
depends_on:
- "mysql-master"

mysql-setup:
image: "mysql:${MYSQL_VERSION}"
environment:
- MYSQL_SCRIPT_OPTIONS=${MYSQL_SCRIPT_OPTIONS}
networks:
- network1
volumes:
- ${MYSQL_SHELL_SCRIPT}:/usr/local/bin/setup_mysql.sh
- ${WAIT_FOR_IT_SCRIPT_PATH}:/usr/local/bin/wait-for-it.sh
depends_on:
- "mysql-slave"
command: ["wait-for-it.sh", "mysql-master:3306", "-t", "120", "--", "setup_mysql.sh"]


networks:
network1:
name: mysql_default
Empty file.
16 changes: 0 additions & 16 deletions mysql/tests/compose/setup_mysql.sh

This file was deleted.

140 changes: 62 additions & 78 deletions mysql/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,16 @@
import subprocess
import time
import os
import sys

import pytest
import pymysql
import logging

import common
from . import common

log = logging.getLogger('test_mysql')


def wait_for_mysql():
"""
Wait for the slave to connect to the master
"""
connected = False
for i in xrange(0, 100):
try:
pymysql.connect(
host=common.HOST,
port=common.PORT,
user=common.USER,
passwd=common.PASS,
connect_timeout=2
)
pymysql.connect(
host=common.HOST,
port=common.SLAVE_PORT,
user=common.USER,
passwd=common.PASS,
connect_timeout=2
)
connected = True
return True
except Exception as e:
log.debug("exception: {}".format(e))
time.sleep(2)

return connected
MYSQL_FLAVOR = os.getenv('MYSQL_FLAVOR', 'mysql')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not have the version here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used only once so I thought it didn't deserve to be a module-global

COMPOSE_FILE = '{}.yaml'.format(MYSQL_FLAVOR)


@pytest.fixture(scope="session")
Expand All @@ -53,35 +25,54 @@ def spin_up_mysql():
up.
"""
env = os.environ

env['MYSQL_DOCKER_REPO'] = _mysql_docker_repo()
env['MYSQL_PORT'] = str(common.PORT)
env['MYSQL_SLAVE_PORT'] = str(common.SLAVE_PORT)
env['MYSQL_SHELL_SCRIPT'] = _mysql_shell_script()
env['WAIT_FOR_IT_SCRIPT_PATH'] = _wait_for_it_script()
env['MYSQL_SCRIPT_OPTIONS'] = _mysql_script_options()

args = [
"docker-compose",
"-f", os.path.join(common.HERE, 'compose', _compose_file())
"-f", os.path.join(common.HERE, 'compose', COMPOSE_FILE)
]
subprocess.check_call(args + ["down"], env=env)

subprocess.check_call(args + ["up", "-d"], env=env)
# wait for the cluster to be up before yielding
if not wait_for_mysql():
containers = [
"compose_mysql-master_1",
"compose_mysql-slave_1",
"compose_mysql-setup_1",
]
for container in containers:
args = [
"docker",
"logs",
container,
]
subprocess.check_call(args, env=env)
raise Exception("not working")

# wait for the master and setup the database
started = False
for _ in xrange(15):
try:
passw = common.MARIA_ROOT_PASS if MYSQL_FLAVOR == 'mariadb' else ''
conn = pymysql.connect(host=common.HOST, port=common.PORT, user='root', password=passw)
_setup_master(conn)
sys.stderr.write("Master connected!\n")
started = True
break
except Exception as e:
sys.stderr.write("Exception starting master: {}\n".format(e))
time.sleep(2)

if not started:
subprocess.check_call(args + ["logs", "mysql-master"], env=env)
subprocess.check_call(args + ["down"], env=env)
raise Exception("Timeout starting master")

# wait for the slave
started = False
for _ in xrange(60):
try:
pymysql.connect(host=common.HOST, port=common.SLAVE_PORT, user=common.USER, passwd=common.PASS)
sys.stderr.write("Slave connected!\n")
started = True
break
except Exception as e:
sys.stderr.write("Exception starting slave: {}\n".format(e))
time.sleep(2)

if not started:
subprocess.check_call(args + ["logs", "mysql-slave"], env=env)
subprocess.check_call(args + ["down"], env=env)
raise Exception("Timeout starting slave")

yield
subprocess.check_call(args + ["down"], env=env)

Expand All @@ -93,41 +84,34 @@ def aggregator():
return aggregator


def _mysql_script_options():
if _mysql_flavor() == 'mariadb':
return '--password=master_root_password'
return ''


def _compose_file():
if _mysql_flavor() == 'mysql':
return 'mysql.yaml'
elif _mysql_flavor() == 'mariadb':
return 'maria.yaml'


def _mysql_flavor():
return os.getenv('MYSQL_FLAVOR', 'mysql')


def _mysql_version():
return os.getenv('MYSQL_VERSION', 'mysql')


def _mysql_shell_script():
return os.path.join(common.HERE, 'compose', 'setup_mysql.sh')
def _setup_master(conn):
cur = conn.cursor()
cur.execute("CREATE USER 'dog'@'%' IDENTIFIED BY 'dog';")
cur.execute("GRANT REPLICATION CLIENT ON *.* TO 'dog'@'%' WITH MAX_USER_CONNECTIONS 5;")
cur.execute("GRANT PROCESS ON *.* TO 'dog'@'%';")
cur.execute("CREATE DATABASE testdb;")
cur.execute("CREATE TABLE testdb.users (name VARCHAR(20), age INT);")
cur.execute("GRANT SELECT ON testdb.users TO 'dog'@'%'")
cur.execute("INSERT INTO testdb.users (name,age) VALUES('Alice',25);")
cur.execute("INSERT INTO testdb.users (name,age) VALUES('Bob',20);")
cur.execute("GRANT SELECT ON performance_schema.* TO 'dog'@'%'")
cur.close()


def _wait_for_it_script():
"""
FIXME: relying on the filesystem layout is a bad idea, the testing helper
should expose its path through the api instead
"""
dir = os.path.join(common.TESTS_HELPER_DIR, 'scripts', 'wait-for-it.sh')
return os.path.abspath(dir)


def _mysql_docker_repo():
if _mysql_flavor() == 'mysql':
if _mysql_version() == '5.5':
if MYSQL_FLAVOR == 'mysql':
if os.getenv('MYSQL_VERSION') == '5.5':
return 'jfullaondo/mysql-replication'
else:
return 'bergerx/mysql-replication'
elif _mysql_flavor() == 'mariadb':
elif MYSQL_FLAVOR == 'mariadb':
return 'bitnami/mariadb'
Loading