-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47650/#review134180
-----------------------------------------------------------


Fix it, then Ship it!





geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalLocator.java
 (line 123)
<https://reviews.apache.org/r/47650/#comment198828>

    What was the reason for changing the prefix on this system property?
    I think we want to get rid of this "gemfire." prefix in geode. The old 
"Locator." prefix would work in both gemfire and geode.


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.

- Darrel Schneider


On May 20, 2016, 10 a.m., Bruce Schuchardt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47650/
> -----------------------------------------------------------
> 
> (Updated May 20, 2016, 10 a.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/test/java/com/gemstone/gemfire/internal/logging/DistributedSystemLogFileJUnitTest.java
>  d9352c0c90873750402ef66ad07bec2f12935198 
>   
> 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
> 
>

Reply via email to