Merge lp:~james-page/charm-helpers/misc-percona-changes into lp:charm-helpers

Proposed by James Page
Status: Merged
Merged at revision: 697
Proposed branch: lp:~james-page/charm-helpers/misc-percona-changes
Merge into: lp:charm-helpers
Diff against target: 134 lines (+38/-14)
3 files modified
charmhelpers/contrib/database/mysql.py (+9/-2)
test_requirements.txt (+1/-0)
tests/contrib/database/test_mysql.py (+28/-12)
To merge this branch: bzr merge lp:~james-page/charm-helpers/misc-percona-changes
Reviewer Review Type Date Requested Status
Ryan Beisner (community) Approve
Jorge Niedbalski (community) Approve
Review via email: mp+318253@code.launchpad.net

Description of the change

Update default unset memory configuration behaviour and fixup unit tests.

To post a comment you must log in.
698. By James Page

Amend NOTE

Revision history for this message
James Page (james-page) wrote :

This review also fixes up the mysql tests to work under py2 and py3

Revision history for this message
Jorge Niedbalski (niedbalski) wrote :

James,

Thanks for this addition. I agree with your rationale on this change.

Regarding tests, all the unit tests are passing OK.

LGTM

review: Approve
Revision history for this message
Ryan Beisner (1chb1n) wrote :

Also good with this, many thanks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/database/mysql.py'
2--- charmhelpers/contrib/database/mysql.py 2017-02-16 03:04:17 +0000
3+++ charmhelpers/contrib/database/mysql.py 2017-02-28 12:04:55 +0000
4@@ -334,6 +334,7 @@
5
6 DEFAULT_PAGE_SIZE = 16 * 1024 * 1024
7 DEFAULT_INNODB_BUFFER_FACTOR = 0.50
8+ DEFAULT_INNODB_BUFFER_SIZE_MAX = 512 * 1024 * 1024
9
10 def human_to_bytes(self, human):
11 """Convert human readable configuration options to bytes."""
12@@ -416,8 +417,14 @@
13 innodb_buffer_pool_size = self.human_to_bytes(
14 dataset_bytes)
15 else:
16- innodb_buffer_pool_size = int(
17- total_memory * self.DEFAULT_INNODB_BUFFER_FACTOR)
18+ # NOTE(jamespage): pick the smallest of 50% of RAM or 512MB
19+ # to ensure that deployments in containers
20+ # without constraints don't try to consume
21+ # silly amounts of memory.
22+ innodb_buffer_pool_size = min(
23+ int(total_memory * self.DEFAULT_INNODB_BUFFER_FACTOR),
24+ self.DEFAULT_INNODB_BUFFER_SIZE_MAX
25+ )
26
27 if innodb_buffer_pool_size > total_memory:
28 log("innodb_buffer_pool_size; {} is greater than system available memory:{}".format(
29
30=== modified file 'test_requirements.txt'
31--- test_requirements.txt 2017-01-25 17:11:37 +0000
32+++ test_requirements.txt 2017-02-28 12:04:55 +0000
33@@ -17,3 +17,4 @@
34 netifaces==0.10 # trusty is 0.8, but using py3 compatible version for tests.
35 Jinja2==2.6 # precise
36 psutil==1.2.1 # trusty
37+dnspython==1.11.1 # trusty
38
39=== added file 'tests/contrib/database/__init__.py'
40=== modified file 'tests/contrib/database/test_mysql.py'
41--- tests/contrib/database/test_mysql.py 2015-06-03 20:20:53 +0000
42+++ tests/contrib/database/test_mysql.py 2017-02-28 12:04:55 +0000
43@@ -50,23 +50,20 @@
44 units = helper.get_allowed_units('dbA', 'userA')
45
46 calls = [mock.call('dbA', 'userA', 'hostA'),
47- mock.call().__nonzero__(),
48 mock.call('dbA', 'userA', '10.0.0.2'),
49- mock.call().__nonzero__(),
50 mock.call('dbA', 'userA', '2001:db8:1::2'),
51- mock.call().__nonzero__(),
52- mock.call('dbA', 'userA', '10.0.0.3'),
53- mock.call().__nonzero__()]
54+ mock.call('dbA', 'userA', '10.0.0.3')]
55
56- helper.grant_exists.assert_has_calls(calls)
57+ helper.grant_exists.assert_has_calls(calls, any_order=True)
58 self.assertEqual(units, set(['unit/0', 'unit/1', 'unit/2']))
59
60+ @mock.patch('charmhelpers.contrib.network.ip.ns_query')
61 @mock.patch('charmhelpers.contrib.network.ip.socket')
62 @mock.patch.object(mysql, 'unit_get')
63 @mock.patch.object(mysql, 'config_get')
64 @mock.patch.object(mysql, 'log')
65 def test_normalize_address(self, mock_log, mock_config_get, mock_unit_get,
66- mock_socket):
67+ mock_socket, mock_ns_query):
68 helper = mysql.MySQLHelper('foo', 'bar', host='hostA')
69 # prefer-ipv6
70 mock_config_get.return_value = False
71@@ -88,6 +85,7 @@
72 mock_config_get.assert_called_with('prefer-ipv6')
73
74 mock_socket.gethostbyname.side_effect = Exception
75+ mock_ns_query.return_value = None
76 out = helper.normalize_address('unresolvable')
77 self.assertEqual('unresolvable', out)
78 mock_config_get.assert_called_with('prefer-ipv6')
79@@ -187,7 +185,8 @@
80 mock.call('userA.passwd', '2'),
81 mock.call('mysql-userA.passwd', '3')]
82
83- mock_peer_store.assert_has_calls(calls)
84+ mock_peer_store.assert_has_calls(calls,
85+ any_order=True)
86 finally:
87 os.rmdir(tmpdir)
88
89@@ -231,8 +230,7 @@
90
91 self.assertEqual(
92 mysql_config.get('innodb_buffer_pool_size'),
93- int(helper.human_to_bytes(mem.return_value) *
94- helper.DEFAULT_INNODB_BUFFER_FACTOR))
95+ helper.DEFAULT_INNODB_BUFFER_SIZE_MAX)
96
97 @mock.patch.object(mysql.PerconaClusterHelper, 'get_mem_total')
98 @mock.patch.object(mysql, 'config_get')
99@@ -248,7 +246,25 @@
100 mysql_config = helper.parse_config()
101
102 self.assertEqual(
103- mysql_config.get('innodb-buffer-pool-size'),
104+ mysql_config.get('innodb_buffer_pool_size'),
105+ helper.DEFAULT_INNODB_BUFFER_SIZE_MAX)
106+
107+ @mock.patch.object(mysql.PerconaClusterHelper, 'get_mem_total')
108+ @mock.patch.object(mysql, 'config_get')
109+ @mock.patch.object(mysql, 'log')
110+ def test_parse_config_innodb_buffer_unset_small(self, mog,
111+ config, mem):
112+ mem.return_value = "512M"
113+ config.return_value = {
114+ 'innodb-buffer-pool-size': None,
115+ 'dataset-size': None,
116+ }
117+
118+ helper = mysql.PerconaClusterHelper()
119+ mysql_config = helper.parse_config()
120+
121+ self.assertEqual(
122+ mysql_config.get('innodb_buffer_pool_size'),
123 int(helper.human_to_bytes(mem.return_value) *
124 helper.DEFAULT_INNODB_BUFFER_FACTOR))
125
126@@ -265,7 +281,7 @@
127 mysql_config = helper.parse_config()
128
129 self.assertEqual(
130- mysql_config.get('innodb-buffer-pool-size'),
131+ mysql_config.get('innodb_buffer_pool_size'),
132 int(helper.human_to_bytes("10G")))
133
134 @mock.patch.object(mysql.PerconaClusterHelper, 'get_mem_total')

Subscribers

People subscribed via source and target branches