#841 LOG: Wrong default logStreamAppHighLimit (1)

[Hans Feldt]

The value is 1 causing writes to be discarded with no buffering.

This was introduced by:
changeset: 4758:2e40297612ec
user: Lennart Lund lennart.lund@ericsson.com
date: Fri Dec 20 13:14:09 2013 +0100
summary: logsv: Handle log files on both active and standby [#152]

Previously the limits were unlimited, this one should be reverted back to 0 (unlimited).

[Hans Feldt]

Another problem, logsv contains an "empty" OpenSafLogConfig instance that makes overload configuration via environment variables a no-op. This instance should probably be removed since it silently discards any settings via environment variables the way it is now.

[elunlen]

The object does not only contains the configuration defined in logsv_objects.xml it also contains default values given in the class definition, logsv_classes.xml. This makes the content of the configuration object looks like this:

immlist logConfig=1,safApp=safLogService

Name Type Value(s)

logStreamSystemLowLimit SA_UINT32_T 0 (0x0)
logStreamSystemHighLimit SA_UINT32_T 0 (0x0)
logStreamAppLowLimit SA_UINT32_T 0 (0x0)
logStreamAppHighLimit SA_UINT32_T 1 (0x1)
logRootDirectory SA_STRING_T /repl_opensaf/saflog
logMaxLogrecsize SA_UINT32_T 1024 (0x400)
logMaxApplicationStreams SA_UINT32_T 64 (0x40)
logFileSysConfig SA_UINT32_T 1 (0x1)
logFileIoTimeout SA_UINT32_T 500 (0x1f4)
logConfig SA_STRING_T logConfig=1
logAmfHctDefMaxDuration SA_TIME_T 180000000000 (0x29e8d60800, Thu Jan 1 01:03:00 1970)
SaImmAttrImplementerName SA_STRING_T safLogService
SaImmAttrClassName SA_STRING_T OpenSafLogConfig
SaImmAttrAdminOwnerName SA_STRING_T IMMLOADER

All attributes can be given another value than the value given in the class definition by setting a new value in the object (in the logsv_objects.xml file or an equivalent used in system configuration).
A system using the log service should have its own logsv_objects.xml file containing a suitable configuration for the log service and that file should be used when creating the imm.xml file instead of the default Open SAF file. Attributes that does not have to be configured in any other way than pre-defined may be omitted.

[elunlen]

changeset: 5147:8dc071efd65f
tag: tip
parent: 5145:01673cfc493a
user: Lennart Lund lennart.lund@ericsson.com
date: Fri Apr 11 13:08:26 2014 +0200
summary: lgsv: Change logStreamAppHighLimit default from 1 to 0 [#841]

rev: 8dc071efd65f2af959f0bdddc5b013b2859ce829

changeset: 5146:cf14e876e1fe
branch: opensaf-4.4.x
parent: 5144:c2fe0de38607
user: Lennart Lund lennart.lund@ericsson.com
date: Fri Apr 11 13:08:26 2014 +0200
summary: lgsv: Change logStreamAppHighLimit default from 1 to 0 [#841]

rev: cf14e876e1fea64484ba5e0755401ad70673c17f

[Hans Feldt]

Introducing an "empty" OpenSafLogConfig instance was a non backwards compatible change. How do you plan to adress that?

For convenience I had mentioned it as part of this ticket. Now it is closed but the problem still remains.

[Mathi Naickan]

Just trying to understand the backward compatiblity problem mentioned here.
This config object can also be created dynamically isn't it.
(OR) Did you mean that when this object is not configured, the limits are not getting loaded with default values(from environs)?

[Hans Feldt]

Because of the opensaf default created "empty" OpenSafLogConfig instance, configuration via environment variables is deliberately by design skipped. Even though no configuration of this is included in the OpenSafLogConfig instance. This is described in the README but is not backwards compatible and has caused us quite a lot of trouble.

So either
1) opensaf does not create any OpenSafLogConfig instance or
2) if limits are empty in the OpenSafLogConfig instance it should use environment variables (as before)

I don't see a new ticket for this problem.

[Mathi Naickan]

Attached is a patch that works for 2) above.
But, I should say this is quite a strange case of 1) or 2) above!

The basic idea of http://devel.opensaf.org/ticket/2463 was to introduce this configuration object such that the default limits of LOG are changeable at runtime. We shouldn't move away from this object. The integrator does has the flexiblity to avoid creating this configuration object(empty or not) by removing it from the logsv_objects.xml.
When a user migrates to the next release say 4.3, it is expected that the user updates the integration scripts also such that the scripts can add/modify this logconfig object at runtime.

However, if we should still support the case of the cautious-adopter of enhancements, then we are giving preference to environment variables over a configuration object, and this could only be a temporary thing and not an ideal way to move forward.

[elunlen]

The main purpose of introducing a configuration object was not to make configuration changes in runtime except for one specified configuration, the root directory. However it was discussed to make more configurations runtime changable in the future and a configuration object makes that possible.

An important possibility when having the configuration in a configuration object is the possibility to read the configuration and this I think makes it impossible to use this solution strait off. If environment variables are used instead of the values in the configuration object there will be a mismatch between the actual configuration and the configuration that can be read in the IMM object.

Solution comments:
Solution A:
Handling of environment variables are done as in your patch with the following addition.
The IMM object is changed to reflect the actual value. This means that all attributes that can be replaced by environment variables has to be runtime changable and if an environment variable is used the log service has to use the IMM Om interface to change the corresponding IMM attribute.
All replacements should be logged in the system log.
This may seems a bit strange for the user. There may be conflicts between for example old environment variables and new configuration in definition of configuration object.

Solution B:
For each configuration, check if there is an environment variable. If so don't change the configuration or the configuration object but log a warning or possibly an error in the system log.
Make some more configuration runtime changable e.g. the mailbox limits.

NOTE:
Not all configuration is not possible to change in runtime at least not "logFileSysConfig"

[Mathi Naickan]

Hi Lennart,

I think Solution B is easier to maintain moving forward.
i.e. To my patch, I shall add a LOG_NOTICE whenever the logconfig object attributes are "internally" overwridden with the values from environmnet variables.

So, the end behaviour will be as below:

As usual when LGS is coming up,
(a) the values of the logConfig attributes are used, i.e. if the object and attributes are configured.
(b) when no logConfig attributes are configured(except logRootDirectory) then, "the default values are used".
Now, after this patch
(c) when no logConfig attributes are configured, i.e "the default values are detected" but if there are environment variables are configured then LGS shall log a NOTICE message and "internally" overwrides the values of the logConfig attribute with the values obtained from the environment variable.
- In future when support for modifiying these attributes dynamically through CCB operations is added then from a situation (c) above, the values passed through the CCB shall overwride the values stored in (c).

Thanks,
Mathi.

[elunlen]

Solution B was to not at any time overwrite values read from the config object even if they are the same as default and an environment variable with another value exist. In this solution this shall be detected and a warning shall be written to the syslog. Making it possible to change some attributes in runtime can give the user a chance to fix such a problem, at least the limits could be made possible to change in runtime but security must be taken into consideration, see (d.) below.
It is NOT OK to change configuration using environment variables without to also change in the configuration object. Configuration object and the actual settings must match.

To consider:
a.
If environment variables exist on one SC node but not on the other (or differ) we end up with different configuration on the nodes.
b.
A system may do a correct configuration in a configuration object but if for some reason an old logd.conf file containing some old configuration exist the correct configuration will be overwritten. This means that an upgrade always must contain a removal of eventually old configuration. Not needed today since a configuration object always have precedence. I see this as a risk. May have consequences as in (a.).
c.
If a configuration object exist and is used, some configuration is changed via environment variables and the configuration object is not updated to reflect the changes there will be a mismatch between the real configuration and what can be read using for example immlist. This must be considered to be an error and is a risk.
d.
To make more configuration attributes possible to change in runtime may be considered as a security risk. However there is already one attribute that can be changed, the root path, but there is some security built into that possibility and that is that it can only be changed to a path that exists. If the path does not exist the change will be rejected. When this configuration possibility was implemented the security aspect was a very important issue. I don’t know how security can be handled in this case but if bad configuration changes are done it may lead to problems that can affect the system e.g. node restarts.
e.
If attributes are runtime configurable and environment variables can be used a situation where a configuration changed in runtime will change back to something else if the node is restarted may happen. In the current patch a configuration value coming from the configuration object will always be replaced by a value in an environment variable if exist, except for logMaxLogrecsize and logMaxApplicationStreams that must have a value of 1024 and 64 respectively in order to be replaced. The logRootDirecory is not affected.

This is what was considered when the current construction was made. This I why configuration using configuration object and environment variables is not mixed and why only one attribute is runtime configurable. This is also why the configuration object is always used if it exists even if there are environment variables.

The current construction:

If a configuration object exists it is used for all configurations.

The class definition for the configuration object provides default values for all attributes except the root path (logsv_classes.xml). All default values can be overridden by adding and setting attributes in the logsv_objects.xml file

The log root path can be configured in runtime. For security reasons it can only be set to an already existing path i.e. a new root path is never created by the log service when changed in runtime. For security reasons no other configurations can be changed in runtime.

If a configuration object exists but some attributes are missing e.g. if the object is an old version hard coded default values are used for the missing values.

If no configuration object exists environment variables are used (old method for configuration). If a value is not configured in an environment variable a hard coded default value is used.

[elunlen]

We should keep the current configuration handling. An improvement can be to check if there are any environment variables also if a configuration object exist but not use them to change the configuration instead a warning can be written in the syslog.

[Mathi Naickan]

Okay. I will investigate why the errno is getting set during strotul(),
and the "warning" log part. Will send a patch.

[Mathi Naickan]

Assigned to me, for Part two of the ticket. Patch sent for review in
https://sourceforge.net/p/opensaf/mailman/opensaf-devel/thread/patchbomb.1399067120%40ubuntu/#msg32292830

[Mathi Naickan]

[staging:6bf358]
[staging:3e678a]
[staging:16b728]
[staging:88e4c5]
[staging:df5163]
[staging:fb2540]

changeset: 5216:6bf358e2b08b
branch: opensaf-4.3.x
parent: 5213:cbc7b5661533
user: Mathivanan N.P.mathi.naickan@oracle.com
date: Tue May 06 19:24:33 2014 -0400
summary: log: ignore environment variables when config object exists [#841]

changeset: 5217:3e678abccbf8
branch: opensaf-4.3.x
user: Mathivanan N.P.mathi.naickan@oracle.com
date: Tue May 06 19:26:23 2014 -0400
summary: log: fix errno usage for calls to strtoul [#841]

changeset: 5218:16b728b2ff49
branch: opensaf-4.4.x
parent: 5214:0c5247698039
user: Mathivanan N.P.mathi.naickan@oracle.com
date: Tue May 06 19:24:33 2014 -0400
summary: log: ignore environment variables when config object exists [#841]

changeset: 5219:88e4c54bab4e
branch: opensaf-4.4.x
user: Mathivanan N.P.mathi.naickan@oracle.com
date: Tue May 06 19:26:23 2014 -0400
summary: log: fix errno usage for calls to strtoul [#841]

changeset: 5220:df5163217029
parent: 5215:e3479f3a8c82
user: Mathivanan N.P.mathi.naickan@oracle.com
date: Tue May 06 19:24:33 2014 -0400
summary: log: ignore environment variables when config object exists [#841]

changeset: 5221:fb2540b81df4
tag: tip
user: Mathivanan N.P.mathi.naickan@oracle.com
date: Tue May 06 19:26:23 2014 -0400
summary: log: fix errno usage for calls to strtoul [#841]