Bug 1891332

Hi, some information I got just in case to make Artur's life easier ;)

The problem introduced when you start using constructorCache (https://gerrit.ovirt.org/#/c/106355/).
Each time you get into CommandsFactory::getCommandConstructor, the expectedParams is a new class instance, therefore a new Pair and a new key to the constructorCache map.
It adds up each time and then you have a really big map having multiple constructors which are the same one causing to the memory usage increment.

I hope it will save you some time.

(In reply to Liran Rotenberg from comment #1)
> Hi, some information I got just in case to make Artur's life easier ;)
> 
> The problem introduced when you start using constructorCache
> (https://gerrit.ovirt.org/#/c/106355/).
> Each time you get into CommandsFactory::getCommandConstructor, the
> expectedParams is a new class instance, therefore a new Pair and a new key
> to the constructorCache map.
> It adds up each time and then you have a really big map having multiple
> constructors which are the same one causing to the memory usage increment.
> 
> I hope it will save you some time.

Liran, thanks for additional information it made my life easier indeed :)
I will soon try to provide fix for the leak.

(In reply to Liran Rotenberg from comment #1)
> Hi, some information I got just in case to make Artur's life easier ;)
> 
> The problem introduced when you start using constructorCache
> (https://gerrit.ovirt.org/#/c/106355/).
> Each time you get into CommandsFactory::getCommandConstructor, the
> expectedParams is a new class instance, therefore a new Pair and a new key
> to the constructorCache map.

It caught my eyes because of the term "Class instance" that was used here - note that the JVM maintains one Class object per type so comparing Classes as we do should be ok
but yeah, the expectedParams is an array, and arrays should not be compared using Objects#equals

Any chance to get this fix for 4.4.3 ?

(In reply to mlehrer from comment #4)
> Any chance to get this fix for 4.4.3 ?

Technically I can prepare a patch for 4.4.3, the question is -  would it fit into the release schedule, Martin?

let's backport to 4.4.3

Verified version:

[root@rhev-red-01 httpd]# rpm -qa | grep -i rel
rhv-release-4.4.3-13-001.noarch
redhat-release-8.3-1.0.el8.x86_64

[root@rhev-red-01 httpd]# rpm -qa | grep -i ovirt-engine-4
ovirt-engine-4.4.3.10-0.1.el8ev.noarch



Eclipse Memory Analyzer report (zip):
   https://drive.google.com/drive/folders/1AfNoi9gQkMyLEB0U7It_m7jL2LFSe6Ex?usp=sharing


Still have a memory leak (Results looks the same)

Also, updated google doc (Attached)- Includes before & after fix results

when was a similar test run on earlier versions? is this a regression from 4.4.2?

As mention in https://bugzilla.redhat.com/show_bug.cgi?id=1891332#c7.

we succeed to reproduce it on our system, with versions:
rel8.3
rhv4.4.3-7
ovirt-engine-4.4.3.5

(In reply to David Vaanunu from comment #11)
> As mention in https://bugzilla.redhat.com/show_bug.cgi?id=1891332#c7.
> 
> we succeed to reproduce it on our system, with versions:
> rel8.3
> rhv4.4.3-7
> ovirt-engine-4.4.3.5

That wasn't the question, but anyway, in offline discussion it was mentioned that it's happening in 4.4.1, so not a 4.4.2/4.4.3 regression

postponing for further investigation in 4.4.4 since it doesn't seem to be fixed.

Verified version:

[root@rhev-red-01 tmp]# rpm -qa | grep -i rel
rhv-release-4.4.3-14-001.noarch
redhat-release-8.3-1.0.el8.x86_64

[root@rhev-red-01 tmp]# rpm -qa | grep -i ovirt-engine-4
ovirt-engine-4.4.3.12-0.1.el8ev.noarch


Results Doc:
https://docs.google.com/document/d/1HLkJTT5Ph2-DobNIQVJJFdDNLQc2wJ3LCBcwtopI-Yc/edit?usp=sharing