Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.

Bug 1650576

Summary: [Logs] Reduce OpenDaylight's JVM GC pressure and lock contention by ditching the PaxOsgi log appender
Product: Red Hat OpenStack Reporter: Michael Vorburger <vorburger>
Component: puppet-opendaylightAssignee: Michel Peterson <mpeterso>
Status: CLOSED WONTFIX QA Contact: Noam Manos <nmanos>
Severity: medium Docs Contact:
Priority: medium    
Version: 13.0 (Queens)CC: amcleod, jjoyce, joflynn, jschluet, mkolesni, mpeterso, slinaber, trozet, tvignaud, vorburger, vpickard
Target Milestone: z5Keywords: Triaged, ZStream
Target Release: 13.0 (Queens)   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: Logs
Fixed In Version: puppet-opendaylight-8.2.2-5.9126c8dgit.el7ost Doc Type: Enhancement
Doc Text:
Previously, OpenDaylight packaging used the default OpenDaylight `log_pattern` values and included the PaxOsgi appender. These default values are not always appropriate for every deployment and it is appropriate to configure custom values. With this update, `puppet-opendaylight` has two additional configuration variables: 1) `log_pattern`: Use this variable to configure which log pattern you want to use with the OpenDaylight logger log4j2. 2) `enable_paxosgi_appender`: Use this boolean flag to enable or disable the PaxOsgi appender. `puppet-opendaylight` also modifies the OpenDaylight defaults. Deployments that use `puppet-opendaylight` have new defaults: - `log_pattern`: %d{ISO8601} | %-5p | %-16t | %-60c{6} | %m%n - `enable_paxosgi_appender`: false .New variable configuration options ##### `log_pattern` String that controls the log pattern used for logging. Default: `%d{ISO8601} | %-5p | %-16t | %-60c{6} | %m%n` Valid options: A string that is a valid log4j2 pattern. ##### `enable_paxosgi_logger` Boolean that controls whether the PaxOsgi appender is enabled for logging. If you enable the `enable_paxosgi_logger` variable, you must also modify the log pattern to utilize the additional capabilities. Modify the `log_pattern` variable and include a pattern that contains the PaxOsgi tokens. For example, set the `log_pattern` variable to a string that includes the following values: '%X{bundle.id} - %X{bundle.name} - %X{bundle.version}' If you do not edit the `log_pattern` variable, the PaxOsgi appender is still enabled and continues to run but logging does not utilize the additional functionality. For example, set the `enable_paxosgi_logger` variable to `true` and set the `log_pattern` variable to the following value: '%d{ISO8601} | %-5p | %-16t | %-32c{1} | %X{bundle.id} - %X{bundle.name} - %X{bundle.version} | %m%n' Default: `false` Valid options: The boolean values `true` and `false`.
Story Points: ---
Clone Of:
: 1660167 (view as bug list) Environment:
Last Closed: 2019-03-06 16:15:31 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1577975, 1651668, 1660167    

Description Michael Vorburger 2018-11-16 14:17:04 UTC
Based on analysis from bug 1577975, I believe that one of the ways with which we can reduce object allocation in OpenDaylight, thus causing less pressure on the Java Virtual Machine's Garbage Collector running ODL, with a simple logging configuration change.

The full technical background is discussed upstream in https://jira.opendaylight.org/browse/ODLPARENT-176.  The proposed change is https://git.opendaylight.org/gerrit/#/c/77790/2/karaf/opendaylight-karaf-resources/src/main/resources/etc/org.ops4j.pax.logging.cfg

The impact of that is that the ODL karaf.log will not include the ID, name and version of the respective OSGi bundle which logs anymore.  This is a very small price to pay (nobody will miss that IMHO) if there is even a slight chance that this simple change can help ODL's memory management.

Let's discuss where this change can be made, I'm hoping that this is a 5' change in some ... template.. in .. Triple'O?  Filing this for component puppet-opendaylight, plz change to opendaylight if needed.

Comment 1 Michael Vorburger 2018-11-16 14:18:20 UTC
Tim, would this be trivial for you (or for to ask Janki) to change?

Comment 2 Michael Vorburger 2018-11-16 18:26:59 UTC
Updated Summary due to new finding that this could also help with lock contention (ODLPARENT-176).

Comment 3 Tim Rozet 2018-11-19 15:45:10 UTC
Yeah it is pretty easy to change. Are we able to always set the log4j2.pattern as "%d{ISO8601} | %-5p | %-16t | %-60c{6} | %m%n"? In other words, we don't expect that pattern to change over future releases right?

Also, looks like the patch was abandoned to make this as a default change in ODL. Is it better to just change this as the default in ODL?

Comment 4 Michael Vorburger 2018-11-20 14:01:30 UTC
> Yeah it is pretty easy to change. Are we able to always set the
> log4j2.pattern as "%d{ISO8601} | %-5p | %-16t | %-60c{6} | %m%n"? In other
> words, we don't expect that pattern to change over future releases right?

Correct, this pattern would be fairly fixed (unless we realized it wasn't "wide" enought and wanted to change the numbers again; but I actually ran it with this, and it looked about right, to me).

> Also, looks like the patch was abandoned to make this as a default change in
> ODL. Is it better to just change this as the default in ODL?

No Go, we can't.  It's one of those "timeline" kind of things... one of the other odlparent parent committers (not myself, nor skitt) is against changing this in the short or even medium term, because it's an "incompatible change" - of log format output.  Strictly speaking correct of course, but .. anyway.  IMHO the possible benefits outweight the negligable disadvantage of the format change, for us downstream - I'm not aware of any automated tools parsing ODL logs in RHOSP which could trip over this change (shout if you are).  This is why I suggest we make this change downstream rather sooner than later.   If may also be applied upstream, possibly in odlparent 5.0.0 - this means the release after Neon, at the earliest (!) - not the sort of timeframe we want to work with here.  Makes sense?

Based on this, would you be willing to Take this issue?  Or Janki?

Comment 5 Tim Rozet 2018-11-20 14:52:39 UTC
OK Michael. I'm fine then with going the puppet-opendaylight route. I'll let Mike triage/assign this bug.

Comment 6 Michel Peterson 2018-11-22 08:52:13 UTC
Taking this.

Comment 8 Michel Peterson 2018-11-25 15:29:52 UTC
Two patches were commited in u/s's master branch that allow the configuration of these parameters, defaulting to the requested values. 

Once accepted merged to master they will be backported as required and then the status changed to MODIFED.

Comment 11 Michael Vorburger 2018-12-03 13:16:11 UTC
It seems that this will also help for bug 1651668 ...

Comment 29 Franck Baudin 2019-03-06 16:15:31 UTC
As per depreciation notice [1], closing this bug. Please reopen if relevant for RHOSP13, as this is the only version shipping ODL.

[1] https://access.redhat.com/documentation/en-us/red_hat_openstack_platform/14/html-single/release_notes/index#deprecated_functionality

Comment 30 Franck Baudin 2019-03-06 16:17:25 UTC
As per depreciation notice [1], closing this bug. Please reopen if relevant for RHOSP13, as this is the only version shipping ODL.

[1] https://access.redhat.com/documentation/en-us/red_hat_openstack_platform/14/html-single/release_notes/index#deprecated_functionality