zentol commented on a change in pull request #11839:
URL: https://github.com/apache/flink/pull/11839#discussion_r415757517
##########
File path: flink-dist/src/main/flink-bin/bin/flink-console.sh
##########
@@ -58,7 +58,19 @@ esac
FLINK_TM_CLASSPATH=`constructFlinkClassPath`
-log_setting=("-Dlog4j.configuration=file:${FLINK_CONF_DIR}/log4j-console.properties"
"-Dlog4j.configurationFile=file:${FLINK_CONF_DIR}/log4j-console.properties"
"-Dlogback.configurationFile=file:${FLINK_CONF_DIR}/logback-console.xml")
+if [ "$FLINK_IDENT_STRING" = "" ]; then
+ FLINK_IDENT_STRING="$USER"
+fi
+
+pid=$FLINK_PID_DIR/flink-foreground-$FLINK_IDENT_STRING-$SERVICE.pid
+id=$(getFlinkDaemonID $pid 0)
+
+FLINK_LOG_PREFIX="${FLINK_LOG_DIR}/flink-foreground-${FLINK_IDENT_STRING}-${SERVICE}-${id}-${HOSTNAME}"
Review comment:
I see no reason why we should have this in the name.
```suggestion
FLINK_LOG_PREFIX="${FLINK_LOG_DIR}/flink-${FLINK_IDENT_STRING}-${SERVICE}-${id}-${HOSTNAME}"
```
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -660,3 +660,39 @@ extractExecutionParams() {
echo ${execution_config} | sed "s/$EXECUTION_PREFIX//"
}
+
+getFlinkDaemonID() {
+ local pid=$1
+ local clean_up_when_all_processes_dead=${2:-1}
+
+ mkdir -p "$FLINK_PID_DIR"
+
+ # Log files for daemons are indexed from the process ID's position in the
PID
+ # file. The following lock prevents a race condition during daemon startup
+ # when multiple daemons read, index, and write to the PID file
concurrently.
+ # The lock is created on the PID directory since a lock file cannot be
safely
+ # removed. The daemon is started with the lock closed and the lock remains
+ # active in this script until the script exits.
+ command -v flock >/dev/null 2>&1
+ if [[ $? -eq 0 ]]; then
+ exec 200<"$FLINK_PID_DIR"
+ flock 200
+ fi
+
+ # Remove the pid file when all the processes are dead
Review comment:
this surely is not a side-effect this method should have
##########
File path: flink-dist/src/main/flink-bin/bin/flink-console.sh
##########
@@ -58,7 +58,18 @@ esac
FLINK_TM_CLASSPATH=`constructFlinkClassPath`
-log_setting=("-Dlog4j.configuration=file:${FLINK_CONF_DIR}/log4j-console.properties"
"-Dlog4j.configurationFile=file:${FLINK_CONF_DIR}/log4j-console.properties"
"-Dlogback.configurationFile=file:${FLINK_CONF_DIR}/logback-console.xml")
+if [ "$FLINK_IDENT_STRING" = "" ]; then
+ FLINK_IDENT_STRING="$USER"
+fi
+
+RANDOM_ID=$(echo $RANDOM | tr '[0-9]' '[a-z]')
+
+FLINK_LOG_PREFIX="${FLINK_LOG_DIR}/flink-${FLINK_IDENT_STRING}-${SERVICE}-${RANDOM_ID}-${HOSTNAME}"
+log="${FLINK_LOG_PREFIX}.log"
+out="${FLINK_LOG_PREFIX}.out"
+err="${FLINK_LOG_PREFIX}.err"
Review comment:
> And i think it is better to separate them if we could.
That may be so, but the scripts should have a consistent behavior.
##########
File path: flink-dist/src/main/flink-bin/bin/config.sh
##########
@@ -660,3 +660,39 @@ extractExecutionParams() {
echo ${execution_config} | sed "s/$EXECUTION_PREFIX//"
}
+
+getFlinkDaemonID() {
+ local pid=$1
+ local clean_up_when_all_processes_dead=${2:-1}
+
+ mkdir -p "$FLINK_PID_DIR"
+
+ # Log files for daemons are indexed from the process ID's position in the
PID
+ # file. The following lock prevents a race condition during daemon startup
+ # when multiple daemons read, index, and write to the PID file
concurrently.
+ # The lock is created on the PID directory since a lock file cannot be
safely
+ # removed. The daemon is started with the lock closed and the lock remains
+ # active in this script until the script exits.
+ command -v flock >/dev/null 2>&1
+ if [[ $? -eq 0 ]]; then
+ exec 200<"$FLINK_PID_DIR"
+ flock 200
+ fi
+
+ # Remove the pid file when all the processes are dead
+ if [ -f "$pid" ];then
+ local all_dead=0
+ while read each_pid; do
+ kill -0 $each_pid > /dev/null 2>&1
+ if [[ $? -eq 0 ]] ; then
+ all_dead=1
+ fi
+ done < "$pid"
+ [ $clean_up_when_all_processes_dead -eq 0 -a $all_dead -eq 0 ] && rm
$pid
+ fi
+
+ # Ascending ID depending on number of lines in pid file.
+ # This allows us to start multiple daemon of each type.
+ local id=$([ -f "$pid" ] && echo $(wc -l < "$pid") || echo "0")
+ echo $id
Review comment:
Can we not simplify this?
```suggestion
echo $([ -f "$pid" ] && echo $(wc -l < "$pid") || echo "0")
```
----------------------------------------------------------------
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:
[email protected]
