Re: [DISCUSS]: Handling of SDTOUT/STDERR and Signals
Hello again, Regarding my last question from the original email (unformatted jetty logging when enabling --*redirect-output*), it looks like the offending library within the *geode-pulse.war* is *commons-logging-x.x.jar.* The library is configured as *providedCompile* in *build.gradle* and, by definition, this means it should be included in the *compile classpath* but *not included in the packaged war file*. This happens because *commons-logging* is a transient dependency of another direct pulse dependency, not configured as *providedCompile; *the fix is quite simple and it just implies adding *exclude module: 'commons-logging'* to this direct dependencies. That said, two questions remain unanswered: - Any comments about the suggested approach to fix GEODE-4101 (deprecate old system properties, add new flag and set an internal system properties to ignore the old ones)?. - Anyone knows if Geode is, somehow, catching the QUIT signal and preventing the thread dump from going to the member's log file?. Can you point me to the relevant section of code or class?. Best regards. On Wed, Dec 20, 2017 at 6:33 AM, Juan José Ramos wrote: > Hello Kirk, > > Thanks for your reply. Please find my answers below, in line for an easier > follow-up. > > *>> By default Geode redirects stdout and stderr to the value of the > log-file property when the Cache starts. If log-file is set to "" then it > won't redirect them.* > > This doesn't seem to be true at the moment, I've tested the behavior > before sending the email and creating the JIRA. If you start a basic server > with the defaults + *log-file*, the output written from a > function/listener to stdout/stderr is lost, nothing is printed on the > member's logs. > > > *>> So we reluctantly made the code redirect stdout and stderr when > LogWriterAppender is registered (at the same point in Cache startup where > the behavior originally existed). If you specify a custom log4j2.xml then > this behavior does NOT happen.* > > Also tested this one and it doesn't seem to be true, the result is the same > as before: the output written to *stderr/stdout* is lost, nothing is > printed on the member's logs. > > > *>> So a user currently has two options:* > *>> 1) keep the old behavior,* > *>> 2) use a custom log4j2.xml which can be specified using the Log4J 2 > system property "log4j.configurationFile".* > > The attached zip file contains a reproducible scenario (you just need to > modify *setenv.txt* and execute *reproduce.sh*) showing that both > approaches fail, the output from *System.out.println()* and > *System.err.println()* is lost, along with the thread dump generated by *kill > -3 PID*. > Hope this is clear now, hope you can answer the questions from my previous > email so I can proceed with the fix. > Best regards > > > On Tue, Dec 19, 2017 at 2:56 PM, Kirk Lund wrote: > >> By default Geode redirects stdout and stderr to the value of the log-file >> property when the Cache starts. If log-file is set to "" then it won't >> redirect them. >> >> When we changed GemFire to use Log4J2 internally, we tried to leave stdout >> and stderr alone such that the LogWriterAppender would append log >> statements to the log-file and it would leave the stdout appender >> registered and appending to stdout. This was a change in behavior from the >> previous version of GemFire, and the management at that time decided that >> it needed to maintain the old behavior, so we reluctantly made the code >> redirect stdout and stderr when LogWriterAppender is registered (at the >> same point in Cache startup where the behavior originally existed). >> >> If you specify a custom log4j2.xml then this behavior does NOT happen. So >> a >> User currently has two options: 1) keep the old behavior, 2) use a custom >> log4j2.xml which can be specified using the Log4J 2 system property >> "log4j.configurationFile". >> >> If you copy geode-core/src/main/resources/log4j2.xml for use as the >> starting basis for your log4j2.xml then you must NOT keep this line (or >> set >> it to false): >> >> true >> >> That is the line that Geode uses to determine that it is using its own >> default log4j2.xml and this enables redirecting of stdout and stderr when >> the LogWriterAppender starts up. >> >> On Tue, Dec 19, 2017 at 7:11 AM, Ju@N wrote: >> >> > Hello Geode devs, >> > >> > Currently GEODE is "swallowing" all output sent to stdout and stderr by >> > default and there's no way of changing this behavior when starting >> members >> > through *gfsh*. >> > This, between other things, prevents users, between other things, from >> > playing around with *System.out.println()* during development phases and >> > getting thread dumps by executing a plain *kill -3* or *kill -QUIT* >> using >> > the processId, which is critical in troubleshooting. >> > Currently there are two internal flags that can be used to prevent this >> > default behavior, both have to be used at the same time and both are >> very >> > counterintui
Re: [DISCUSS]: Handling of SDTOUT/STDERR and Signals
Hi Juan, Thanks for the detailed analysis. I did some digging too and cannot see why the QUIT signal might be getting lost. I'll do some more investigation... Just for reference, you can still use the 'jstack' tool to get thread dumps although they just show on the terminal where you run the command. Although I haven't confirmed, I think there may be a difference in how stdout is handled, WRT log files, when a server or locator is started using the Server/LocatorLauncher API (as opposed to directly with gfsh). --Jens On Wed, Dec 20, 2017 at 4:12 AM, Juan José Ramos wrote: > Hello again, > > Regarding my last question from the original email (unformatted jetty > logging when enabling --*redirect-output*), it looks like the offending > library within the *geode-pulse.war* is *commons-logging-x.x.jar.* The > library is configured as *providedCompile* in *build.gradle* and, by > definition, this means it should be included in the *compile > classpath* but *not > included in the packaged war file*. This happens because *commons-logging* > is > a transient dependency of another direct pulse dependency, not configured > as *providedCompile; *the fix is quite simple and it just implies > adding *exclude > module: 'commons-logging'* to this direct dependencies. > That said, two questions remain unanswered: > - Any comments about the suggested approach to fix GEODE-4101 (deprecate > old system properties, add new flag and set an internal system properties > to ignore the old ones)?. > - Anyone knows if Geode is, somehow, catching the QUIT signal and > preventing the thread dump from going to the member's log file?. Can you > point me to the relevant section of code or class?. > > Best regards. > > On Wed, Dec 20, 2017 at 6:33 AM, Juan José Ramos > wrote: > > > Hello Kirk, > > > > Thanks for your reply. Please find my answers below, in line for an > easier > > follow-up. > > > > *>> By default Geode redirects stdout and stderr to the value of the > > log-file property when the Cache starts. If log-file is set to "" then it > > won't redirect them.* > > > > This doesn't seem to be true at the moment, I've tested the behavior > > before sending the email and creating the JIRA. If you start a basic > server > > with the defaults + *log-file*, the output written from a > > function/listener to stdout/stderr is lost, nothing is printed on the > > member's logs. > > > > > > *>> So we reluctantly made the code redirect stdout and stderr when > > LogWriterAppender is registered (at the same point in Cache startup where > > the behavior originally existed). If you specify a custom log4j2.xml then > > this behavior does NOT happen.* > > > > Also tested this one and it doesn't seem to be true, the result is the > same > > as before: the output written to *stderr/stdout* is lost, nothing is > > printed on the member's logs. > > > > > > *>> So a user currently has two options:* > > *>> 1) keep the old behavior,* > > *>> 2) use a custom log4j2.xml which can be specified using the Log4J 2 > > system property "log4j.configurationFile".* > > > > The attached zip file contains a reproducible scenario (you just need to > > modify *setenv.txt* and execute *reproduce.sh*) showing that both > > approaches fail, the output from *System.out.println()* and > > *System.err.println()* is lost, along with the thread dump generated by > *kill > > -3 PID*. > > Hope this is clear now, hope you can answer the questions from my > previous > > email so I can proceed with the fix. > > Best regards > > > > > > On Tue, Dec 19, 2017 at 2:56 PM, Kirk Lund wrote: > > > >> By default Geode redirects stdout and stderr to the value of the > log-file > >> property when the Cache starts. If log-file is set to "" then it won't > >> redirect them. > >> > >> When we changed GemFire to use Log4J2 internally, we tried to leave > stdout > >> and stderr alone such that the LogWriterAppender would append log > >> statements to the log-file and it would leave the stdout appender > >> registered and appending to stdout. This was a change in behavior from > the > >> previous version of GemFire, and the management at that time decided > that > >> it needed to maintain the old behavior, so we reluctantly made the code > >> redirect stdout and stderr when LogWriterAppender is registered (at the > >> same point in Cache startup where the behavior originally existed). > >> > >> If you specify a custom log4j2.xml then this behavior does NOT happen. > So > >> a > >> User currently has two options: 1) keep the old behavior, 2) use a > custom > >> log4j2.xml which can be specified using the Log4J 2 system property > >> "log4j.configurationFile". > >> > >> If you copy geode-core/src/main/resources/log4j2.xml for use as the > >> starting basis for your log4j2.xml then you must NOT keep this line (or > >> set > >> it to false): > >> > >> true > >> > >> That is the line that Geode uses to determine that it is using its own > >> default log4j2.xml and this enables redirec
Re: Discussion: Native PDX Reader/Writer std::string
Given that C++17 is feature-complete, it seems prudent to adopt those patterns as much as possible. My fear with #4 is that we're introducing a special case API that users become accustomed to and then unwilling to later switch to std::optional. And then we'd end up supporting multiple ways to do the same thing. And even if we can delete the special case API after C++17, there's still API churn for our users. Because of that, I favor using optional even if we have to roll our own. And then after C++17 we change our implementation to alias std::optional and there shouldn't be any code changes required from user point of view. Thanks, David On Tue, Dec 19, 2017 at 2:30 PM, Jacob Barrett wrote: > On Tue, Dec 19, 2017 at 1:52 PM Michael William Dodge > wrote: > > > Of those, #1 seems better than #2 and #3. Is there an elegant way to use > a > > sentinel value of std::string to represent null, e.g., static const > > PdxReader::NULL_STRING = ? > > > > I am absolutely opposed to sentinel values just from the standpoint that > once you pick a value someone will want that value. > > The C++17 solution to this problem is std::optional, but > unfortunately C++11 is our minimum. We could embrace boost::optional (for > which C++17 std::optional derives), but this will cause clients to also > depend on boost, which isn't ideal. We could roll our own optional... > > -Jake >
Re: [DISCUSS]: Handling of SDTOUT/STDERR and Signals
Sorry, my info on redirecting of stdout and stderr was stale (circa GemFire 7.0). Now that the code base is an Apache project, I believe redirecting of stdout and stderr no longer occurs. However, it does shut off the Log4j2 ConsoleAppender to stdout when the LogWriterAppender is activated and turns it back on when the LogWriterAppender is closed. The old code base redirected stdout and stderr using native code which is not in Apache Geode. If you want to dump stack traces, you need to use the GFSH command "export stack-traces". On Wed, Dec 20, 2017 at 1:33 AM, Juan José Ramos wrote: > Hello Kirk, > > Thanks for your reply. Please find my answers below, in line for an easier > follow-up. > > *>> By default Geode redirects stdout and stderr to the value of the > log-file property when the Cache starts. If log-file is set to "" then it > won't redirect them.* > > This doesn't seem to be true at the moment, I've tested the behavior > before sending the email and creating the JIRA. If you start a basic server > with the defaults + *log-file*, the output written from a > function/listener to stdout/stderr is lost, nothing is printed on the > member's logs. > > > *>> So we reluctantly made the code redirect stdout and stderr when > LogWriterAppender is registered (at the same point in Cache startup where > the behavior originally existed). If you specify a custom log4j2.xml then > this behavior does NOT happen.* > > Also tested this one and it doesn't seem to be true, the result is the same > as before: the output written to *stderr/stdout* is lost, nothing is > printed on the member's logs. > > > *>> So a user currently has two options:* > *>> 1) keep the old behavior,* > *>> 2) use a custom log4j2.xml which can be specified using the Log4J 2 > system property "log4j.configurationFile".* > > The attached zip file contains a reproducible scenario (you just need to > modify *setenv.txt* and execute *reproduce.sh*) showing that both > approaches fail, the output from *System.out.println()* and > *System.err.println()* is lost, along with the thread dump generated by *kill > -3 PID*. > Hope this is clear now, hope you can answer the questions from my previous > email so I can proceed with the fix. > Best regards > > > On Tue, Dec 19, 2017 at 2:56 PM, Kirk Lund wrote: > >> By default Geode redirects stdout and stderr to the value of the log-file >> property when the Cache starts. If log-file is set to "" then it won't >> redirect them. >> >> When we changed GemFire to use Log4J2 internally, we tried to leave stdout >> and stderr alone such that the LogWriterAppender would append log >> statements to the log-file and it would leave the stdout appender >> registered and appending to stdout. This was a change in behavior from the >> previous version of GemFire, and the management at that time decided that >> it needed to maintain the old behavior, so we reluctantly made the code >> redirect stdout and stderr when LogWriterAppender is registered (at the >> same point in Cache startup where the behavior originally existed). >> >> If you specify a custom log4j2.xml then this behavior does NOT happen. So >> a >> User currently has two options: 1) keep the old behavior, 2) use a custom >> log4j2.xml which can be specified using the Log4J 2 system property >> "log4j.configurationFile". >> >> If you copy geode-core/src/main/resources/log4j2.xml for use as the >> starting basis for your log4j2.xml then you must NOT keep this line (or >> set >> it to false): >> >> true >> >> That is the line that Geode uses to determine that it is using its own >> default log4j2.xml and this enables redirecting of stdout and stderr when >> the LogWriterAppender starts up. >> >> On Tue, Dec 19, 2017 at 7:11 AM, Ju@N wrote: >> >> > Hello Geode devs, >> > >> > Currently GEODE is "swallowing" all output sent to stdout and stderr by >> > default and there's no way of changing this behavior when starting >> members >> > through *gfsh*. >> > This, between other things, prevents users, between other things, from >> > playing around with *System.out.println()* during development phases and >> > getting thread dumps by executing a plain *kill -3* or *kill -QUIT* >> using >> > the processId, which is critical in troubleshooting. >> > Currently there are two internal flags that can be used to prevent this >> > default behavior, both have to be used at the same time and both are >> very >> > counterintuitive: *gemfire.OSProcess.ENABLE_OUTPUT_REDIRECTION=true* >> and >> > *gemfire.OSProcess.DISABLE_OUTPUT_REDIRECTION=false*. These flags, >> > however, >> > don't work when starting members through *gfsh*, and that's because the >> > relevant commands wrongly assume that the flags are already part of the >> > system properties too early in the lifecycle execution of the command: >> > >> > >> > *StartXCommand.java* >> > >> > @CliCommand(value = CliStrings.START_X, help = >> > CliStrings.START_X__HELP) >> > >> > @CliMetaData(shellOnly = true, r
Geode unit tests completed in '/' with non-zero exit code
Pipeline results can be found at: Concourse: https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/AcceptanceTest/builds/68
Geode unit tests completed in '/' with non-zero exit code
Pipeline results can be found at: Concourse: https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/FlakyTest/builds/58
Re: Discussion: Native PDX Reader/Writer std::string
I agree that if we could develop a version of optional that could be typedefed to C++17 optional in the future that could be an interesting win. I still hesitate to do this as the general use API though since it promotes the idea of null strings still. My current leaning is to only provide option 1 and promote all null strings to empty string. Then if it plays out that a null string actually is necessary we could add to the API geode::optional readOptionalString(...) for those that want to support null string. While that does create a new API method I think I favor that over promoting null strings in a single API of geode::optional readString(...) where all calls would have to check the optional for a value when in normal C++ there should always be a value for std::string. -Jake On Wed, Dec 20, 2017 at 9:01 AM David Kimura wrote: > Given that C++17 is feature-complete, it seems prudent to adopt those > patterns as much as possible. > > My fear with #4 is that we're introducing a special case API that users > become accustomed to and then unwilling to later switch to std::optional. > And then we'd end up supporting multiple ways to do the same thing. And > even if we can delete the special case API after C++17, there's still API > churn for our users. Because of that, I favor using optional even if we > have to roll our own. And then after C++17 we change our implementation to > alias std::optional and there shouldn't be any code changes required from > user point of view. > > Thanks, > David > > On Tue, Dec 19, 2017 at 2:30 PM, Jacob Barrett > wrote: > > > On Tue, Dec 19, 2017 at 1:52 PM Michael William Dodge > > > wrote: > > > > > Of those, #1 seems better than #2 and #3. Is there an elegant way to > use > > a > > > sentinel value of std::string to represent null, e.g., static const > > > PdxReader::NULL_STRING = ? > > > > > > > I am absolutely opposed to sentinel values just from the standpoint that > > once you pick a value someone will want that value. > > > > The C++17 solution to this problem is std::optional, but > > unfortunately C++11 is our minimum. We could embrace boost::optional (for > > which C++17 std::optional derives), but this will cause clients to also > > depend on boost, which isn't ideal. We could roll our own optional... > > > > -Jake > > >
[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #772 was SUCCESSFUL (with 2324 tests)
--- Spring Data GemFire > Nightly-ApacheGeode > #772 was successful. --- Scheduled 2326 tests in total. https://build.spring.io/browse/SGF-NAG-772/ -- This message is automatically generated by Atlassian Bamboo
Geode unit tests completed in '/' with non-zero exit code
Pipeline results can be found at: Concourse: https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/DistributedTest/builds/41
Geode unit tests completed in 'develop/DistributedTest' with non-zero exit code
Pipeline results can be found at: Concourse: https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/DistributedTest/builds/42
