Bug 1260990 - heat updates raw_template when there is no change in raw_template
Summary: heat updates raw_template when there is no change in raw_template
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat OpenStack
Classification: Red Hat
Component: openstack-heat
Version: 6.0 (Juno)
Hardware: All
OS: Linux
urgent
high
Target Milestone: async
: 6.0 (Juno)
Assignee: Steve Baker
QA Contact: Amit Ugol
URL:
Whiteboard:
Depends On:
Blocks: 1264234
TreeView+ depends on / blocked
 
Reported: 2015-09-08 11:18 UTC by Jaison Raju
Modified: 2023-02-22 23:02 UTC (History)
14 users (show)

Fixed In Version: openstack-heat-2014.2.3-7.el7ost
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1264234 (view as bug list)
Environment:
Last Closed: 2015-10-15 12:30:38 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
raw_template_update patch (579 bytes, patch)
2015-09-10 05:38 UTC, Steve Baker
no flags Details | Diff
Make ResourceDefinition round-trip stable to avoid extra writes (2.99 KB, patch)
2015-09-11 06:24 UTC, Steve Baker
no flags Details | Diff
Log all calls to raw_template_update() (1019 bytes, patch)
2015-09-15 22:20 UTC, Zane Bitter
no flags Details | Diff
Work with copies of the DB contents (669 bytes, patch)
2015-09-15 22:38 UTC, Zane Bitter
no flags Details | Diff
Only write to files dict if template data has changed (1.05 KB, patch)
2015-09-17 04:24 UTC, Steve Baker
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Launchpad 1494108 0 None None None Never
OpenStack gerrit 222372 0 None None None Never
Red Hat Knowledge Base (Solution) 2114981 0 None None None 2016-01-07 07:57:39 UTC
Red Hat Product Errata RHBA-2015:1900 0 normal SHIPPED_LIVE openstack-heat bug fix advisory 2015-10-15 16:28:30 UTC

Description Jaison Raju 2015-09-08 11:18:19 UTC
Description of problem:
heat updates raw_template when there is no change in raw_template
Looking into the Heat code it looks that Heat should update the raw_template only where there is a change in the template (update stack) .
The UPDATEs does not seem to update anything - same values are used over and over again.
This lead to disk explosion in case of a large templates (100K-200K for a line) on Maria DB.

Version-Release number of selected component (if applicable):


How reproducible:
OSP5 RHEL 6

Steps to Reproduce:
1.
2.
3.

Actual results:
when there are no changes in raw_teplates , stack updates .

Expected results:


Additional info:

Its suspected that logic code is not functioning correctly and sending some extra updates.

This is from the code Heat that we suspect sending extra updaes:
-----------------------
def raw_template_update(context, template_id, values):
    raw_template_ref = raw_template_get(context, template_id)
    # get only the changed values
    values = dict((k, v) for k, v in values.items()
                  if getattr(raw_template_ref, k) != v)

    if values:
        raw_template_ref.update_and_save(values)

    return raw_template_ref
-----------------------

Reference Logs:
http://10.65.231.4/sysreports/1554147/01504448/
10-heat-api.log.gz  10-MariaDB.txt.gz  20-heat-engine.log.gz

Comment 6 Zane Bitter 2015-09-08 20:38:54 UTC
I looked through the MariaDB log and confirmed that it appears to be rewriting the raw_template table with the exact same template and files that it already contained (i.e. the SET and WHERE sections are identical except for the modification time). I'm not sure yet why this is happening - the code you pasted above is intended to check for this possibility and avoid unnecessary updates. The most likely reason is that the template deserialised from the DB compares differently to the one in memory, even though they both serialise to the same JSON representation - but I can't see how that could occur (the obvious one - unicode vs str keys and values - compares correctly). The way to check that would be to compare the serialised versions instead:

    # get only the changed values
    values = dict((k, v) for k, v in values.items()
                  if json.loads(getattr(raw_template_ref, k)) != json.loads(v))

Comment 10 Steve Baker 2015-09-10 03:23:22 UTC
I've reproduced locally and have filed an upstream bug.

Comment 11 Steve Baker 2015-09-10 05:31:07 UTC
Please ignore comment 10.

Comment 12 Steve Baker 2015-09-10 05:38:06 UTC
Created attachment 1072014 [details]
raw_template_update patch

I haven't reproduced locally yet, but can you please apply the attached patch then restart heat-engine and confirm if the updates are still occurring?

I do have a local unit test which confirms that this patch doesn't cause a regression, but it sounds like you've reproduced in a test environment anyway.

Comment 14 Jaison Raju 2015-09-10 09:06:17 UTC
Hi Steve ,

Also wanted to confirm if this patch applies for juno too ?
The file looks similar .

Regards,
Jaison R

Comment 19 Steve Baker 2015-09-11 06:24:53 UTC
Created attachment 1072461 [details]
Make ResourceDefinition round-trip stable to avoid extra writes

The part of a ResourceDefinition that lists explicit dependencies was not
round-trip stable. As a result, when we copied a new resource definition
into the existing template during a stack update, we would end up rewriting
the template unnecesarily (i.e. even though we check for changes) every
time if depends_on was not specified in the resource originally. At the end
of each update, we write the new template to the DB in its entirety, which
removes these extra lines again, ensuring that we will experience the same
problem on every update. This was causing a *lot* of unnecessary writes.

This change ensures that the definition remains stable across a round-trip,
so that no unnecessary changes appear in the template.

Comment 30 Zane Bitter 2015-09-15 22:20:11 UTC
Created attachment 1073870 [details]
Log all calls to raw_template_update()

I'm attaching a patch to log all calls to raw_template_update(). I still can't reproduce this issue locally, but if we try it on the system where we are experiencing the problem, this should either give us a good idea of what the cause is if it's in this function or it will rule out this function as the source.

Comment 31 Zane Bitter 2015-09-15 22:38:15 UTC
Created attachment 1073871 [details]
Work with copies of the DB contents

My current best guess is that the problem is *not* caused by raw_template_update(). Most likely we are not calling update_and_save() on the DB object, but rather making some innocuous-seeming change to the DB object itself and committing it as part of some other transaction. (I don't see the larger transaction in the MariaDB logs, but then I don't see *any* operations other than writes to the raw_template table in the logs - not even reads.)

In particular, I think we wrote the code with the assumption that the files dict is immutable, but in some cases (a TemplateResource where the the template is not available and we have to fetch it by URL) we do actually update that dict. The templates being used are full of TemplateResources.

I've attached a completely untested patch that ensures that the Template class works only with copies of the template data, and not the ones retrieved directly from the DB proxy object. Steve, please have a play around with this and check that it doesn't crash and burn horribly, and see if you can come up with a reproducer.

Comment 38 Steve Baker 2015-09-17 04:22:43 UTC
I believe I can make the following assertions about this bug now:
- an UPDATE raw_template is triggered on heat resource-list for templates with template resources (likely for other calls too)
- It is the files dict being modified which triggers the update (not the template dict)
- It is caused by template data being written to the dict whether it has changed or not [1], which causes an UPDATE because the Json type is backed by a MutableDict [2]
- This affects Juno and Kilo but not Liberty because we no longer use Mutable sqlalchemy types [3]

I think the appropriate course would be to fix [1] in Juno, Kilo and Liberty.

I will attach a WIP patch for Juno which solves the problem in my environment. 

[1] https://github.com/openstack/heat/blob/master/heat/engine/resources/template_resource.py#L206
[2] https://github.com/openstack/heat/blob/stable/juno/heat/db/sqlalchemy/types.py#L47
[3] https://github.com/openstack/heat/commit/27bdb8ce794bfc80b4d15043b2ff37bd2e52b332

Comment 39 Steve Baker 2015-09-17 04:24:23 UTC
Created attachment 1074247 [details]
Only write to files dict if template data has changed

Comment 48 Amit Ugol 2015-10-14 12:42:35 UTC
tested it better, thanks therve

Comment 50 errata-xmlrpc 2015-10-15 12:30:38 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2015-1900.html


Note You need to log in before you can comment on or make changes to this bug.