Bug 1650576 - [Logs] Reduce OpenDaylight's JVM GC pressure and lock contention by ditching the PaxOsgi log appender
Summary: [Logs] Reduce OpenDaylight's JVM GC pressure and lock contention by ditching ...
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat OpenStack
Classification: Red Hat
Component: puppet-opendaylight
Version: 13.0 (Queens)
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: z5
: 13.0 (Queens)
Assignee: Michel Peterson
QA Contact: Noam Manos
URL:
Whiteboard: Logs
Depends On:
Blocks: 1577975 1651668 1660167
TreeView+ depends on / blocked
 
Reported: 2018-11-16 14:17 UTC by Michael Vorburger
Modified: 2019-10-22 08:47 UTC (History)
11 users (show)

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`.
Clone Of:
: 1660167 (view as bug list)
Environment:
Last Closed: 2019-03-06 16:15:31 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
OpenDaylight gerrit 78054 0 None None None 2018-11-22 14:52:08 UTC
OpenDaylight gerrit 78111 0 None None None 2018-11-26 10:44:35 UTC
OpenDaylight gerrit 78392 0 None None None 2018-12-12 18:10:39 UTC
OpenDaylight gerrit 78393 0 None None None 2018-12-12 18:10:39 UTC

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


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