From d37db6fb512827c48b74de8ac7f508c1edea382c Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Dec 2019 11:50:24 +0100 Subject: [PATCH 1/8] Try using mongoengine work which reduces some unecessary and expensive serialization and derserialization operations. --- fixed-requirements.txt | 2 +- st2common/st2common/fields.py | 13 ++++++++++--- st2common/st2common/models/db/__init__.py | 7 ++++--- st2common/st2common/models/db/stormbase.py | 6 ++---- st2common/tests/unit/test_db_fields.py | 2 +- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/fixed-requirements.txt b/fixed-requirements.txt index 3ac7f05d29..b399144ad7 100644 --- a/fixed-requirements.txt +++ b/fixed-requirements.txt @@ -18,8 +18,8 @@ requests[security]==2.22.0 apscheduler==3.6.3 gitpython==2.1.11 jsonschema==2.6.0 +git+https://github.com/closeio/mongoengine.git@master#egg=mongoengine pymongo==3.10.0 -mongoengine==0.18.2 passlib==1.7.1 lockfile==0.12.2 python-gnupg==0.4.5 diff --git a/st2common/st2common/fields.py b/st2common/st2common/fields.py index cd741bf6fd..8c23e438af 100644 --- a/st2common/st2common/fields.py +++ b/st2common/st2common/fields.py @@ -16,7 +16,7 @@ import datetime import calendar -from mongoengine import LongField +from mongoengine import IntField from st2common.util import date as date_utils @@ -27,7 +27,7 @@ SECOND_TO_MICROSECONDS = 1000000 -class ComplexDateTimeField(LongField): +class ComplexDateTimeField(IntField): """ Date time field which handles microseconds exactly and internally stores the timestamp as number of microseconds since the unix epoch. @@ -92,6 +92,9 @@ def __get__(self, instance, owner): return self._convert_from_db(data) def __set__(self, instance, value): + #if isinstance(value, int): + # value = self.to_python(value) + value = self._convert_from_datetime(value) if value else value return super(ComplexDateTimeField, self).__set__(instance, value) @@ -109,8 +112,12 @@ def to_python(self, value): return original_value def to_mongo(self, value): + if value is None: + return value value = self.to_python(value) return self._convert_from_datetime(value) def prepare_query_value(self, op, value): - return self._convert_from_datetime(value) + if isinstance(value, datetime.datetime): + return self._convert_from_datetime(value) + return value diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index 098a8e45da..fd4456e583 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -236,9 +236,10 @@ def cleanup_extra_indexes(model_class): """ Finds any extra indexes and removes those from mongodb. """ - extra_indexes = model_class.compare_indexes().get('extra', None) - if not extra_indexes: - return 0 + return 0 + #extra_indexes = model_class.compare_indexes().get('extra', None) + #if not extra_indexes: + # return 0 # mongoengine does not have the necessary method so we need to drop to # pymongo interfaces via some private methods. diff --git a/st2common/st2common/models/db/stormbase.py b/st2common/st2common/models/db/stormbase.py index a9eb755867..bd9d68f651 100644 --- a/st2common/st2common/models/db/stormbase.py +++ b/st2common/st2common/models/db/stormbase.py @@ -123,8 +123,7 @@ class EscapedDictField(me.DictField): def to_mongo(self, value, use_db_field=True, fields=None): value = mongoescape.escape_chars(value) - return super(EscapedDictField, self).to_mongo(value=value, use_db_field=use_db_field, - fields=fields) + return super(EscapedDictField, self).to_mongo(value) def to_python(self, value): value = super(EscapedDictField, self).to_python(value) @@ -142,8 +141,7 @@ class EscapedDynamicField(me.DynamicField): def to_mongo(self, value, use_db_field=True, fields=None): value = mongoescape.escape_chars(value) - return super(EscapedDynamicField, self).to_mongo(value=value, use_db_field=use_db_field, - fields=fields) + return super(EscapedDynamicField, self).to_mongo(value) def to_python(self, value): value = super(EscapedDynamicField, self).to_python(value) diff --git a/st2common/tests/unit/test_db_fields.py b/st2common/tests/unit/test_db_fields.py index 4151da4a2b..9329f85860 100644 --- a/st2common/tests/unit/test_db_fields.py +++ b/st2common/tests/unit/test_db_fields.py @@ -68,7 +68,7 @@ def test_round_trip_conversion(self): expected_value = datetime_values[index] self.assertEqual(actual_value, expected_value) - @mock.patch('st2common.fields.LongField.__get__') + @mock.patch('st2common.fields.IntField.__get__') def test_get_(self, mock_get): field = ComplexDateTimeField() From afb03aeccce9f902504419804276da0e0401652f Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Dec 2019 12:13:25 +0100 Subject: [PATCH 2/8] Update more affected code. --- st2actions/tests/unit/test_executions.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/st2actions/tests/unit/test_executions.py b/st2actions/tests/unit/test_executions.py index b896378729..b022c6687e 100644 --- a/st2actions/tests/unit/test_executions.py +++ b/st2actions/tests/unit/test_executions.py @@ -100,7 +100,7 @@ def test_basic_execution(self): self.assertEqual(execution.result, liveaction.result) self.assertEqual(execution.status, liveaction.status) self.assertEqual(execution.context, liveaction.context) - self.assertEqual(execution.liveaction['callback'], liveaction.callback) + self.assertEqual(execution.liveaction.get('callback', {}), liveaction.callback) self.assertEqual(execution.liveaction['action'], liveaction.action) def test_basic_execution_history_create_failed(self): @@ -127,7 +127,7 @@ def test_chained_executions(self): self.assertEqual(execution.result, liveaction.result) self.assertEqual(execution.status, liveaction.status) self.assertEqual(execution.context, liveaction.context) - self.assertEqual(execution.liveaction['callback'], liveaction.callback) + self.assertEqual(execution.liveaction.get('callback', {}), liveaction.callback) self.assertEqual(execution.liveaction['action'], liveaction.action) self.assertGreater(len(execution.children), 0) @@ -184,7 +184,7 @@ def test_triggered_execution(self): self.assertEqual(execution.result, liveaction.result) self.assertEqual(execution.status, liveaction.status) self.assertEqual(execution.context, liveaction.context) - self.assertEqual(execution.liveaction['callback'], liveaction.callback) + self.assertEqual(execution.liveaction.get('callback', {}), liveaction.callback) self.assertEqual(execution.liveaction['action'], liveaction.action) def _get_action_execution(self, **kwargs): From c2031f3ce070c0f1e1fb057bc50058d9bcfd1a38 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Dec 2019 12:16:02 +0100 Subject: [PATCH 3/8] Use our fork with some fixes. --- fixed-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fixed-requirements.txt b/fixed-requirements.txt index b399144ad7..5f3d9a9b1a 100644 --- a/fixed-requirements.txt +++ b/fixed-requirements.txt @@ -18,7 +18,7 @@ requests[security]==2.22.0 apscheduler==3.6.3 gitpython==2.1.11 jsonschema==2.6.0 -git+https://github.com/closeio/mongoengine.git@master#egg=mongoengine +git+https://github.com/StackStorm/mongoengine.git@master#egg=st2_patched_mallard pymongo==3.10.0 passlib==1.7.1 lockfile==0.12.2 From 0913c56c5a8eff78ec1560d002a40ca98f62fc04 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Dec 2019 12:19:32 +0100 Subject: [PATCH 4/8] Use a valid key. --- st2actions/tests/unit/policies/test_retry_policy.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/st2actions/tests/unit/policies/test_retry_policy.py b/st2actions/tests/unit/policies/test_retry_policy.py index 6de6c13efe..f28dea7cb1 100644 --- a/st2actions/tests/unit/policies/test_retry_policy.py +++ b/st2actions/tests/unit/policies/test_retry_policy.py @@ -13,7 +13,9 @@ # limitations under the License. from __future__ import absolute_import + import mock +from bson import ObjectId import st2actions from st2common.constants.action import LIVEACTION_STATUS_REQUESTED @@ -258,7 +260,7 @@ def test_no_retry_on_workflow_task(self): live_action_db = LiveActionDB( action='wolfpack.action-1', parameters={'actionstr': 'foo'}, - context={'parent': {'execution_id': 'abcde'}} + context={'parent': {'execution_id': ObjectId('5609e91832ed356d04a93cc0')}} ) live_action_db, execution_db = action_service.request(live_action_db) From 511f4f9a7bc61795a4d29b244da959e6efb8b377 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Dec 2019 12:28:13 +0100 Subject: [PATCH 5/8] Revert "Use our fork with some fixes." This reverts commit c2031f3ce070c0f1e1fb057bc50058d9bcfd1a38. --- fixed-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fixed-requirements.txt b/fixed-requirements.txt index 5f3d9a9b1a..b399144ad7 100644 --- a/fixed-requirements.txt +++ b/fixed-requirements.txt @@ -18,7 +18,7 @@ requests[security]==2.22.0 apscheduler==3.6.3 gitpython==2.1.11 jsonschema==2.6.0 -git+https://github.com/StackStorm/mongoengine.git@master#egg=st2_patched_mallard +git+https://github.com/closeio/mongoengine.git@master#egg=mongoengine pymongo==3.10.0 passlib==1.7.1 lockfile==0.12.2 From fe6411aea61ca54781b3bb718ca141b4b54694c6 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Dec 2019 12:31:23 +0100 Subject: [PATCH 6/8] Use better approach for updating an existing document. --- st2common/st2common/bootstrap/actionsregistrar.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/st2common/st2common/bootstrap/actionsregistrar.py b/st2common/st2common/bootstrap/actionsregistrar.py index 074f48b7cf..3e44be64df 100644 --- a/st2common/st2common/bootstrap/actionsregistrar.py +++ b/st2common/st2common/bootstrap/actionsregistrar.py @@ -176,7 +176,10 @@ def _register_action(self, pack, action): else: LOG.debug('Action %s found. Will be updated from: %s to: %s', action_ref, existing, model) + # NOTE: _lazy is needed because of mongo mallard changes to amke sure we update an + # existing object which doesn't have _db_data attribute populated model.id = existing.id + model._lazy = True try: model = Action.add_or_update(model) From 42b28ad1738dbe8c2d960a2ace01784e19ee0761 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Dec 2019 12:36:25 +0100 Subject: [PATCH 7/8] Correctly indicate to mongo mallard that we are updating and existing document and not inserting a new one. --- st2api/st2api/controllers/v1/actionalias.py | 2 ++ st2api/st2api/controllers/v1/actions.py | 2 ++ st2api/st2api/controllers/v1/auth.py | 2 ++ st2api/st2api/controllers/v1/keyvalue.py | 2 ++ st2api/st2api/controllers/v1/policies.py | 2 ++ st2api/st2api/controllers/v1/rules.py | 2 ++ st2api/st2api/controllers/v1/sensors.py | 2 ++ st2api/st2api/controllers/v1/triggers.py | 4 ++++ 8 files changed, 18 insertions(+) diff --git a/st2api/st2api/controllers/v1/actionalias.py b/st2api/st2api/controllers/v1/actionalias.py index 812c666b42..d07b4e298a 100644 --- a/st2api/st2api/controllers/v1/actionalias.py +++ b/st2api/st2api/controllers/v1/actionalias.py @@ -166,6 +166,8 @@ def put(self, action_alias, ref_or_id, requester_user): old_action_alias_db = action_alias_db action_alias_db = ActionAliasAPI.to_model(action_alias) action_alias_db.id = ref_or_id + # mongo mallard way of saying that we are updating an existing document + action_alias_db._lazy = True action_alias_db = ActionAlias.add_or_update(action_alias_db) except (ValidationError, ValueError) as e: LOG.exception('Validation failed for action alias data=%s', action_alias) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 792d456829..005264498e 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -171,6 +171,8 @@ def put(self, action, ref_or_id, requester_user): action_db = ActionAPI.to_model(action) LOG.debug('/actions/ PUT incoming action: %s', action_db) action_db.id = action_id + # mongo mallard way of saying that we are updating an existing document + action_db._lazy = True action_db = Action.add_or_update(action_db) LOG.debug('/actions/ PUT after add_or_update: %s', action_db) except (ValidationError, ValueError) as e: diff --git a/st2api/st2api/controllers/v1/auth.py b/st2api/st2api/controllers/v1/auth.py index 9cf3b1dd6f..91269b9fe1 100644 --- a/st2api/st2api/controllers/v1/auth.py +++ b/st2api/st2api/controllers/v1/auth.py @@ -205,6 +205,8 @@ def put(self, api_key_api, api_key_id_or_key, requester_user): raise ValueError('Update of key_hash is not allowed.') api_key_db.id = old_api_key_db.id + # mongo mallard way of saying that we are updating an existing document + api_key_db._lazy = True api_key_db = ApiKey.add_or_update(api_key_db) extra = {'old_api_key_db': old_api_key_db, 'new_api_key_db': api_key_db} diff --git a/st2api/st2api/controllers/v1/keyvalue.py b/st2api/st2api/controllers/v1/keyvalue.py index 5620da86a9..186422d68a 100644 --- a/st2api/st2api/controllers/v1/keyvalue.py +++ b/st2api/st2api/controllers/v1/keyvalue.py @@ -310,6 +310,8 @@ def put(self, kvp, name, requester_user, scope=FULL_SYSTEM_SCOPE): if existing_kvp_api: kvp_db.id = existing_kvp_api.id + # mongo mallard way of saying that we are updating an existing document + kvp_db._lazy = True kvp_db = KeyValuePair.add_or_update(kvp_db) except (ValidationError, ValueError) as e: diff --git a/st2api/st2api/controllers/v1/policies.py b/st2api/st2api/controllers/v1/policies.py index 7dd50ecf45..a215a25805 100644 --- a/st2api/st2api/controllers/v1/policies.py +++ b/st2api/st2api/controllers/v1/policies.py @@ -198,6 +198,8 @@ def put(self, instance, ref_or_id, requester_user): try: db_model = self.model.to_model(instance) db_model.id = db_model_id + # mongo mallard way of saying that we are updating an existing document + db_model._lazy = True db_model = self.access.add_or_update(db_model) except (ValidationError, ValueError) as e: LOG.exception('%s unable to update object: %s', op, db_model) diff --git a/st2api/st2api/controllers/v1/rules.py b/st2api/st2api/controllers/v1/rules.py index 4921b7b279..1b92294da4 100644 --- a/st2api/st2api/controllers/v1/rules.py +++ b/st2api/st2api/controllers/v1/rules.py @@ -196,6 +196,8 @@ def put(self, rule, rule_ref_or_id, requester_user): rule_api=rule) rule_db.id = rule_ref_or_id + # mongo mallard way of saying that we are updating an existing document + rule_db._lazy = True rule_db = Rule.add_or_update(rule_db) # After the rule has been added modify the ref_count. This way a failure to add # the rule due to violated constraints will have no impact on ref_count. diff --git a/st2api/st2api/controllers/v1/sensors.py b/st2api/st2api/controllers/v1/sensors.py index 456fa8e270..b94df3d2d3 100644 --- a/st2api/st2api/controllers/v1/sensors.py +++ b/st2api/st2api/controllers/v1/sensors.py @@ -92,6 +92,8 @@ def put(self, sensor_type, ref_or_id, requester_user): try: old_sensor_type_db = sensor_type_db sensor_type_db.id = sensor_type_id + # mongo mallard way of saying that we are updating an existing document + sensor_type_db._lazy = True sensor_type_db.enabled = getattr(sensor_type, 'enabled', False) sensor_type_db = SensorType.add_or_update(sensor_type_db) except (ValidationError, ValueError) as e: diff --git a/st2api/st2api/controllers/v1/triggers.py b/st2api/st2api/controllers/v1/triggers.py index 23bc527b58..9b11871d79 100644 --- a/st2api/st2api/controllers/v1/triggers.py +++ b/st2api/st2api/controllers/v1/triggers.py @@ -110,6 +110,8 @@ def put(self, triggertype, triggertype_ref_or_id): LOG.warning('Discarding mismatched id=%s found in payload and using uri_id=%s.', triggertype.id, triggertype_id) triggertype_db.id = triggertype_id + # mongo mallard way of saying that we are updating an existing document + triggertype_db._lazy = True old_triggertype_db = triggertype_db triggertype_db = TriggerType.add_or_update(triggertype_db) except (ValidationError, ValueError) as e: @@ -254,6 +256,8 @@ def put(self, trigger, trigger_id): trigger.id, trigger_id) trigger_db = TriggerAPI.to_model(trigger) trigger_db.id = trigger_id + # mongo mallard way of saying that we are updating an existing document + trigger_db._lazy = True trigger_db = Trigger.add_or_update(trigger_db) except (ValidationError, ValueError) as e: LOG.exception('Validation failed for trigger data=%s', trigger) From 359a43a8572a75c36ceb81eb767c6ffbb72ea438 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Dec 2019 12:41:22 +0100 Subject: [PATCH 8/8] More mongo mallard compatibility changes. --- st2api/st2api/controllers/v1/actions.py | 9 ++++++--- st2common/st2common/bootstrap/aliasesregistrar.py | 1 + st2common/st2common/models/db/execution.py | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 005264498e..6807fb62f5 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -290,9 +290,12 @@ def _update_pack_model(self, pack_ref, data_files, written_file_paths): file_paths.append(file_path) pack_db = Pack.get_by_ref(pack_ref) - pack_db.files = set(pack_db.files) - pack_db.files.update(set(file_paths)) - pack_db.files = list(pack_db.files) + + pack_files = set(pack_db.files) + pack_files.update(set(file_paths)) + pack_files = list(pack_files) + + pack_db.files = pack_files pack_db = Pack.add_or_update(pack_db) return pack_db diff --git a/st2common/st2common/bootstrap/aliasesregistrar.py b/st2common/st2common/bootstrap/aliasesregistrar.py index 082834bc4c..86c28df397 100644 --- a/st2common/st2common/bootstrap/aliasesregistrar.py +++ b/st2common/st2common/bootstrap/aliasesregistrar.py @@ -146,6 +146,7 @@ def _register_action_alias(self, pack, action_alias): try: action_alias_db.id = ActionAlias.get_by_name(action_alias_db.name).id + action_alias._lazy = True except StackStormDBObjectNotFoundError: LOG.debug('ActionAlias %s not found. Creating new one.', action_alias) diff --git a/st2common/st2common/models/db/execution.py b/st2common/st2common/models/db/execution.py index b73d275246..3ce93ebcec 100644 --- a/st2common/st2common/models/db/execution.py +++ b/st2common/st2common/models/db/execution.py @@ -103,7 +103,7 @@ def get_uid(self): def mask_secrets(self, value): result = copy.deepcopy(value) - liveaction = result['liveaction'] + liveaction = result.get('liveaction', {}) parameters = {} # pylint: disable=no-member parameters.update(value.get('action', {}).get('parameters', {}))