You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/08/27 07:21:24 UTC
[GitHub] [flink] rmetzger opened a new pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
rmetzger opened a new pull request #13260:
URL: https://github.com/apache/flink/pull/13260
This experimental change aims to make the watchdog more accurate by extending the monitoring from the output to log files produced by the tests.
In particular the S3 tests seem to produce no output while they are producing output to the log files.
The risk of this change is that a hanging test is still producing log output (e.g. the test is stuck in a retry-loop).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13260:
URL: https://github.com/apache/flink/pull/13260#issuecomment-681681113
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 858238cb47175439c563bc2c6d23ded57b1ab6f2 (Thu Aug 27 07:23:54 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rmetzger commented on a change in pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
rmetzger commented on a change in pull request #13260:
URL: https://github.com/apache/flink/pull/13260#discussion_r481143381
##########
File path: tools/ci/test_controller.sh
##########
@@ -69,6 +69,10 @@ export JAVA_TOOL_OPTIONS="-XX:+HeapDumpOnOutOfMemoryError"
# some tests provide additional logs if they find this variable
export IS_CI=true
+export WATCHDOG_ADDITIONAL_MONITORING_FILES="$DEBUG_FILES_OUTPUT_DIR/mvn-*.log"
Review comment:
Somehow I consider "source" something like an "import" statement, that doesn't support passing variables.
And this seems to be a feature only available in bash: https://unix.stackexchange.com/questions/5024/passing-variables-to-a-bash-script-when-sourcing-it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rmetzger commented on a change in pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
rmetzger commented on a change in pull request #13260:
URL: https://github.com/apache/flink/pull/13260#discussion_r481137022
##########
File path: tools/ci/watchdog.sh
##########
@@ -38,8 +38,26 @@ CMD_EXIT="/tmp/watchdog.exit"
# Utility functions
# =============================================
+max_of() {
+ local max number
+
+ max="$1"
+
+ for number in "${@:2}"; do
+ if ((number > max)); then
+ max="$number"
+ fi
+ done
+
+ printf '%d\n' "$max"
+}
+
+# Returns the highest modification time out of $CMD_OUT (which is the command output file)
+# and any file(s) named "mvn-*.log" (which are logging files created by Flink's tests)
mod_time () {
- echo `stat -c "%Y" $CMD_OUT`
+ CMD_OUT_MOD_TIME=`stat -c "%Y" $CMD_OUT`
+ ADDITIONAL_FILES_MOD_TIMES=`stat -c "%Y" $WATCHDOG_ADDITIONAL_MONITORING_FILES 2> /dev/null`
+ echo `max_of $CMD_OUT_MOD_TIME $ADDITIONAL_FILES_MOD_TIMES`
Review comment:
damn, sometimes, I'm overthinking it. Thanks for the pointer.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13260:
URL: https://github.com/apache/flink/pull/13260#issuecomment-681696040
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "858238cb47175439c563bc2c6d23ded57b1ab6f2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "858238cb47175439c563bc2c6d23ded57b1ab6f2",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 858238cb47175439c563bc2c6d23ded57b1ab6f2 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] XComp commented on a change in pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
XComp commented on a change in pull request #13260:
URL: https://github.com/apache/flink/pull/13260#discussion_r481144126
##########
File path: tools/ci/watchdog.sh
##########
@@ -38,8 +38,26 @@ CMD_EXIT="/tmp/watchdog.exit"
# Utility functions
# =============================================
+max_of() {
+ local max number
+
+ max="$1"
+
+ for number in "${@:2}"; do
+ if ((number > max)); then
+ max="$number"
+ fi
+ done
+
+ printf '%d\n' "$max"
Review comment:
:-D I like how the accepted answer feels to be 10 pages long for something I considered a minor thing ^^
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13260:
URL: https://github.com/apache/flink/pull/13260#issuecomment-681696040
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "858238cb47175439c563bc2c6d23ded57b1ab6f2",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=5914",
"triggerID" : "858238cb47175439c563bc2c6d23ded57b1ab6f2",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 858238cb47175439c563bc2c6d23ded57b1ab6f2 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=5914)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] XComp commented on a change in pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
XComp commented on a change in pull request #13260:
URL: https://github.com/apache/flink/pull/13260#discussion_r478375435
##########
File path: tools/ci/watchdog.sh
##########
@@ -38,8 +38,26 @@ CMD_EXIT="/tmp/watchdog.exit"
# Utility functions
# =============================================
+max_of() {
+ local max number
+
+ max="$1"
+
+ for number in "${@:2}"; do
+ if ((number > max)); then
+ max="$number"
+ fi
+ done
+
+ printf '%d\n' "$max"
Review comment:
I forgot to ask here: Is there a reason why you didn't just use `echo "$max"` instead?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rmetzger commented on a change in pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
rmetzger commented on a change in pull request #13260:
URL: https://github.com/apache/flink/pull/13260#discussion_r481140585
##########
File path: tools/ci/watchdog.sh
##########
@@ -38,8 +38,26 @@ CMD_EXIT="/tmp/watchdog.exit"
# Utility functions
# =============================================
+max_of() {
+ local max number
+
+ max="$1"
+
+ for number in "${@:2}"; do
+ if ((number > max)); then
+ max="$number"
+ fi
+ done
+
+ printf '%d\n' "$max"
Review comment:
Well, my code is (of course) inspired by Stackoverflow.
But there seems to be some arguments why printf is better than echo: https://unix.stackexchange.com/questions/65803/why-is-printf-better-than-echo
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13260:
URL: https://github.com/apache/flink/pull/13260#issuecomment-681696040
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "858238cb47175439c563bc2c6d23ded57b1ab6f2",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=5914",
"triggerID" : "858238cb47175439c563bc2c6d23ded57b1ab6f2",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 858238cb47175439c563bc2c6d23ded57b1ab6f2 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=5914)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rmetzger merged pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
rmetzger merged pull request #13260:
URL: https://github.com/apache/flink/pull/13260
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] rmetzger commented on pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
rmetzger commented on pull request #13260:
URL: https://github.com/apache/flink/pull/13260#issuecomment-684864947
Thanks for the review!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] XComp commented on a change in pull request #13260: [FLINK-16768][tests] Let the watchdog also monitor mvn logs
Posted by GitBox <gi...@apache.org>.
XComp commented on a change in pull request #13260:
URL: https://github.com/apache/flink/pull/13260#discussion_r478365211
##########
File path: tools/ci/test_controller.sh
##########
@@ -69,6 +69,10 @@ export JAVA_TOOL_OPTIONS="-XX:+HeapDumpOnOutOfMemoryError"
# some tests provide additional logs if they find this variable
export IS_CI=true
+export WATCHDOG_ADDITIONAL_MONITORING_FILES="$DEBUG_FILES_OUTPUT_DIR/mvn-*.log"
Review comment:
Wouldn't it be better to pass this variable as a parameter to `source "${HERE}/watchdog.sh"`? Of course, we would have to handle the variable in `watchdog.sh` properly, then, since it has to be accessed through `$1`. The variable `$WATCHDOG_ADDITIONAL_MONITORING_FILES` is nowhere else used.
##########
File path: tools/ci/watchdog.sh
##########
@@ -38,8 +38,26 @@ CMD_EXIT="/tmp/watchdog.exit"
# Utility functions
# =============================================
+max_of() {
+ local max number
+
+ max="$1"
+
+ for number in "${@:2}"; do
+ if ((number > max)); then
+ max="$number"
+ fi
+ done
+
+ printf '%d\n' "$max"
+}
+
+# Returns the highest modification time out of $CMD_OUT (which is the command output file)
+# and any file(s) named "mvn-*.log" (which are logging files created by Flink's tests)
mod_time () {
- echo `stat -c "%Y" $CMD_OUT`
+ CMD_OUT_MOD_TIME=`stat -c "%Y" $CMD_OUT`
+ ADDITIONAL_FILES_MOD_TIMES=`stat -c "%Y" $WATCHDOG_ADDITIONAL_MONITORING_FILES 2> /dev/null`
+ echo `max_of $CMD_OUT_MOD_TIME $ADDITIONAL_FILES_MOD_TIMES`
Review comment:
It looks alright. FYI: The following one-liner should do the same:
```
mod_time() {
stat -c "%Y" "$CMD_OUT $WATCHDOG_ADDITIONAL_MONITORING_FILES" | sort -nr | head -1
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org