summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRenat Akhmerov <renat.akhmerov@gmail.com>2017-06-14 20:10:03 +0700
committerRenat Akhmerov <renat.akhmerov@gmail.com>2017-06-19 18:03:56 +0700
commitc15c378011702bde2e29922d238ed8b9b9688f39 (patch)
treed7bc8669fa0279110e550996ed316f94586ff063
parent58955acef686ae359e3bfe65428532df4f072460 (diff)
Make sure that the field "state_info" trimmed as expected
* Before this patch, "state_info" field of execution objects wasn't truncated properly before saving into DB which led to the DB error like: DBDataError: (_mysql_exceptions.DataError) (1406, "Data too long for column 'state_info' at row 1") It was happening because the method utils.cut() didn't work accurately enough in case if we passed it with a dictionary. This patch doesn't fix utils.cut() method but it just saves space for possible method result difference with the expected length so that we make sure that the truncated string representation is always less than 65536 bytes (i.e. the size of MySQL TEXT type). The total difference is not critical anyway. Closes-Bug: #1698125 Change-Id: I18449710ace6276224aaea564588c53a3e2c6adc (cherry picked from commit 5a43d54a058ea4e13924c596b0bee93b727a1f95)
Notes
Notes (review): Code-Review+2: Renat Akhmerov <renat.akhmerov@gmail.com> Workflow+1: Renat Akhmerov <renat.akhmerov@gmail.com> Verified+2: Jenkins Submitted-by: Jenkins Submitted-at: Mon, 19 Jun 2017 13:57:47 +0000 Reviewed-on: https://review.openstack.org/475316 Project: openstack/mistral Branch: refs/heads/stable/ocata
-rw-r--r--mistral/db/v2/sqlalchemy/models.py9
-rw-r--r--mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py4
-rw-r--r--mistral/tests/unit/engine/test_execution_fields_size_limitation.py68
-rw-r--r--mistral/utils/__init__.py21
4 files changed, 91 insertions, 11 deletions
diff --git a/mistral/db/v2/sqlalchemy/models.py b/mistral/db/v2/sqlalchemy/models.py
index 4b406f6..021842b 100644
--- a/mistral/db/v2/sqlalchemy/models.py
+++ b/mistral/db/v2/sqlalchemy/models.py
@@ -256,9 +256,16 @@ class TaskExecution(Execution):
256for cls in utils.iter_subclasses(Execution): 256for cls in utils.iter_subclasses(Execution):
257 event.listen( 257 event.listen(
258 # Catch and trim Execution.state_info to always fit allocated size. 258 # Catch and trim Execution.state_info to always fit allocated size.
259 # Note that the limit is 65500 which is less than 65535 (2^16 -1).
260 # The reason is that utils.cut() is not exactly accurate in case if
261 # the value is not a string, but, for example, a dictionary. If we
262 # limit it exactly to 65535 then once in a while it may go slightly
263 # beyond the allowed maximum size. It may depend on the order of
264 # keys in a string representation and other things that are hidden
265 # inside utils.cut_dict() method.
259 cls.state_info, 266 cls.state_info,
260 'set', 267 'set',
261 lambda t, v, o, i: utils.cut(v, 65532), 268 lambda t, v, o, i: utils.cut(v, 65500),
262 retval=True 269 retval=True
263 ) 270 )
264 271
diff --git a/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py b/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py
index 0605be4..190b2a5 100644
--- a/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py
+++ b/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py
@@ -1356,7 +1356,7 @@ class ActionExecutionTest(SQLAlchemyTest):
1356 self.assertEqual('FAILED', updated.state) 1356 self.assertEqual('FAILED', updated.state)
1357 state_info = db_api.load_action_execution(updated.id).state_info 1357 state_info = db_api.load_action_execution(updated.id).state_info
1358 self.assertEqual( 1358 self.assertEqual(
1359 65535, 1359 65503,
1360 len(state_info) 1360 len(state_info)
1361 ) 1361 )
1362 1362
@@ -1652,7 +1652,7 @@ class WorkflowExecutionTest(SQLAlchemyTest):
1652 self.assertEqual('FAILED', updated.state) 1652 self.assertEqual('FAILED', updated.state)
1653 state_info = db_api.load_workflow_execution(updated.id).state_info 1653 state_info = db_api.load_workflow_execution(updated.id).state_info
1654 self.assertEqual( 1654 self.assertEqual(
1655 65535, 1655 65503,
1656 len(state_info) 1656 len(state_info)
1657 ) 1657 )
1658 1658
diff --git a/mistral/tests/unit/engine/test_execution_fields_size_limitation.py b/mistral/tests/unit/engine/test_execution_fields_size_limitation.py
index 0b3512c..5573a71 100644
--- a/mistral/tests/unit/engine/test_execution_fields_size_limitation.py
+++ b/mistral/tests/unit/engine/test_execution_fields_size_limitation.py
@@ -15,6 +15,7 @@
15from oslo_config import cfg 15from oslo_config import cfg
16 16
17from mistral.actions import base as actions_base 17from mistral.actions import base as actions_base
18
18from mistral.db.v2 import api as db_api 19from mistral.db.v2 import api as db_api
19from mistral import exceptions as exc 20from mistral import exceptions as exc
20from mistral.services import workflows as wf_service 21from mistral.services import workflows as wf_service
@@ -36,27 +37,42 @@ wf:
36 input: 37 input:
37 - workflow_input: '__WORKFLOW_INPUT__' 38 - workflow_input: '__WORKFLOW_INPUT__'
38 - action_output_length: 0 39 - action_output_length: 0
40 - action_output_dict: false
41 - action_error: false
39 42
40 tasks: 43 tasks:
41 task1: 44 task1:
42 action: my_action 45 action: my_action
43 input: 46 input:
44 action_input: '__ACTION_INPUT__' 47 input: '__ACTION_INPUT__'
45 action_output_length: <% $.action_output_length %> 48 output_length: <% $.action_output_length %>
49 output_dict: <% $.action_output_dict %>
50 error: <% $.action_error %>
46 publish: 51 publish:
47 p_var: '__TASK_PUBLISHED__' 52 p_var: '__TASK_PUBLISHED__'
48""" 53"""
49 54
50 55
51class MyAction(actions_base.Action): 56class MyAction(actions_base.Action):
52 def __init__(self, action_input, action_output_length): 57 def __init__(self, input, output_length, output_dict=False, error=False):
53 self.action_input = action_input 58 self.input = input
54 self.action_output_length = action_output_length 59 self.output_length = output_length
60 self.output_dict = output_dict
61 self.error = error
55 62
56 def run(self): 63 def run(self):
57 return wf_utils.Result( 64 if not self.output_dict:
58 data=''.join('A' for _ in range(self.action_output_length)) 65 result = ''.join('A' for _ in range(self.output_length))
59 ) 66 else:
67 result = {}
68
69 for i in range(self.output_length):
70 result[i] = 'A'
71
72 if not self.error:
73 return wf_utils.Result(data=result)
74 else:
75 return wf_utils.Result(error=result)
60 76
61 def test(self): 77 def test(self):
62 raise NotImplementedError 78 raise NotImplementedError
@@ -229,3 +245,39 @@ class ExecutionFieldsSizeLimitTest(base.EngineTestCase):
229 "Size of 'params' is 1KB which exceeds the limit of 0KB", 245 "Size of 'params' is 1KB which exceeds the limit of 0KB",
230 e.message 246 e.message
231 ) 247 )
248
249 def test_task_execution_state_info_trimmed(self):
250 # No limit on output, input and other JSON fields.
251 cfg.CONF.set_default(
252 'execution_field_size_limit_kb',
253 -1,
254 group='engine'
255 )
256
257 wf_service.create_workflows(WF)
258
259 # Start workflow.
260 wf_ex = self.engine.start_workflow(
261 'wf',
262 {
263 'action_output_length': 80000,
264 'action_output_dict': True,
265 'action_error': True
266 }
267 )
268
269 self.await_workflow_error(wf_ex.id)
270
271 with db_api.transaction():
272 wf_ex = db_api.get_workflow_execution(wf_ex.id)
273
274 task_ex = self._assert_single_item(
275 wf_ex.task_executions,
276 state=states.ERROR
277 )
278
279 # "state_info" must be trimmed so that it's not greater than 65535.
280 self.assertLess(len(task_ex.state_info), 65536)
281 self.assertGreater(len(task_ex.state_info), 65490)
282 self.assertLess(len(wf_ex.state_info), 65536)
283 self.assertGreater(len(wf_ex.state_info), 65490)
diff --git a/mistral/utils/__init__.py b/mistral/utils/__init__.py
index 2f66f77..561d4a6 100644
--- a/mistral/utils/__init__.py
+++ b/mistral/utils/__init__.py
@@ -185,6 +185,27 @@ def get_file_list(directory):
185 185
186 186
187def cut_dict(d, length=100): 187def cut_dict(d, length=100):
188 """Removes dictionary entries according to the given length.
189
190 This method removes a number of entries, if needed, so that a
191 string representation would fit into the given length.
192 The intention of this method is to optimize truncation of string
193 representation for dictionaries where the exact precision is not
194 critically important. Otherwise, we'd always have to convert a dict
195 into a string first and then shrink it to a needed size which will
196 increase memory footprint and reduce performance in case of large
197 dictionaries (i.e. tens of thousands entries).
198 Note that the method, due to complexity of the algorithm, has some
199 non-zero precision which depends on exact keys and values placed into
200 the dict. So for some dicts their reduced string representations will
201 be only approximately equal to the given value (up to around several
202 chars difference).
203
204 :param d: A dictionary.
205 :param length: A length limiting the dictionary string representation.
206 :return: A dictionary which is a subset of the given dictionary.
207 """
208
188 if not isinstance(d, dict): 209 if not isinstance(d, dict):
189 raise ValueError("A dictionary is expected, got: %s" % type(d)) 210 raise ValueError("A dictionary is expected, got: %s" % type(d))
190 211