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/01/14 08:49:58 UTC
[GitHub] [flink] dawidwys opened a new pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
dawidwys opened a new pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850
## Brief change log
We build a separate jar for BashJavaUtils with bundled log4j classes and
a configuration that logs to the stdout. Using this jar we run the
utility to capture the output. Out of the last line we extract jvm
parameters. The rest we pass as an environment variable (INHERITED_LOGS)
to the TaskManager process.
As part of the EnvironmentInformation printing we log whatever was
passed through the INHERITED_LOGS in the TM/JM process.
## Verifying this change
* BashJavaUtilsITCase should still run
* Start a taskmanager process, see the additional logs from the BashJavaUtils at the very top.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (**yes** / no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
- The serializers: (yes / **no** / don't know)
- The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / **no** / don't know)
- The S3 file system connector: (yes / **no** / don't know)
## Documentation
- Does this pull request introduce a new feature? (yes / **no**)
- If yes, how is the feature documented? (**not applicable** / docs / JavaDocs / not documented)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r368548666
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -603,38 +623,32 @@ TMSlaves() {
runBashJavaUtilsCmd() {
local cmd=$1
- local class_path=$2
- local conf_dir=$3
- local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+ local conf_dir=$2
+ local class_path="${3:-$FLINK_BIN_DIR/bash-java-utils.jar:`findFlinkDistJar`}"
Review comment:
`findFlinkDistJar` will already add `:`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r367485549
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/util/EnvironmentInformation.java
##########
@@ -270,14 +270,22 @@ public static void logEnvironmentInfo(Logger log, String componentName, String[]
if (log.isInfoEnabled()) {
RevisionInformation rev = getRevisionInformation();
String version = getVersion();
-
+
String jvmVersion = getJvmVersion();
String[] options = getJvmStartupOptionsArray();
-
+
String javaHome = System.getenv("JAVA_HOME");
-
+
+ String inheritedLogs = System.getenv("INHERITED_LOGS");
Review comment:
I would add a project specific prefix to the environment variable to avoid potential conflicts, i.e., `FLINK_INHERITED_LOGS`. WDYT?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
dawidwys commented on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-576149530
@GJL I applied your comments. Could you have another look?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144288064 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144310946 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
Hash:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144966288 TriggerType:PUSH TriggerID:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589
Hash:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4446 TriggerType:PUSH TriggerID:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144288064) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320)
* f3dc16d5020c2140439a6980cb7f2642218b8ad4 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144310946) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330)
* 84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144966288) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4446)
<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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r368013126
##########
File path: flink-dist/src/main/resources/log4j-bash-utils.properties
##########
@@ -0,0 +1,24 @@
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+
+# Logging configuration for BashJavaUtils utility
+log4j.rootLogger=INFO, console
+log4j.appender.console=org.apache.log4j.ConsoleAppender
+log4j.appender.console.layout=org.apache.log4j.PatternLayout
+log4j.appender.console.layout.ConversionPattern=%x - %m%n
Review comment:
ok
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144288064 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144288064) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320)
* f3dc16d5020c2140439a6980cb7f2642218b8ad4 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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r369525706
##########
File path: flink-dist/src/main/flink-bin/bin/taskmanager.sh
##########
@@ -49,20 +49,33 @@ if [[ $STARTSTOP == "start" ]] || [[ $STARTSTOP == "start-foreground" ]]; then
# Startup parameters
- jvm_params=$(getTmResourceJvmParams)
+ jvm_params_output=`runBashJavaUtilsCmd GET_TM_RESOURCE_JVM_PARAMS ${FLINK_CONF_DIR}`
+ jvm_params=`extractExecutionParams "$jvm_params_output"`
if [[ $? -ne 0 ]]; then
echo "[ERROR] Could not get JVM parameters properly."
exit 1
fi
export JVM_ARGS="${JVM_ARGS} ${jvm_params}"
IFS=$" "
- dynamic_configs=($(getTmResourceDynamicConfigs))
+
+ dynamic_configs_output=`runBashJavaUtilsCmd GET_TM_RESOURCE_DYNAMIC_CONFIGS ${FLINK_CONF_DIR}`
+ dynamic_configs=`extractExecutionParams "$dynamic_configs_output"`
Review comment:
Tracked by: https://issues.apache.org/jira/browse/FLINK-15727
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] tillrohrmann commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r369151878
##########
File path: flink-dist/src/main/flink-bin/bin/taskmanager.sh
##########
@@ -49,20 +49,33 @@ if [[ $STARTSTOP == "start" ]] || [[ $STARTSTOP == "start-foreground" ]]; then
# Startup parameters
- jvm_params=$(getTmResourceJvmParams)
+ jvm_params_output=`runBashJavaUtilsCmd GET_TM_RESOURCE_JVM_PARAMS ${FLINK_CONF_DIR}`
+ jvm_params=`extractExecutionParams "$jvm_params_output"`
if [[ $? -ne 0 ]]; then
echo "[ERROR] Could not get JVM parameters properly."
exit 1
fi
export JVM_ARGS="${JVM_ARGS} ${jvm_params}"
IFS=$" "
- dynamic_configs=($(getTmResourceDynamicConfigs))
+
+ dynamic_configs_output=`runBashJavaUtilsCmd GET_TM_RESOURCE_DYNAMIC_CONFIGS ${FLINK_CONF_DIR}`
+ dynamic_configs=`extractExecutionParams "$dynamic_configs_output"`
Review comment:
This is out of scope for this PR but I'm wondering why we need to call the utility twice. This will generate almost the same diagnostic output twice which will appear in the main process logs. I'd suggest to rework this as a follow up.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] tillrohrmann commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r369144001
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -603,38 +623,32 @@ TMSlaves() {
runBashJavaUtilsCmd() {
local cmd=$1
- local class_path=$2
- local conf_dir=$3
- local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+ local conf_dir=$2
+ local class_path="${3:-$FLINK_BIN_DIR/bash-java-utils.jar:`findFlinkDistJar`}"
+ class_path=`manglePathList ${class_path}`
- local output=`${JAVA_RUN} -classpath ${class_path} org.apache.flink.runtime.util.BashJavaUtils ${cmd} --configDir ${conf_dir} | tail -n 1`
+ local output=`${JAVA_RUN} -classpath ${class_path} org.apache.flink.runtime.util.BashJavaUtils ${cmd} --configDir ${conf_dir} | tail -n 1000`
if [[ $? -ne 0 ]]; then
echo "[ERROR] Cannot run BashJavaUtils to execute command ${cmd}." 1>&2
# Print the output in case the user redirect the log to console.
echo $output 1>&2
exit 1
fi
- if ! [[ $output =~ ^${EXECUTION_PREFIX}.* ]]; then
- echo "[ERROR] Unexpected result: $output" 1>&2
+ echo "$output"
+}
+
+extractExecutionParams() {
+ local output=$1
+ local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+
+ local execution_config=`echo "$output" | tail -n 1`
+ if ! [[ $execution_config =~ ^${EXECUTION_PREFIX}.* ]]; then
+ echo "[ERROR] Unexpected result: $execution_config" 1>&2
echo "[ERROR] The last line of the BashJavaUtils outputs is expected to be the execution result, following the prefix '${EXECUTION_PREFIX}'" 1>&2
echo $output 1>&2
Review comment:
```suggestion
echo "$output" 1>&2
```
in order to maintain new lines.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r368001097
##########
File path: flink-dist/src/main/resources/log4j-bash-utils.properties
##########
@@ -0,0 +1,24 @@
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+
+# Logging configuration for BashJavaUtils utility
+log4j.rootLogger=INFO, console
+log4j.appender.console=org.apache.log4j.ConsoleAppender
+log4j.appender.console.layout=org.apache.log4j.PatternLayout
+log4j.appender.console.layout.ConversionPattern=%x - %m%n
Review comment:
I am not sure about it. I am a bit afraid that 1000 characters per message might not be enough e.g. for exceptions.
I think it is highly unlikely that we will hit the environment variable size limit with 2000 lines (not messages).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144288064 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144310946 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
Hash:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144966288 TriggerType:PUSH TriggerID:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589
Hash:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589 Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4446 TriggerType:PUSH TriggerID:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144288064) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320)
* f3dc16d5020c2140439a6980cb7f2642218b8ad4 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144310946) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330)
* 84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144966288) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4446)
<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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r368520583
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -22,7 +22,7 @@ constructFlinkClassPath() {
local FLINK_CLASSPATH
while read -d '' -r jarfile ; do
- if [[ "$jarfile" =~ .*flink-dist.*.jar ]]; then
+ if [[ "$jarfile" =~ .*/flink-dist[^/]*.jar$ ]]; then
Review comment:
ok
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/144288064 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/144288064) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320)
<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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144288064 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144310946 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
Hash:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589 Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144288064) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320)
* f3dc16d5020c2140439a6980cb7f2642218b8ad4 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144310946) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330)
* 84c3ca6cd1fc8f4a9edbed0c698d1c3f73a7f589 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
With regards,
Apache Git Services
[GitHub] [flink] tillrohrmann commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r369150944
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -603,38 +623,32 @@ TMSlaves() {
runBashJavaUtilsCmd() {
local cmd=$1
- local class_path=$2
- local conf_dir=$3
- local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+ local conf_dir=$2
+ local class_path="${3:-$FLINK_BIN_DIR/bash-java-utils.jar:`findFlinkDistJar`}"
+ class_path=`manglePathList ${class_path}`
- local output=`${JAVA_RUN} -classpath ${class_path} org.apache.flink.runtime.util.BashJavaUtils ${cmd} --configDir ${conf_dir} | tail -n 1`
+ local output=`${JAVA_RUN} -classpath ${class_path} org.apache.flink.runtime.util.BashJavaUtils ${cmd} --configDir ${conf_dir} | tail -n 1000`
Review comment:
```suggestion
local output=`${JAVA_RUN} -classpath ${class_path} org.apache.flink.runtime.util.BashJavaUtils ${cmd} --configDir ${conf_dir} 2>&1 | tail -n 1000`
```
In order to make the exception appear in order with the other logging statements.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
dawidwys commented on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574068645
Could you have a look @azagrebin @tillrohrmann whenever you have time.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144288064 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/144310946 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144288064) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320)
* f3dc16d5020c2140439a6980cb7f2642218b8ad4 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/144310946) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330)
<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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r368549395
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -42,6 +42,26 @@ constructFlinkClassPath() {
echo "$FLINK_CLASSPATH""$FLINK_DIST"
}
+findFlinkDistJar() {
+ local FLINK_DIST
+
+ while read -d '' -r jarfile ; do
Review comment:
Wouldn't it be easier to run one of the following?
```
$ ls lib/flink-dist_*.jar
lib/flink-dist_2.11-1.11-SNAPSHOT.jar
```
or
```
$ find lib -name 'flink-dist*.jar'
lib/flink-dist_2.11-1.11-SNAPSHOT.jar
```
If I understand correctly `FLINK_DIST` should contain only one jar, and there are no other variables that are updated. Hence, the while loop should not be needed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r367889265
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -22,7 +22,7 @@ constructFlinkClassPath() {
local FLINK_CLASSPATH
while read -d '' -r jarfile ; do
- if [[ "$jarfile" =~ .*flink-dist.*.jar ]]; then
+ if [[ "$jarfile" =~ .*/flink-dist[^/]*.jar$ ]]; then
Review comment:
I did not run into problems with class loading, but the resulting classpath was different from what is expected by this method.
If I understand this method correctly, it tries to put the dist jar at the end of the class path and the `FLINK_DIST` variable should contain the `flink-dist.jar` not the whole classpath.
Because I run it within flink source directory within the `flink-dist` module, all jars from the `lib` directory ended up in the `FLINK_DIST` variable, because the old regex matches:
..../**flink-dist**/target/flink-1.11-SNAPSHOT-bin/flink-1.11-SNAPSHOT/lib/*.**jar**
The new regex matches only within the last part of the path.
..../flink-dist/target/flink-1.11-SNAPSHOT-bin/flink-1.11-SNAPSHOT/lib/log4j-1.2.17.jar <- does not match
..../flink-dist/target/flink-1.11-SNAPSHOT-bin/flink-1.11-SNAPSHOT/lib/**flink-dist_2.11-1.11-SNAPSHOT.jar** <- matches
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r369417080
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -603,38 +623,32 @@ TMSlaves() {
runBashJavaUtilsCmd() {
local cmd=$1
- local class_path=$2
- local conf_dir=$3
- local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+ local conf_dir=$2
+ local class_path="${3:-$FLINK_BIN_DIR/bash-java-utils.jar:`findFlinkDistJar`}"
Review comment:
According to the other comment I reworked the findFlinkDistJar method. It does not longer return the `:`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on issue #10850: [FLINK-15519][configuration]
Preserve logs from BashJavaUtils and make them part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574760492
I can confirm that the solution works. On my machine the log is rendered as
```
2020-01-15 18:05:11,552 INFO org.apache.flink.runtime.taskexecutor.TaskManagerRunner - --------------------------------------------------------------------------------
2020-01-15 18:05:11,554 INFO org.apache.flink.runtime.taskexecutor.TaskManagerRunner - Preconfiguration:
2020-01-15 18:05:11,554 INFO org.apache.flink.runtime.taskexecutor.TaskManagerRunner -
TM_RESOURCES_JVM_PARAMS extraction logs:
- Loading configuration property: jobmanager.rpc.address, localhost
- Loading configuration property: jobmanager.rpc.port, 6123
- Loading configuration property: jobmanager.heap.size, 1024m
- Loading configuration property: taskmanager.memory.process.size, 1024m
- Loading configuration property: taskmanager.numberOfTaskSlots, 1
- Loading configuration property: parallelism.default, 1
- Loading configuration property: jobmanager.execution.failover-strategy, region
- The derived from fraction jvm overhead memory (107374184b) is less than its min value 134217728 bytes, max value will be used instead
BASH_JAVA_UTILS_EXEC_RESULT:-Xmx268435450 -Xms268435450 -XX:MaxDirectMemorySize=214748366 -XX:MaxMetaspaceSize=134217728
TM_RESOURCES_DYNAMIC_CONFIGS extraction logs:
- Loading configuration property: jobmanager.rpc.address, localhost
- Loading configuration property: jobmanager.rpc.port, 6123
- Loading configuration property: jobmanager.heap.size, 1024m
- Loading configuration property: taskmanager.memory.process.size, 1024m
- Loading configuration property: taskmanager.numberOfTaskSlots, 1
- Loading configuration property: parallelism.default, 1
- Loading configuration property: jobmanager.execution.failover-strategy, region
- The derived from fraction jvm overhead memory (107374184b) is less than its min value 134217728 bytes, max value will be used instead
BASH_JAVA_UTILS_EXEC_RESULT:-D taskmanager.memory.shuffle.max=80530638b -D taskmanager.memory.framework.off-heap.size=134217728b -D taskmanager.memory.framework.heap.size=134217728b -D taskmanager.memory.managed.size=322122552b -D taskmanager.cpu.cores=1.0 -D taskmanager.memory.task.heap.size=134217722b -D taskmanager.memory.task.off-heap.size=0b -D taskmanager.memory.shuffle.min=80530638b
2020-01-15 18:05:11,554 INFO org.apache.flink.runtime.taskexecutor.TaskManagerRunner - --------------------------------------------------------------------------------
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144288064 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144288064) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320)
<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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r368548441
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -603,38 +623,32 @@ TMSlaves() {
runBashJavaUtilsCmd() {
local cmd=$1
- local class_path=$2
- local conf_dir=$3
- local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+ local conf_dir=$2
+ local class_path="${3:-$FLINK_BIN_DIR/bash-java-utils.jar:`findFlinkDistJar`}"
Review comment:
`findFlinkDistJar` will already add `:`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] tillrohrmann commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r369100714
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -42,6 +42,26 @@ constructFlinkClassPath() {
echo "$FLINK_CLASSPATH""$FLINK_DIST"
}
+findFlinkDistJar() {
+ local FLINK_DIST
+
+ while read -d '' -r jarfile ; do
Review comment:
I agree with Gary. The current logic seems a bit too complicated for the job it is doing.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574069960
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 f902fd5c6cc693bac289427094b53d9e11114274 (Tue Jan 14 08:54:04 UTC 2020)
**Warnings:**
* **1 pom.xml files were touched**: Check for build and licensing issues.
* 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
With regards,
Apache Git Services
[GitHub] [flink] tillrohrmann commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r369144197
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -603,38 +623,32 @@ TMSlaves() {
runBashJavaUtilsCmd() {
local cmd=$1
- local class_path=$2
- local conf_dir=$3
- local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+ local conf_dir=$2
+ local class_path="${3:-$FLINK_BIN_DIR/bash-java-utils.jar:`findFlinkDistJar`}"
+ class_path=`manglePathList ${class_path}`
- local output=`${JAVA_RUN} -classpath ${class_path} org.apache.flink.runtime.util.BashJavaUtils ${cmd} --configDir ${conf_dir} | tail -n 1`
+ local output=`${JAVA_RUN} -classpath ${class_path} org.apache.flink.runtime.util.BashJavaUtils ${cmd} --configDir ${conf_dir} | tail -n 1000`
if [[ $? -ne 0 ]]; then
echo "[ERROR] Cannot run BashJavaUtils to execute command ${cmd}." 1>&2
# Print the output in case the user redirect the log to console.
echo $output 1>&2
Review comment:
```suggestion
echo "$output" 1>&2
```
in order to maintain new lines.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r366998778
##########
File path: flink-dist/src/main/resources/log4j-bash-utils.properties
##########
@@ -0,0 +1,24 @@
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+
+# Logging configuration for BashJavaUtils utility
+log4j.rootLogger=INFO, console
+log4j.appender.console=org.apache.log4j.ConsoleAppender
+log4j.appender.console.layout=org.apache.log4j.PatternLayout
+log4j.appender.console.layout.ConversionPattern=%x - %m%n
Review comment:
Should we add something like `%.1000m`? We can be sure that the environment variable will be at most ~2mb.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r369417334
##########
File path: flink-dist/src/main/flink-bin/bin/taskmanager.sh
##########
@@ -49,20 +49,33 @@ if [[ $STARTSTOP == "start" ]] || [[ $STARTSTOP == "start-foreground" ]]; then
# Startup parameters
- jvm_params=$(getTmResourceJvmParams)
+ jvm_params_output=`runBashJavaUtilsCmd GET_TM_RESOURCE_JVM_PARAMS ${FLINK_CONF_DIR}`
+ jvm_params=`extractExecutionParams "$jvm_params_output"`
if [[ $? -ne 0 ]]; then
echo "[ERROR] Could not get JVM parameters properly."
exit 1
fi
export JVM_ARGS="${JVM_ARGS} ${jvm_params}"
IFS=$" "
- dynamic_configs=($(getTmResourceDynamicConfigs))
+
+ dynamic_configs_output=`runBashJavaUtilsCmd GET_TM_RESOURCE_DYNAMIC_CONFIGS ${FLINK_CONF_DIR}`
+ dynamic_configs=`extractExecutionParams "$dynamic_configs_output"`
Review comment:
I think this is a good suggestion.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r367490800
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -603,38 +623,32 @@ TMSlaves() {
runBashJavaUtilsCmd() {
local cmd=$1
- local class_path=$2
- local conf_dir=$3
- local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+ local conf_dir=$2
+ local class_path="${3:-$FLINK_BIN_DIR/bash-java-utils.jar:`findFlinkDistJar`}"
+ class_path=`manglePathList ${class_path}`
- local output=`${JAVA_RUN} -classpath ${class_path} org.apache.flink.runtime.util.BashJavaUtils ${cmd} --configDir ${conf_dir} | tail -n 1`
+ local output=`${JAVA_RUN} -classpath ${class_path} org.apache.flink.runtime.util.BashJavaUtils ${cmd} --configDir ${conf_dir} | tail -n 1000`
if [[ $? -ne 0 ]]; then
echo "[ERROR] Cannot run BashJavaUtils to execute command ${cmd}." 1>&2
# Print the output in case the user redirect the log to console.
echo $output 1>&2
exit 1
fi
- if ! [[ $output =~ ^${EXECUTION_PREFIX}.* ]]; then
- echo "[ERROR] Unexpected result: $output" 1>&2
+ echo "$output"
+}
+
+extractExecutionParams() {
+ local output=$1
+ local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+
+ local execution_config=`echo "$output" | tail -n 1`
+ if ! [[ $execution_config =~ ^${EXECUTION_PREFIX}.* ]]; then
+ echo "[ERROR] Unexpected result: $execution_config" 1>&2
echo "[ERROR] The last line of the BashJavaUtils outputs is expected to be the execution result, following the prefix '${EXECUTION_PREFIX}'" 1>&2
echo $output 1>&2
exit 1
fi
- echo ${output} | sed "s/$EXECUTION_PREFIX//g"
-}
-
-getTmResourceJvmParams() {
- local class_path=`constructFlinkClassPath`
- class_path=`manglePathList ${class_path}`
-
- runBashJavaUtilsCmd GET_TM_RESOURCE_JVM_PARAMS ${class_path} ${FLINK_CONF_DIR}
-}
-
-getTmResourceDynamicConfigs() {
- local class_path=`constructFlinkClassPath`
- class_path=`manglePathList ${class_path}`
-
- runBashJavaUtilsCmd GET_TM_RESOURCE_DYNAMIC_CONFIGS ${class_path} ${FLINK_CONF_DIR}
+ echo ${execution_config} | sed "s/$EXECUTION_PREFIX//g"
Review comment:
I think `/g` (global replacement) is not needed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r367868956
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -22,7 +22,7 @@ constructFlinkClassPath() {
local FLINK_CLASSPATH
while read -d '' -r jarfile ; do
- if [[ "$jarfile" =~ .*flink-dist.*.jar ]]; then
+ if [[ "$jarfile" =~ .*/flink-dist[^/]*.jar$ ]]; then
Review comment:
Did you run into issues with the old regex?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot commented on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144288064 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/144310946 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
Hash:f3dc16d5020c2140439a6980cb7f2642218b8ad4 Status:PENDING URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330 TriggerType:PUSH TriggerID:f3dc16d5020c2140439a6980cb7f2642218b8ad4
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144288064) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320)
* f3dc16d5020c2140439a6980cb7f2642218b8ad4 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/144310946) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4330)
<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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r367490505
##########
File path: flink-dist/pom.xml
##########
@@ -697,6 +698,60 @@ under the License.
<id>shade-flink</id>
<phase>none</phase>
</execution>
+ <!--Build BashJavaUtils jar-->
Review comment:
nit:
```
<!-- Build BashJavaUtils jar -->
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r368548441
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -603,38 +623,32 @@ TMSlaves() {
runBashJavaUtilsCmd() {
local cmd=$1
- local class_path=$2
- local conf_dir=$3
- local EXECUTION_PREFIX="BASH_JAVA_UTILS_EXEC_RESULT:"
+ local conf_dir=$2
+ local class_path="${3:-$FLINK_BIN_DIR/bash-java-utils.jar:`findFlinkDistJar`}"
Review comment:
`findFlinkDistJar` will already add `:`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] dawidwys closed pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
dawidwys closed pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] GJL commented on a change in pull request #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#discussion_r368549395
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -42,6 +42,26 @@ constructFlinkClassPath() {
echo "$FLINK_CLASSPATH""$FLINK_DIST"
}
+findFlinkDistJar() {
+ local FLINK_DIST
+
+ while read -d '' -r jarfile ; do
Review comment:
Wouldn't it be easier to run one of the following?
```
$ ls lib/flink-dist_*.jar
lib/flink-dist_2.11-1.11-SNAPSHOT.jar
```
or
```
$ find lib -name 'flink-dist*.jar'
lib/flink-dist_2.11-1.11-SNAPSHOT.jar
```
If I understand correctly `FLINK_DIST` should contain only one jar.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [flink] flinkbot edited a comment on issue #10850:
[FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them
part of TM logs
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #10850: [FLINK-15519][configuration] Preserve logs from BashJavaUtils and make them part of TM logs
URL: https://github.com/apache/flink/pull/10850#issuecomment-574076308
<!--
Meta data
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/144288064 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
Hash:f902fd5c6cc693bac289427094b53d9e11114274 Status:FAILURE URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320 TriggerType:PUSH TriggerID:f902fd5c6cc693bac289427094b53d9e11114274
-->
## CI report:
* f902fd5c6cc693bac289427094b53d9e11114274 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/144288064) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=4320)
<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
With regards,
Apache Git Services