Bug 702109 - hibernate detach utility should skip constants (static final variables)
hibernate detach utility should skip constants (static final variables)
Status: CLOSED CURRENTRELEASE
Product: RHQ Project
Classification: Other
Component: Core Server (Show other bugs)
4.0.0
All All
urgent Severity medium (vote)
: ---
: ---
Assigned To: John Mazzitelli
Mike Foley
:
Depends On:
Blocks: jon3 rhq401 712192
  Show dependency treegraph
 
Reported: 2011-05-04 15:19 EDT by John Mazzitelli
Modified: 2013-09-02 03:16 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-09-02 03:16:54 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description John Mazzitelli 2011-05-04 15:19:46 EDT
In HibernateDetachUtility.nullOutFieldsByFieldAccess, we do this:

        Class tmpClass = object.getClass();
        List<Field> fieldsToClean = new ArrayList<Field>();
        while (tmpClass != null && tmpClass != Object.class) {
            Collections.addAll(fieldsToClean, tmpClass.getDeclaredFields());
            tmpClass = tmpClass.getSuperclass();
        }

We should be more selective about what goes into fieldsToClean. We should check each field with this:

if (java.lang.reflect.Modifier.isFinal(field.getModifiers()) &&
    java.lang.reflect.Modifier.isStatic(field.getModifiers()))
{
  .. this field is a static final - no need to cleanse this...
}

Its quite possible we should not cleanse pure "static" variables, whether or not they are final.  Need to think about that some more.
Comment 1 Jay Shaughnessy 2011-05-04 15:27:42 EDT
I think that's probably true, we're not passing back static fields defined in the class.

Also, this may be a decent area for a Map of Class to Fields, why inspect the class each time, we're always getting the same set of valid Fields, I think.
Comment 2 John Mazzitelli 2011-05-04 16:13:47 EDT
we should also skip "transient" fields as well
Comment 3 John Mazzitelli 2011-05-05 10:17:38 EDT
(In reply to comment #1)
> Also, this may be a decent area for a Map of Class to Fields, why inspect the
> class each time, we're always getting the same set of valid Fields, I think.

AFAIK, the JRE already does its own caching of this data. Earlier versions of the JRE did a poor job of this, but later versions have highly efficient reflection API.
Comment 4 John Mazzitelli 2011-05-05 10:20:20 EDT
(In reply to comment #3)
> (In reply to comment #1)
> > Also, this may be a decent area for a Map of Class to Fields, why inspect the
> > class each time, we're always getting the same set of valid Fields, I think.
> 
> AFAIK, the JRE already does its own caching of this data. Earlier versions of
> the JRE did a poor job of this, but later versions have highly efficient
> reflection API.

Now as I say that, since we are stripping out fields we don't want to process, we could cache only the fields we want to process. For now, I won't do that - but that could be a place for further optimization.
Comment 5 John Mazzitelli 2011-05-05 13:37:43 EDT
commit - 3e3e382

this is a code change/optimization. Nothing for QA to test.

Note that I changed the warning message that is spit out if the util recurses too deep:

            LOG.warn("Recursed too deep [" + depth + " > " + depthAllowed
                + "], will not attempt to detach object of type ["
                + ((value != null) ? value.getClass().getName() : "N/A")
                + "]. This may cause serialization errors later. If so, "
                + "you can try to work around this by setting the system property [" + DEPTH_ALLOWED_SYSPROP
                + "] to a value higher than [" + depth + "].");

Also note that we can now configure how deep we will allow this utility to go. This is in case we hit another bug like bug #702390 - we'll at least have a workaround. To configure the depth allowed, put this in rhq-server.properties:

rhq.server.hibernate-detach-utility.depth-allowed=100

where 100 can be any depth you want.  The default is 50.
Comment 6 Charles Crouch 2011-05-09 16:11:50 EDT
Added for triage for RHQ 4.0.1
Comment 7 John Mazzitelli 2011-05-10 12:39:15 EDT
cherry picked 3e3e382 to release-4.0.0 branch, commit 610b23bf0b88470c76727b81a6c9bc032ab3f147
Comment 8 Mike Foley 2011-05-11 17:40:58 EDT
internal refactoring.  expressly untestable in itself, as documented above.
Comment 9 Heiko W. Rupp 2013-09-02 03:16:54 EDT
Bulk closing of issues that were VERIFIED, had no target release and where the status changed more than a year ago.

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