Bug 819521

Summary: [plugin container] management of the Server Resource -> ProcessInfo mappings needs to be refactored
Product: [Other] RHQ Project Reporter: Ian Springer <ian.springer>
Component: Plugin Container, PerformanceAssignee: RHQ Project Maintainer <rhq-maint>
Status: NEW --- QA Contact: Mike Foley <mfoley>
Severity: medium Docs Contact:
Priority: medium    
Version: 4.3CC: hbrock, hrupp, jshaughn, lkrejci
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=802003
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 620931    

Description Ian Springer 2012-05-07 09:20:49 EDT
The management of the mappings is currently done within ResourceContext and is not integrated with discovery at all. Whenever server discovery runs, the mappings should get updated - this does not currently happen. So as it is today, when plugins call ResourceContext.getNativeProcess(), unnecessary discovery scans often occur. This can make that method take a long time to return and waste CPU cycles.

We should create a new ServerProcessManager class that hangs directly off InventoryManager. This class would manage the mappings and server discovery scan listeners would update the mappings whenever server discovery scans are run. The class would only run additional server discovery scans when the requested mapping corresponded to a process that was no longer running.
Comment 1 Mike Foley 2012-05-08 15:33:37 EDT
per BZ triage ... crouch, loleary, foley
Comment 2 Lukas Krejci 2012-05-24 11:25:49 EDT
Refreshing the process info on discovery scans is not enough... It opens up a possibility for unnoticed process restarts, where the process info would no longer correspond to the actual process the resource should be managing.

The prominent plugin that needs always-up-to-date process info is Apache, because it actually needs the process info to be able to figure out the correct runtime configuration of the apache resource.

As to "unnecessary discovery scans often occur" - the discovery scans only occur if the code in ResourceContext.getNativeProcess() establishes that the last known processInfo is no longer running - at that point new process scan is done and discovery is kicked off to see whether it can re-match the resource to the currently running process.
Comment 3 Ian Springer 2012-05-24 12:33:00 EDT
> Refreshing the process info on discovery scans is not enough... 
> It opens up a > possibility for unnoticed process restarts, where
> the process info would no longer correspond to the actual process 
> the resource should be managing.

We would always verify the ProcessInfo is still running. If not, we would run a new discovery scan to get the new process. As part of this, I would enhance ProcessInfo to detect when the underlying process has died and mark the object as permanently defunct, and to make sure the process's name, command line, etc. has not changed. That way there would be much less chance of the ProcessInfo ending up representing some new process with the same pid as the original process.

> The prominent plugin that needs always-up-to-date process info is 
> Apache, because it actually needs the process info to be able to 
> figure out the correct runtime configuration of the apache resource.

It would still have this. The changes I'm suggesting would just make the underlying impl that returns the server's ProcessInfo more efficient, only doing discovery scans when absolutely necessary.

> As to "unnecessary discovery scans often occur" - the discovery 
> scans only occur if the code in ResourceContext.getNativeProcess()
> establishes that the last known processInfo is no longer running
> - at that point new process scan is done and discovery is kicked
> off to see whether it can re-match the resource to the currently 
> running process.

As I recall, it looked to like a discovery scan would always be done
the first time getNativeProcess() is called for a particular ResourceType.