It's a race condition in how GfshRule, GfshExecution, and ProcessLogger fork a process before attaching the stdout and stderr listeners. The changes for geode-log4j caused this race condition window to take slightly longer. I'll have a fix soon and ConnectCommandAcceptanceTest should then be back to consistently green
On Tue, Sep 24, 2019 at 12:23 PM Kirk Lund <kl...@apache.org> wrote: > It's not a problem in TomcatInstall. Nothing obvious anywhere. Even the > output that GfshRule says is missing the string actually does contain the > missing text. I have no idea why this passed in precheckin but fails now... > > On Tue, Sep 24, 2019 at 11:16 AM Kirk Lund <kl...@apache.org> wrote: > >> ConnectCommandAcceptanceTest is now failing on develop. Looks like the >> test is using Geode 1.3.0 (not sure why). >> >> It also looks like my commit and the following commit didn't merge >> properly despite not having any conflicts. Looking into how to fix it now. >> >> It's possible that GEODE-7168 removed geode-log4j from the Tomcat >> classpath... >> >> commit a0f7a0f041c47830053b0fd1c8d9c4e51fb9a058 >> Author: Bruce Schuchardt <bschucha...@pivotal.io> >> Date: Wed Sep 11 15:49:35 2019 -0700 >> >> GEODE-7168 tomcat rolling upgrade test failure >> >> This PR reinstates the changes to this test and modifies the >> geode-old-versions subproject to preserve the full version string >> (with >> periods) for older versions. This lets the tomcat test build a >> classpath without requiring the target version to have representation >> in >> Version.java. >> >> On Tue, Sep 24, 2019 at 10:24 AM Kirk Lund <kl...@apache.org> wrote: >> >>> All classes that use *log4j-core* have now moved to the new module >>> *geode-log4j >>> *on develop. The default log4j2.xml configuration file for Geode >>> Locators and Servers has also moved to geode-log4j. >>> >>> The first Geode release expected to include this change is 1.11.0. >>> >>> The geode-log4j module is included in geode-dependencies.jar so that >>> Locators and Servers will continue to use the code and configuration in >>> geode-log4j by default. It is also included in the classpath of most >>> *integrationTest* and *distributedTest* targets to facilitate non-unit >>> test debugging, support *dunit grep for suspect strings*, and also to >>> avoid moving or changing tests that involve or require Geode Alerting or >>> Logging functionality. >>> >>> Precheckin was GREEN before merging to develop, so all Geode tests >>> should be unaffected. >>> >>> *A. The effects of not having geode-log4j on the classpath >>> (client/server/locator) are:* >>> >>> 1) Log4j will not be configurable by Geode >>> >>> 2) Log4j will print out an error message if not configured by User or >>> Geode >>> >>> This is printed out by Apache Log4j (not by Geode): >>> >>> ERROR StatusLogger No Log4j 2 configuration file found. Using default >>> configuration (logging only errors to the console), or user >>> programmatically provided configurations. Set system property >>> 'log4j2.debug' to show Log4j 2 internal initialization logging. See >>> https://logging.apache.org/log4j/2.x/manual/configuration.html for >>> instructions on how to configure Log4j 2 >>> >>> 3) Geode will not create a log file by default (when starting a server >>> or locator using GFSH) >>> >>> 4) Logging configuration properties do nothing >>> >>> Logging configuration properties include: >>> * log-disk-space-limit >>> * log-file >>> * log-file-size-limit >>> * log-level >>> >>> 5) Geode Alerts will never be generated >>> >>> Geode Alerts are exposed primarily exposed via: >>> * DistributedSystemMXBean JMX Notifications >>> * Viewable in Pulse (via DistributedSystemMXBean) >>> * (Deprecated) Admin API support for Alert Listening >>> >>> 6) GFSH commands or options involving Logging and Alerting do nothing >>> >>> GFSH commands or options involving Logging and Alerting: >>> * alter runtime (log-disk-space-limit, log-file, log-file-size-limit, >>> log-level) >>> * export logs >>> * show log >>> * start locator (log-level, redirect-output) >>> * start server (log-level, redirect-output) >>> >>> *B. The primary reasons a user might not include geode-log4j in their >>> classpath:* >>> >>> a) User wants to use Logback or other backend instead of log4j-core >>> (especially true for users of Spring Boot) >>> >>> b) User wants greater control over logging, especially in a client or >>> embedded application >>> >>> Since Geode uses log4j-api Loggers for its internal logging, swapping >>> out log4j-core for logback as the backend imposes a significant performance >>> penalty. Although, the logging APIs and backends are interchangeable, using >>> the intended backend with its matching API provides the best performance >>> for both log4j-api/log4j-core and slf4j/logback. >>> >>> *C. Impact on IDE usage* >>> >>> IntelliJ projects for Geode should automatically pull in geode-log4j >>> when it re-imports from gradle. Note that may see warnings about *"**No >>> Log4j 2 configuration file found.**"* if you're running a test that >>> doesn't have geode-log4j on its classpath. >>> >>> For more advanced projects in IntelliJ with additional non-Geode >>> modules, you might benefit from using *-Dclasspath* (or other >>> proprietary/environment options) in *Gradle VM options*. >>> >>> The commit on develop: >>> >>> commit efc2362d2bae0877a427ce2c29beae94118d6567 >>> Author: Kirk Lund <kl...@apache.org> >>> Date: Thu Aug 8 15:17:54 2019 -0700 >>> >>> GEODE-6964: Move geode log4j core classes to geode-log4j >>> >>> Introduce new Logging and Alerting SPIs. Extract all log4j-core code >>> to >>> geode-log4j module. >>> >>> The geode-core module no longer contains log4j2.xml and no longer >>> has a >>> dependency on log4j-core. >>> >>> All code that uses log4j-core has moved to the new module >>> geode-log4j. >>> The log4j2.xml for Geode now lives in geode-log4j as well. These >>> changes ensure that users have better control over logging including >>> which backend to use. This should improve user experience when using >>> Spring Boot. >>> >>> Co-authored-by: Mark Hanson <mhan...@pivotal.io> >>> >>> Let me know if there are any questions or if anyone runs into anything >>> interesting because of these changes. >>> >>>
