dotCMS / core

Headless/Hybrid Content Management System for Enterprises
http://dotcms.com
Other
862 stars 468 forks source link

Hibernate forces unnecessary updates for fields with null values #4298

Closed wezell closed 11 years ago

wezell commented 11 years ago

We have multiple places in our code that forces hibernate to update after selecting. We have seen this behavior on multiple tables: inode, fields, containers. This behavior was locking during push publishing while a site search was running on an Oracle db. I can reproduce the behavior on MySQL - on the "containers.notes" field. I think we need to check all of or models:

  1. update your db (starter) and set containers.notes = null:
update containers set notes=null, pre_loop=null, post_loop=null, lucene_query=null, sort_contentlets_by=null, for_metadata=null where inode ='97b67789-453c-487b-858c-82c9e09623bd';
  1. do a select and verify that everything is right:
select containers.inode, notes, pre_loop, post_loop, lucene_query, sort_contentlets_by, for_metadata from containers where inode ='97b67789-453c-487b-858c-82c9e09623bd';

+--------------------------------------+-------+----------+-----------+--------------+---------------------+--------------+
| inode                                | notes | pre_loop | post_loop | lucene_query | sort_contentlets_by | for_metadata |
+--------------------------------------+-------+----------+-----------+--------------+---------------------+--------------+
| 97b67789-453c-487b-858c-82c9e09623bd | NULL  | NULL     | NULL      | NULL         | NULL                |            0 |
+--------------------------------------+-------+----------+-----------+--------------+---------------------+--------------+
  1. run this in a .jsp - it is just a select:
<%@page import="com.dotmarketing.business.APILocator"%>
<%@page import="com.dotmarketing.portlets.containers.model.Container"%>
<%@page import="com.dotmarketing.db.HibernateUtil"%>
<%
Container c = APILocator.getContainerAPI().getWorkingContainerById("56bd55ea-b04b-480d-9e37-5d6f9217dcc3", APILocator.getUserAPI().getSystemUser(), false);
%>
<%=c.getTitle() %> : <%=c.getInode() %>
  1. Rerun the select above and you get:
mysql> select containers.inode, notes, pre_loop, post_loop, lucene_query, sort_contentlets_by, for_metadata from containers where inode ='97b67789-453c-487b-858c-82c9e09623bd';

+--------------------------------------+-------+----------+-----------+--------------+---------------------+--------------+
| inode                                | notes | pre_loop | post_loop | lucene_query | sort_contentlets_by | for_metadata |
+--------------------------------------+-------+----------+-----------+--------------+---------------------+--------------+
| 97b67789-453c-487b-858c-82c9e09623bd |       | NULL     | NULL      | NULL         | NULL                |            0 |
+--------------------------------------+-------+----------+-----------+--------------+---------------------+--------------+
jtesser commented 11 years ago

@dotjorgeu as we discussed checkout WIll's branch and solve as we discussed

dotjorgeu commented 11 years ago

those are the classes / methods that might return something different to the value got from hibernate and in consequence forcing an update

Inode.      getInode   getIdentifier
UserProxy.  setMailSubscription setNoclicktracking setLastMessage setLastResult setUserId setPrefix setSuffix setTitle setSchool setGraduation_year setCompany setHowHeard setLongLivedCookie setWebsite setGraduationYear setOrganization setVar1..setVar25
WebAsset. setFriendlyName getOwner 
Folder. getInode(stupid) 
Field. getValues 
Structure. getFolder getHost
VirtualLink. getInode 
Container. getInode
Contentlet. getInode 
File. getInode
HTMLPage. getInode
Link. getInode getUrl 
Template. getInode
WorkflowMessage. getInode 
Tree. getParent getChild
Language. setCountryCode setLanguageCode
ClickStream. setUserAgent
ClickStreamRequest. setRequestURI
ClickStream404. setRequestURI setRefererURI setQueryString
Rating. setRating
Identifier. getId setId 

In most cases the method getInode isn't a big deal.

dotjorgeu commented 11 years ago

ran timemachine and the only update operations are permission_reference inserts. no update sql

jtesser commented 11 years ago

@DeanGonzalez @bryanboza @prasadbhvn

This needs to be tested really well. I am thinking a few days or so. What I can do is describe what we generally changed. We altered the way new objects get created for Links, Structure Fields, and Containers. Also effected is the owner of all objects.

In general we need to edit a bunch of objects. Create new ones. Edit old ones.

For containers leave no notes, add notes, edit notes, Remove notes. But we need a good vetting through of the UI

bryanboza commented 9 years ago

Fixed, tested in the latest release and works fine