[IMP] jinja_to_qweb: store validation data in the DB

upg-2994884

Avoid `MemoryError` and/or killed process because of `malloc()` failing within
`lxml` / `libxml2`.

Debugging this by determining the size of involved datastructures through means
of `sys.getsizeof()` showed that the overly large memory consumption has 3
different sources:
1. During conversion: The global variable `templates_to_check` grows to
   hundreds of MiB after the various calls to `upgrade_jinja_fields()` by
   upgrade scripts.
2. During conversion: The call to `cr.dictfetchall()` to gather all
   templates(fields) that are to be converted, already consumes hundreds of
   MiB.
3. At the start of function `verify_upgraded_jinja_fields()`, the process is
   at ~1.5GiB because of (1) and (2). While iterating over all the templates in
   `templates_to_check`, no significant amount of memory is allocated on top of
   this *by python datastructures*. But, with each call to
   `is_converted_template_valid()`, the size of the process increases until it
   hits the RLIMIT. This function calls into `lxml` multiple times, suggesting
   that the memory is allocated in `malloc()` calls in the `lxml` and/or
   `libxml2` C library/ies, evading python's memory accounting and garbage
   collection.
   Internet research shows that `lxml` has a long history of different memory
   leaks in C code, plus some caching mechanism *across documents* that could
   be responsible[^1]. More recent versions of the module seem to have been
   improved, but still we're stuck with old versions.

This patch takes care of (1) by, instead of keeping the converted data in
memory for the validation step, creating a "temporary" table and inserting the
data there. After validation, the table is dropped.

[^1]: https://benbernardblog.com/tracking-down-a-freaky-python-memory-leak-part-2/

Part-of: #288
Signed-off-by: Christophe Simonis (chs) <[email protected]>

Author: cawo-odoo

src/util/jinja_to_qweb.py

@@ -57,8 +57,6 @@
     re.X | re.DOTALL,
 )
 
-templates_to_check = {}
-
 
 def _remove_safe(expression):
     return re.sub(REMOVE_SAFE_REGEX, " ", expression).strip()
@@ -226,6 +224,48 @@ def _replace_endif(string):
     return reg.sub(r"\1</t>", string)
 
 
+def setup_templates_to_check(cr):
+    if table_exists(cr, "_upgrade_jinja_to_qweb"):
+        return
+    cr.execute(
+        """
+        CREATE UNLOGGED TABLE _upgrade_jinja_to_qweb(
+            table_name          varchar,
+            template_type       varchar,
+            table_id            int4,
+            template_field      varchar,
+            model_name          varchar,
+            template_name       varchar,
+            template_name_field varchar,
+            template_converted  varchar,
+            PRIMARY KEY(table_name, template_type, table_id, template_field)
+        )
+        """
+    )
+
+
+def insert_templates_to_check(cr, *args):
+    cr.execute(
+        """
+        INSERT INTO _upgrade_jinja_to_qweb (
+            table_name,
+            template_type,
+            table_id,
+            template_field,
+            model_name,
+            template_name,
+            template_name_field,
+            template_converted
+        ) VALUES(%s, %s, %s, %s, %s, %s, %s, %s)
+        """,
+        args,
+    )
+
+
+def cleanup_templates_to_check(cr):
+    cr.execute("DROP TABLE _upgrade_jinja_to_qweb")
+
+
 def upgrade_jinja_fields(
     cr,
     table_name,
@@ -246,7 +286,7 @@ def upgrade_jinja_fields(
     sql_where_qweb_fields = [field + r"~ '(\$\{|%\s*(if|for))'" for field in qweb_fields]
     sql_where_fields = " OR ".join(sql_where_inline_fields + sql_where_qweb_fields)
 
-    templates_to_check[table_name] = []
+    setup_templates_to_check(cr)
     model = model_of_table(cr, table_name)
 
     cr.commit()  # ease the processing for PG
@@ -260,18 +300,44 @@ def upgrade_jinja_fields(
     for data in cr.dictfetchall():
         _logger.info("process %s(%s) %s", table_name, data["id"], data[name_field])
 
+        # only for mailing.mailing
+        if fetch_model_name:
+            cr.execute(
+                """
+                SELECT model FROM ir_model WHERE id=%s
+            """,
+                [data[table_model_name]],
+            )
+            model_name = cr.fetchone()[0]
+        else:
+            model_name = model_name or data[table_model_name]
+
         # convert the fields
         templates_converted = {}
-
         for field in inline_template_fields:
             _logger.info(" `- convert inline field %s", field)
-            template = data[field]
-            templates_converted[field] = convert_jinja_to_inline(template) if template else ""
+            converted = convert_jinja_to_inline(data[field]) if data[field] else ""
+            templates_converted[field] = converted
+            if data[field]:
+                insert_templates_to_check(
+                    cr,
+                    table_name,
+                    "inline_template",
+                    data["id"],
+                    field,
+                    model_name,
+                    data[name_field],
+                    name_field,
+                    converted,
+                )
 
         for field in qweb_fields:
             _logger.info(" `- convert qweb field %s", field)
-            template = data[field]
-            templates_converted[field] = convert_jinja_to_qweb(template) if template else ""
+            converted = convert_jinja_to_qweb(data[field]) if data[field] else ""
+            templates_converted[field] = converted
+            insert_templates_to_check(
+                cr, table_name, "qweb", data["id"], field, model_name, data[name_field], name_field, converted
+            )
 
         fields = [f for f in (inline_template_fields + qweb_fields) if data[f] != templates_converted[f]]
         if fields:
@@ -286,30 +352,6 @@ def upgrade_jinja_fields(
                 """,
                 field_values + [data["id"]],
             )
-        # prepare data to check later
-
-        # only for mailing.mailing
-        if fetch_model_name:
-            cr.execute(
-                """
-                SELECT model FROM ir_model WHERE id=%s
-            """,
-                [data[table_model_name]],
-            )
-            model_name = cr.fetchone()[0]
-        else:
-            model_name = model_name or data[table_model_name]
-
-        templates_to_check[table_name].append(
-            (
-                data,
-                name_field,
-                model_name,
-                inline_template_fields,
-                qweb_fields,
-                templates_converted,
-            )
-        )
 
     if not table_exists(cr, "ir_translation"):
         return
@@ -402,49 +444,58 @@ def upgrade_jinja_fields(
 
 def verify_upgraded_jinja_fields(cr):
     env = get_env(cr)
-    for table_name, template_data in templates_to_check.items():
+    cr.execute("SELECT DISTINCT(table_name) FROM _upgrade_jinja_to_qweb")
+    for (table_name,) in cr.fetchall():
         field_errors = {}
         missing_records = []
+        ncr = named_cursor(cr, 100)
+        ncr.execute(
+            """
+              SELECT template_type,
+                     table_id,
+                     template_field,
+                     model_name,
+                     template_name,
+                     template_name_field,
+                     template_converted
+                FROM _upgrade_jinja_to_qweb
+               WHERE table_name = %s
+            ORDER BY template_type,
+                     template_field
+            """,
+            [table_name],
+        )
         for (
-            data,
-            name_field,
+            template_type,
+            table_id,
+            template_field,
             model_name,
-            inline_template_fields,
-            qweb_fields,
-            templates_converted,
-        ) in template_data:
+            template_name,
+            template_name_field,
+            template_converted,
+        ) in ncr:
             if model_name not in env:
                 # custom model not loaded yet. Ignore
                 continue
             model = env[model_name]
             record = model.with_context({"active_test": False}).search([], limit=1, order="id")
 
-            key = (data["id"], data[name_field])
+            key = (table_id, template_name, template_name_field)
             field_errors[key] = []
 
             if not record:
                 missing_records.append(key)
 
-            for field in inline_template_fields:
-                if not data[field]:
-                    continue
-                is_valid = is_converted_template_valid(
-                    env, data[field], templates_converted[field], model_name, record.id, engine="inline_template"
-                )
-                if not is_valid:
-                    field_errors[key].append(field)
-
-            for field in qweb_fields:
-                is_valid = is_converted_template_valid(
-                    env, data[field], templates_converted[field], model_name, record.id, engine="qweb"
-                )
-                if not is_valid:
-                    field_errors[key].append(field)
+            is_valid = is_converted_template_valid(
+                env, template_field, template_converted, model_name, record.id, engine=template_type
+            )
+            if not is_valid:
+                field_errors[key].append(template_field)
 
         if missing_records:
             list_items = "\n".join(
                 f'<li>id: "{id}", {html_escape(name_field)}: "{html_escape(name)}" </li>'
-                for id, name in missing_records
+                for id, name, name_field in missing_records
             )
             add_to_migration_reports(
                 f"""
@@ -464,7 +515,7 @@ def verify_upgraded_jinja_fields(cr):
 
         if field_errors:
             string = []
-            for (id, name), fields in field_errors.items():
+            for (id, name, name_field), fields in field_errors.items():
                 fields_string = "\n".join(f"<li>{html_escape(field)}</li>" for field in fields)
                 string.append(
                     f"""<li>id: {id}, {html_escape(name_field)}: {html_escape(name)},
@@ -486,6 +537,8 @@ def verify_upgraded_jinja_fields(cr):
                 "Jinja upgrade",
                 format="html",
             )
+        ncr.close()
+    cleanup_templates_to_check(cr)
 
 
 def is_converted_template_valid(env, template_before, template_after, model_name, record_id, engine="inline_template"):