openstack
/
charm-ceph-osd
Code Issues Proposed changes

Implement new option to enable discard on SSDs 

This change implements a new option called 'bdev-enable-discard' to control
behaviour of issuing discards to SSDs on ceph bluestore. The new code tries
to autodetect cases where it should be enabled by default but will allow
forcing if desired.

Change-Id: I7b83605c827eb4058bc4b46c92eb114c11108c93
Closes-Bug: #1788433
This commit is contained in:
Andre Ruiz 2019-02-26 17:13:40 +01:00
parent 441b9432e7
commit 56495eecba
7 changed files with 115 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
config.yaml
View File

@ -63,6 +63,15 @@ options:
.
For ceph >= 0.56.6 these can also be directories instead of devices - the
charm assumes anything not starting with /dev is a directory instead.
bdev-enable-discard:
type: string
default: auto
description: |
Enables async discard on devices. This option will enable/disable both
bdev-enable-discard and bdev-async-discard options in ceph configuration
at the same time. The default value "auto" will try to autodetect and
should work in most cases. If you need to force a behaviour you can
set it to "enable" or "disable". Only applies for Ceph Mimic or later.
osd-journal:
type: string
default:

8
hooks/ceph_hooks.py
View File

@ -80,6 +80,7 @@ from utils import (
get_cluster_addr,
get_blacklist,
get_journal_devices,
should_enable_discard,
)
from charmhelpers.contrib.openstack.alternatives import install_alternative
from charmhelpers.contrib.network.ip import (
@ -388,6 +389,13 @@ def get_ceph_context(upgrading=False):
'bluestore_block_db_size': config('bluestore-block-db-size'),
}
if config('bdev-enable-discard').lower() == 'enabled':
cephcontext['bdev_discard'] = True
elif config('bdev-enable-discard').lower() == 'auto':
cephcontext['bdev_discard'] = should_enable_discard(get_devices())
else:
cephcontext['bdev_discard'] = False
if config('prefer-ipv6'):
dynamic_ipv6_address = get_ipv6_addr()[0]
if not public_network:

33
hooks/utils.py
View File

@ -15,6 +15,7 @@
import re
import os
import socket
import subprocess
from charmhelpers.core.hookenv import (
unit_get,
@ -23,6 +24,7 @@ from charmhelpers.core.hookenv import (
network_get_primary_address,
log,
DEBUG,
WARNING,
status_set,
storage_get,
storage_list,
@ -194,3 +196,34 @@ def get_journal_devices():
_blacklist = get_blacklist()
return set(device for device in devices
if device not in _blacklist and os.path.exists(device))
def should_enable_discard(devices):
"""
Tries to autodetect if we can enable discard on devices and if that
discard can be asynchronous. We want to enable both options if there's
any SSDs unless any of them are using SATA <= 3.0, in which case
discard is supported but is a blocking operation.
"""
discard_enable = True
for device in devices:
# whitelist some devices that do not need checking
if (device.startswith("/dev/nvme") or
device.startswith("/dev/vd")):
continue
if (device.startswith("/dev/") and
os.path.exists(device) and
is_sata30orless(device)):
discard_enable = False
log("SSD Discard autodetection: {} is forcing discard off"
"(sata <= 3.0)".format(device), level=WARNING)
return discard_enable
def is_sata30orless(device):
result = subprocess.check_output(["/usr/sbin/smartctl", "-i", device])
print(result)
for line in str(result).split("\\n"):
if re.match("SATA Version is: *SATA (1\.|2\.|3\.0)", str(line)):
return True
return False

2
lib/ceph/utils.py
View File

@ -82,7 +82,7 @@ QUORUM = [LEADER, PEON]
PACKAGES = ['ceph', 'gdisk', 'btrfs-tools',
'radosgw', 'xfsprogs',
'lvm2', 'parted']
'lvm2', 'parted', 'smartmontools']
CEPH_KEY_MANAGER = 'ceph'
VAULT_KEY_MANAGER = 'vault'

2
templates/ceph.conf
View File

@ -75,6 +75,8 @@ osd journal size = {{ osd_journal_size }}
filestore xattr use omap = true
journal dio = {{ dio }}
{%- endif %}
bdev enable discard = {{ bdev_discard }}
bdev async discard = {{ bdev_discard }}
{%- if short_object_len %}
osd max object name len = 256

8
unit_tests/test_ceph_hooks.py
View File

@ -37,6 +37,8 @@ CHARM_CONFIG = {'config-flags': '',
'customize-failure-domain': False,
'bluestore': False,
'crush-initial-weight': '0',
'bdev-enable-discard': 'enabled',
'osd-devices': '/dev/vdb',
'bluestore': False,
'bluestore-block-wal-size': 0,
'bluestore-block-db-size': 0,
@ -84,6 +86,7 @@ class CephHooksTestCase(unittest.TestCase):
'short_object_len': True,
'upgrade_in_progress': False,
'use_syslog': 'true',
'bdev_discard': True,
'bluestore': False,
'bluestore_experimental': False,
'bluestore_block_wal_size': 0,
@ -123,6 +126,7 @@ class CephHooksTestCase(unittest.TestCase):
'short_object_len': True,
'upgrade_in_progress': False,
'use_syslog': 'true',
'bdev_discard': True,
'bluestore': False,
'bluestore_experimental': True,
'bluestore_block_wal_size': 0,
@ -168,6 +172,7 @@ class CephHooksTestCase(unittest.TestCase):
'short_object_len': True,
'upgrade_in_progress': False,
'use_syslog': 'true',
'bdev_discard': True,
'bluestore': True,
'bluestore_experimental': False,
'bluestore_block_wal_size': BLUESTORE_WAL_TEST_SIZE,
@ -210,6 +215,7 @@ class CephHooksTestCase(unittest.TestCase):
'short_object_len': True,
'upgrade_in_progress': False,
'use_syslog': 'true',
'bdev_discard': True,
'bluestore': True,
'bluestore_experimental': True,
'bluestore_block_wal_size': BLUESTORE_WAL_TEST_SIZE,
@ -250,6 +256,7 @@ class CephHooksTestCase(unittest.TestCase):
'short_object_len': True,
'upgrade_in_progress': False,
'use_syslog': 'true',
'bdev_discard': True,
'bluestore': False,
'bluestore_experimental': False,
'bluestore_block_wal_size': 0,
@ -292,6 +299,7 @@ class CephHooksTestCase(unittest.TestCase):
'short_object_len': True,
'upgrade_in_progress': False,
'use_syslog': 'true',
'bdev_discard': True,
'bluestore': False,
'bluestore_experimental': False,
'bluestore_block_wal_size': 0,

54
unit_tests/test_ceph_utils.py
View File

@ -61,3 +61,57 @@ class CephUtilsTestCase(unittest.TestCase):
mock_os_path_exists.assert_called()
mock_get_blacklist.assert_called()
self.assertEqual(devices, set(['/dev/vdb']))
@patch('os.path.exists')
@patch.object(utils, 'is_sata30orless')
def test_should_enable_discard_yes(self, mock_is_sata30orless,
mock_os_path_exists):
devices = ['/dev/sda', '/dev/vda', '/dev/nvme0n1']
mock_os_path_exists.return_value = True
mock_is_sata30orless.return_value = False
ret = utils.should_enable_discard(devices)
mock_os_path_exists.assert_called()
mock_is_sata30orless.assert_called()
self.assertEqual(ret, True)
@patch('os.path.exists')
@patch.object(utils, 'is_sata30orless')
def test_should_enable_discard_no(self, mock_is_sata30orless,
mock_os_path_exists):
devices = ['/dev/sda', '/dev/vda', '/dev/nvme0n1']
mock_os_path_exists.return_value = True
mock_is_sata30orless.return_value = True
ret = utils.should_enable_discard(devices)
mock_os_path_exists.assert_called()
mock_is_sata30orless.assert_called()
self.assertEqual(ret, False)
@patch('subprocess.check_output')
def test_is_sata30orless_sata31(self, mock_subprocess_check_output):
extcmd_output = (b'supressed text\nSATA Version is: '
b'SATA 3.1, 6.0 Gb/s (current: 6.0 Gb/s)\n'
b'supressed text\n\n')
mock_subprocess_check_output.return_value = extcmd_output
ret = utils.is_sata30orless('/dev/sda')
mock_subprocess_check_output.assert_called()
self.assertEqual(ret, False)
@patch('subprocess.check_output')
def test_is_sata30orless_sata30(self, mock_subprocess_check_output):
extcmd_output = (b'supressed text\nSATA Version is: '
b'SATA 3.0, 6.0 Gb/s (current: 6.0 Gb/s)\n'
b'supressed text\n\n')
mock_subprocess_check_output.return_value = extcmd_output
ret = utils.is_sata30orless('/dev/sda')
mock_subprocess_check_output.assert_called()
self.assertEqual(ret, True)
@patch('subprocess.check_output')
def test_is_sata30orless_sata26(self, mock_subprocess_check_output):
extcmd_output = (b'supressed text\nSATA Version is: '
b'SATA 2.6, 3.0 Gb/s (current: 3.0 Gb/s)\n'
b'supressed text\n\n')
mock_subprocess_check_output.return_value = extcmd_output
ret = utils.is_sata30orless('/dev/sda')
mock_subprocess_check_output.assert_called()
self.assertEqual(ret, True)