On May 20, 2016, 6:36 p.m., Bruce Schuchardt wrote:
> > I didn't see anything related to isLoner and I didn't see any diff for
> > LogWriterFactory to look at.
> > Going back and looking at the LogWriterFactory I don't understand the
> > !isLoner code in it.
> > The old ticket (35602) had to do with loners never logging a banner so I
> > would have thought this code
> > would need to say something like "if isLoner then logBanner". But instead
> > it is "if !isLoner then logBanner".
> > But I didn't see any problems with the changes you are making.
My confusion with LogWriterFactory is independent of Bruce's change, but maybe
now is a good time to address it as well?
I'm not sure why LogWriterFactory needs boolean isLoner (supposedly to fix TRAC
#35602 (Loner does not log banner)). The logic seems to say that if isLoner is
true then print the banner even if INHIBIT_DM_BANNER is true.
// log the banner
if (InternalDistributedSystem.getReconnectAttemptCounter() == 0 // avoid
filling up logs during auto-reconnect
&& !isSecure && (!isLoner /* do this on a loner to fix bug 35602 */
|| !Boolean.getBoolean(InternalLocator.INHIBIT_DM_BANNER))) {
// LOG:CONFIG:
logger.info(LogMarker.CONFIG, Banner.getString(null));
}
The last block suppresses logging config if isLoner is true.
// log the config
if (logConfig) {
if (!isLoner) {
// LOG:CONFIG: changed from config to info
logger.info(LogMarker.CONFIG,
LocalizedMessage.create(LocalizedStrings.InternalDistributedSystem_STARTUP_CONFIGURATIONN_0,
config.toLoggerString()));
}
}
Both of the above uses of isLoner seem strange to me. Why do we suppress config
and force banner if isLoner is true?
- Kirk
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47650/#review134180
-----------------------------------------------------------
On May 20, 2016, 6:48 p.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47650/
> -----------------------------------------------------------
>
> (Updated May 20, 2016, 6:48 p.m.)
>
>
> Review request for geode, Darrel Schneider, Hitesh Khamesra, Jianxia Chen,
> and Udo Kohlmeyer.
>
>
> Bugs: GEODE-1415
> https://issues.apache.org/jira/browse/GEODE-1415
>
>
> Repository: geode
>
>
> Description
> -------
>
> Banners are normally logged during creation of an InternalDistributedSystem.
> In DUnit tests the output is filled with these banners and they all have the
> same information, so they are just clutter.
>
> This change-set disables banners in non-locator VMs launched by
> DUnitLauncher/ProcessManager. Precheckin testing shows that the amount of
> output for geode-core/distributedTests alone used to be over 1gb and is now
> less than 500mb.
>
> Probably the only change here that requires much scrutiny is in
> LogWriterFactory. Darrel, I couldn't figure out what the isLoner check is
> there for.
>
> There are a lot of diffs in Banner.java that you can ignore - I found that it
> was using tabs for indentation and changed these to spaces. There are no
> other changes to that class.
>
>
> Diffs
> -----
>
>
> geode-core/src/main/java/com/gemstone/gemfire/admin/internal/AdminDistributedSystemImpl.java
> 21a9ae3c14b697a94787f3236f441046ca639d55
>
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalLocator.java
> cf8d91ee9dc12224b8074d6c4dc8c738c00a3b0f
> geode-core/src/main/java/com/gemstone/gemfire/internal/Banner.java
> 4b01e54dff6f11b06f85165ca843e5a5f7b606ad
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/properties.html
> 1cd1b72aa8d2e312c44b435bc215885b2a312489
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/logging/LogWriterFactory.java
> d9352c0c90873750402ef66ad07bec2f12935198
>
> geode-core/src/test/java/com/gemstone/gemfire/internal/logging/DistributedSystemLogFileJUnitTest.java
> 69df873674632a8908a022d8220dcb2f329de375
>
> geode-core/src/test/java/com/gemstone/gemfire/test/dunit/standalone/DUnitLauncher.java
> 0c600ab57cad5e6443e340f1b6a160ae39b966cb
>
> geode-core/src/test/java/com/gemstone/gemfire/test/dunit/standalone/ProcessManager.java
> 5dbdfd6dee85e2de1d3c993e4ad096c5dcc1ed40
>
> Diff: https://reviews.apache.org/r/47650/diff/
>
>
> Testing
> -------
>
> precheckin had three unrelated failures that reproduce consistently without
> these changes.
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
