Yicong-Huang commented on code in PR #6719:
URL: https://github.com/apache/texera/pull/6719#discussion_r3648435743


##########
.github/scripts/smoke-cluster.sh:
##########
@@ -0,0 +1,179 @@
+#!/usr/bin/env bash
+# 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.
+
+# smoke-cluster.sh -- boot a computing-unit-master and a computing-unit-worker
+# from their unpacked dist and assert the worker actually JOINS the master's
+# amber Pekko cluster, not merely that its process survives.
+#
+# Why this can't reuse smoke-boot.sh: ComputingUnitWorker.main only calls
+# AmberRuntime.startActorWorker, which binds 
pekko.remote.artery.canonical.port=0
+# (a random ephemeral port) and starts no HTTP server -- so there is no fixed
+# port to probe. A worker booted standalone just retries its seed forever and
+# stays up, so "did not crash" proves nothing. The real health signal is 
cluster
+# membership, which ClusterListener prints on every membership change:
+#     ---------Now we have N nodes in the cluster---------  (N = 
cluster.state.members.size)
+# master-alone => 1, master+worker joined => 2. We wait for the "2" line -- a
+# deliberate readiness signal the app emits, not a crash-scan (contrast #6332).
+# See issue #6523 (raised from the #6377 review) and #6220 for the rationale.
+#
+# Single host, no external network: the master runs in non-cluster mode, which
+# self-seeds pekko://Amber@localhost:2552; the worker is launched with NO
+# --serverAddr, so it defaults its seed to localhost:2552 and skips the
+# getNodeIpAddress -> http://checkip.amazonaws.com lookup (that only fires when
+# --serverAddr is supplied).
+#
+# Usage:
+#   smoke-cluster.sh <master-glob> <worker-glob> <master-http-port> 
[join_timeout]
+#
+# Requirements: run from the checkout root -- the master resolves its config 
via
+# Utils.amberHomePath -> 
./amber/src/main/resources/computing-unit-master-config.yml
+# -- with postgres already provisioned (the master calls 
SqlServer.initConnection).
+
+set -euo pipefail
+
+master_glob="${1:?usage: smoke-cluster.sh <master-glob> <worker-glob> 
<master-http-port> [join_timeout]}"
+worker_glob="${2:?worker launcher glob required}"
+http_port="${3:?master http port required}"
+join_timeout="${4:-40}"
+master_ready_timeout="${SMOKE_MASTER_READY_TIMEOUT:-60}"
+
+PHRASE_JOINED='Now we have 2 nodes in the cluster'

Review Comment:
   ClusterListener.scala:145-147 now backs a CI contract, but nothing at the 
Scala site says so — whoever rewords that log line only finds out from a red CI 
run (the drift hint helps them recover, but after the fact). A one-line comment 
at the log statement pointing at this script would close the loop.



##########
.github/scripts/smoke-cluster.sh:
##########
@@ -0,0 +1,179 @@
+#!/usr/bin/env bash
+# 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.
+
+# smoke-cluster.sh -- boot a computing-unit-master and a computing-unit-worker
+# from their unpacked dist and assert the worker actually JOINS the master's
+# amber Pekko cluster, not merely that its process survives.
+#
+# Why this can't reuse smoke-boot.sh: ComputingUnitWorker.main only calls
+# AmberRuntime.startActorWorker, which binds 
pekko.remote.artery.canonical.port=0
+# (a random ephemeral port) and starts no HTTP server -- so there is no fixed
+# port to probe. A worker booted standalone just retries its seed forever and
+# stays up, so "did not crash" proves nothing. The real health signal is 
cluster
+# membership, which ClusterListener prints on every membership change:
+#     ---------Now we have N nodes in the cluster---------  (N = 
cluster.state.members.size)
+# master-alone => 1, master+worker joined => 2. We wait for the "2" line -- a
+# deliberate readiness signal the app emits, not a crash-scan (contrast #6332).
+# See issue #6523 (raised from the #6377 review) and #6220 for the rationale.
+#
+# Single host, no external network: the master runs in non-cluster mode, which
+# self-seeds pekko://Amber@localhost:2552; the worker is launched with NO
+# --serverAddr, so it defaults its seed to localhost:2552 and skips the
+# getNodeIpAddress -> http://checkip.amazonaws.com lookup (that only fires when
+# --serverAddr is supplied).
+#
+# Usage:
+#   smoke-cluster.sh <master-glob> <worker-glob> <master-http-port> 
[join_timeout]
+#
+# Requirements: run from the checkout root -- the master resolves its config 
via
+# Utils.amberHomePath -> 
./amber/src/main/resources/computing-unit-master-config.yml
+# -- with postgres already provisioned (the master calls 
SqlServer.initConnection).
+
+set -euo pipefail
+
+master_glob="${1:?usage: smoke-cluster.sh <master-glob> <worker-glob> 
<master-http-port> [join_timeout]}"
+worker_glob="${2:?worker launcher glob required}"
+http_port="${3:?master http port required}"
+join_timeout="${4:-40}"
+master_ready_timeout="${SMOKE_MASTER_READY_TIMEOUT:-60}"
+
+PHRASE_JOINED='Now we have 2 nodes in the cluster'
+
+# Resolve a (globbed) launcher to exactly one executable -- same contract as
+# smoke-boot.sh: erroring on >1 match avoids booting the wrong binary.
+RESOLVED=""
+resolve_launcher() {
+  local glob="$1" role="$2" matches count
+  # shellcheck disable=SC2086  # intentional: $glob must glob-expand here
+  matches="$(ls -d $glob 2>/dev/null || true)"
+  count="$(printf '%s' "$matches" | grep -c . || true)"
+  if [[ "$count" -eq 0 ]]; then
+    echo "::error::smoke-cluster: $role launcher not found: $glob"; return 1
+  fi
+  if [[ "$count" -gt 1 ]]; then
+    echo "::error::smoke-cluster: $role launcher glob matched $count files, 
expected exactly 1: $glob"; return 1
+  fi
+  if [[ ! -x "$matches" ]]; then
+    echo "::error::smoke-cluster: $role launcher not executable: $matches"; 
return 1
+  fi
+  RESOLVED="$matches"
+}
+resolve_launcher "$master_glob" master || exit 1
+master_bin="$RESOLVED"
+resolve_launcher "$worker_glob" worker || exit 1
+worker_bin="$RESOLVED"
+
+port_open() {
+  # Probe 127.0.0.1 explicitly (not "localhost", which can resolve to ::1 first
+  # while the JVM binds IPv4, giving a false "not listening").
+  if command -v nc >/dev/null 2>&1; then
+    nc -z 127.0.0.1 "$http_port" >/dev/null 2>&1
+  else
+    (exec 3<>"/dev/tcp/127.0.0.1/$http_port") 2>/dev/null
+  fi
+}
+if port_open; then
+  echo "::error::smoke-cluster: port $http_port is already in use before 
launching the master"
+  exit 1
+fi
+
+master_log="$(mktemp)"
+worker_log="$(mktemp)"
+master_pid=""
+worker_pid=""
+
+alive() { kill -0 "$1" 2>/dev/null; }
+
+dump_logs() {
+  echo "----- master (last 120 lines) -----"; tail -n 120 "$master_log" 
2>/dev/null || true
+  echo "----- worker (last 120 lines) -----"; tail -n 120 "$worker_log" 
2>/dev/null || true
+}
+
+# Reap both JVMs (SIGTERM, grace, SIGKILL) and remove temp logs on any exit --
+# including a set -e abort -- so a failing run can't orphan a JVM or leak 
files.
+# shellcheck disable=SC2329  # invoked indirectly via `trap cleanup EXIT` below
+cleanup() {
+  for p in "$worker_pid" "$master_pid"; do [[ -n "$p" ]] && kill "$p" 
2>/dev/null || true; done
+  for _ in $(seq 1 10); do
+    { [[ -z "$master_pid" ]] || ! alive "$master_pid"; } &&
+      { [[ -z "$worker_pid" ]] || ! alive "$worker_pid"; } && break
+    sleep 1
+  done
+  for p in "$worker_pid" "$master_pid"; do [[ -n "$p" ]] && kill -9 "$p" 
2>/dev/null || true; done
+  wait 2>/dev/null || true
+  rm -f "$master_log" "$worker_log"
+}
+trap cleanup EXIT
+
+# 1) Master: non-cluster mode self-seeds localhost:2552 and serves Dropwizard 
on
+#    :http_port. Launch from the current dir (checkout root) so its config 
resolves.
+echo "smoke-cluster: launching master '$master_bin'"
+"$master_bin" >"$master_log" 2>&1 &
+master_pid=$!
+
+# Wait for the master to be ready: its HTTP port coming up implies the actor
+# system bound :2552 and DB init finished.
+ready=0
+for ((i = 0; i < master_ready_timeout; i++)); do
+  if port_open; then ready=1; break; fi
+  if ! alive "$master_pid"; then
+    echo "::error::smoke-cluster: master exited before listening on 
:$http_port"; dump_logs; exit 1

Review Comment:
   smoke-boot.sh reaps the dead child and reports "exited on boot (exit code 
$code)" — its comment calls the exit code "the signal". Here (and in the worker 
branch at line 164) the code is dropped, so a log-silent death (an OOM 
SIGKILL's 137) leaves a 120-line dump with no cause in it. `code=0; wait 
"$master_pid" 2>/dev/null || code=$?` before the error line restores it.



##########
.github/workflows/build.yml:
##########
@@ -639,19 +639,22 @@ jobs:
         # amber/src/main/resources/web-config.yml (port 8080).
         if: matrix.os == 'ubuntu-latest'
         run: .github/scripts/smoke-boot.sh 
"/tmp/dists/amber-*/bin/texera-web-application" 8080
-      - name: Smoke-test computing-unit-master boots
-        # computing-unit-master boots on postgres only, like texera-web: main
-        # starts a LOCAL Pekko actor master (clusterMode defaults to false with
-        # no --cluster arg, so no external seed/discovery and the cluster-only
-        # checkip.amazonaws.com lookup is never reached), then run() calls
-        # SqlServer.initConnection against the postgres provisioned above. The
-        # boot-time result-store cleanup is gated on
-        # ApplicationConfig.cleanupAllExecutionResults, which defaults to 
false,
-        # so no iceberg / S3 access. Config resolves via Utils.amberHomePath to
-        # amber/src/main/resources/computing-unit-master-config.yml (port 
8085).
-        # Reuses the amber dist built + unzipped above for the texera-web boot.
+      - name: Smoke-test computing-unit-master boots and the worker joins its 
cluster
+        # One check for the whole computing-unit cluster: boot a
+        # computing-unit-master and a no-arg computing-unit-worker from the
+        # packaged dist and assert the worker JOINS -- the master's 
ClusterListener
+        # logging "2 nodes in the cluster". This supersedes a standalone master
+        # boot check: smoke-cluster.sh already boots the master, waits for it 
to
+        # listen on :8085, and verifies it self-joins its 1-node cluster 
before the
+        # worker is launched, so a #6204-class linkage crash on master startup 
is

Review Comment:
   This clause claims a verification the script deliberately skips: 
smoke-cluster.sh's NB (lines 143-148) says it does **not** wait for the 
master's own "1 nodes" self-join line because keying on it caused a false 
failure — the only gate before the worker launches is the port probe. The 
#6204-class crash is still caught, just by the port-wait. The PR description 
carries the same stale clause; worth updating both.
   ```suggestion
           # listen on :8085 before launching the worker (deliberately not 
keying on
           # the master's own "1 nodes" self-join line -- see the NB in the 
script),
           # so a #6204-class linkage crash on master startup is
   ```



##########
.github/scripts/test_smoke_cluster.sh:
##########
@@ -0,0 +1,159 @@
+#!/usr/bin/env bash
+# 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.
+
+# End-to-end regression tests for smoke-cluster.sh, driven by fake 
master/worker
+# launchers (no real amber dist). These guard the harness's own decision logic:
+#   - worker joins (master logs the 2-node readiness line)  -> PASS
+#   - worker exits before joining                           -> FAIL ("worker 
exited...")
+#   - worker never joins within the timeout                 -> FAIL ("did not 
join...")
+#   - master dies during the join wait                      -> FAIL ("master 
exited...joined")
+#   - master logs no "N nodes" line at all (readiness drift) -> FAIL (timeout 
+ hint)
+#   - master crashes before it listens                      -> FAIL 
("...before listening")
+# The failure cases assert the SPECIFIC error message, so a future edit that
+# deletes a fast-fail branch (and lets the case pass only via the slow timeout)
+# turns the test red instead of silently passing. The happy path ties the join
+# signal to the worker actually running, so it can't pass if the worker is 
never
+# launched. The real cluster join is exercised against the packaged dist in
+# amber-integration. See https://github.com/apache/texera/issues/6523.
+
+set -uo pipefail
+
+command -v python3 >/dev/null || { echo "python3 is required to run these 
tests" >&2; exit 1; }
+
+script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+smoke="$script_dir/smoke-cluster.sh"
+work="$(mktemp -d 2>/dev/null || mktemp -d -t smoke-cluster)"
+trap 'rm -rf "$work"' EXIT
+rc=0
+
+PHRASE_SELF='Now we have 1 nodes in the cluster'
+PHRASE_JOINED='Now we have 2 nodes in the cluster'
+
+free_port() {
+  python3 -c 'import socket; s=socket.socket(); s.bind(("127.0.0.1",0)); 
print(s.getsockname()[1]); s.close()'
+}
+pass()   { echo "ok:   $1"; }
+failed() { echo "FAIL: $1"; rc=1; }
+
+py_listen() {  # emit a python one-liner that binds $1 and holds it for $2 
seconds
+  echo "exec python3 -c \"import socket,time; s=socket.socket(); 
s.setsockopt(socket.SOL_SOCKET,socket.SO_REUSEADDR,1); 
s.bind(('127.0.0.1',$1)); s.listen(); time.sleep($2)\""
+}
+
+# Fake master: binds $port so smoke-cluster's port-wait succeeds. $self=y 
prints
+# the self-join line. $trigger is "n" (never emit the joined line), a number N
+# (emit it N seconds after start), or "wait:<file>" (emit it only once <file>
+# exists -- used to couple the join to the worker actually running). $hold is 
how
+# long to hold the port (short => master dies mid-run).
+#   $1=path $2=port $3=self(y/n) $4=trigger $5=hold_secs
+make_master() {
+  local path="$1" port="$2" self="$3" trigger="$4" hold="$5"
+  {
+    echo '#!/usr/bin/env bash'
+    [[ "$self" == y ]] && echo "echo '$PHRASE_SELF'"
+    case "$trigger" in
+      n) : ;;
+      wait:*) echo "( while [ ! -f '${trigger#wait:}' ]; do sleep 0.2; done; 
echo '$PHRASE_JOINED' ) &" ;;

Review Comment:
   If the happy-path case ever fails with the worker never touching `$sig` — 
exactly the mutation this case guards — this waiter outlives the run and 
busy-polls forever: smoke-cluster's cleanup kills the exec'd python, not this 
pre-`exec` fork. Bounding the poll (e.g. `for _ in \$(seq 1 600)` around it) 
keeps a red run from leaking an orphan on a dev machine.



##########
.github/scripts/smoke-cluster.sh:
##########
@@ -0,0 +1,179 @@
+#!/usr/bin/env bash
+# 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.
+
+# smoke-cluster.sh -- boot a computing-unit-master and a computing-unit-worker
+# from their unpacked dist and assert the worker actually JOINS the master's
+# amber Pekko cluster, not merely that its process survives.
+#
+# Why this can't reuse smoke-boot.sh: ComputingUnitWorker.main only calls
+# AmberRuntime.startActorWorker, which binds 
pekko.remote.artery.canonical.port=0
+# (a random ephemeral port) and starts no HTTP server -- so there is no fixed
+# port to probe. A worker booted standalone just retries its seed forever and
+# stays up, so "did not crash" proves nothing. The real health signal is 
cluster
+# membership, which ClusterListener prints on every membership change:
+#     ---------Now we have N nodes in the cluster---------  (N = 
cluster.state.members.size)
+# master-alone => 1, master+worker joined => 2. We wait for the "2" line -- a
+# deliberate readiness signal the app emits, not a crash-scan (contrast #6332).
+# See issue #6523 (raised from the #6377 review) and #6220 for the rationale.
+#
+# Single host, no external network: the master runs in non-cluster mode, which
+# self-seeds pekko://Amber@localhost:2552; the worker is launched with NO
+# --serverAddr, so it defaults its seed to localhost:2552 and skips the
+# getNodeIpAddress -> http://checkip.amazonaws.com lookup (that only fires when
+# --serverAddr is supplied).
+#
+# Usage:
+#   smoke-cluster.sh <master-glob> <worker-glob> <master-http-port> 
[join_timeout]
+#
+# Requirements: run from the checkout root -- the master resolves its config 
via
+# Utils.amberHomePath -> 
./amber/src/main/resources/computing-unit-master-config.yml
+# -- with postgres already provisioned (the master calls 
SqlServer.initConnection).
+
+set -euo pipefail
+
+master_glob="${1:?usage: smoke-cluster.sh <master-glob> <worker-glob> 
<master-http-port> [join_timeout]}"
+worker_glob="${2:?worker launcher glob required}"
+http_port="${3:?master http port required}"
+join_timeout="${4:-40}"
+master_ready_timeout="${SMOKE_MASTER_READY_TIMEOUT:-60}"
+
+PHRASE_JOINED='Now we have 2 nodes in the cluster'
+
+# Resolve a (globbed) launcher to exactly one executable -- same contract as

Review Comment:
   `resolve_launcher`, `port_open`, and the TERM-grace-KILL reap are 
near-verbatim copies of smoke-boot.sh's, and only this one carries a 
cross-pointer. A future fix to the probe or the reaper (the ::1-vs-IPv4 fix 
these comments encode is exactly that class) lands in one script and silently 
misses the twin. Can we extract a small sourced `smoke-lib.sh` for the shared 
trio? That's where I'd lean now that there are two full copies; failing that, 
matching pointers at `port_open` and the reap keep the next editor honest.



##########
.github/scripts/test_smoke_cluster.sh:
##########
@@ -0,0 +1,159 @@
+#!/usr/bin/env bash
+# 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.
+
+# End-to-end regression tests for smoke-cluster.sh, driven by fake 
master/worker
+# launchers (no real amber dist). These guard the harness's own decision logic:
+#   - worker joins (master logs the 2-node readiness line)  -> PASS
+#   - worker exits before joining                           -> FAIL ("worker 
exited...")
+#   - worker never joins within the timeout                 -> FAIL ("did not 
join...")
+#   - master dies during the join wait                      -> FAIL ("master 
exited...joined")
+#   - master logs no "N nodes" line at all (readiness drift) -> FAIL (timeout 
+ hint)
+#   - master crashes before it listens                      -> FAIL 
("...before listening")
+# The failure cases assert the SPECIFIC error message, so a future edit that
+# deletes a fast-fail branch (and lets the case pass only via the slow timeout)
+# turns the test red instead of silently passing. The happy path ties the join
+# signal to the worker actually running, so it can't pass if the worker is 
never
+# launched. The real cluster join is exercised against the packaged dist in
+# amber-integration. See https://github.com/apache/texera/issues/6523.
+
+set -uo pipefail
+
+command -v python3 >/dev/null || { echo "python3 is required to run these 
tests" >&2; exit 1; }
+
+script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+smoke="$script_dir/smoke-cluster.sh"
+work="$(mktemp -d 2>/dev/null || mktemp -d -t smoke-cluster)"
+trap 'rm -rf "$work"' EXIT
+rc=0
+
+PHRASE_SELF='Now we have 1 nodes in the cluster'
+PHRASE_JOINED='Now we have 2 nodes in the cluster'
+
+free_port() {
+  python3 -c 'import socket; s=socket.socket(); s.bind(("127.0.0.1",0)); 
print(s.getsockname()[1]); s.close()'
+}
+pass()   { echo "ok:   $1"; }
+failed() { echo "FAIL: $1"; rc=1; }
+
+py_listen() {  # emit a python one-liner that binds $1 and holds it for $2 
seconds
+  echo "exec python3 -c \"import socket,time; s=socket.socket(); 
s.setsockopt(socket.SOL_SOCKET,socket.SO_REUSEADDR,1); 
s.bind(('127.0.0.1',$1)); s.listen(); time.sleep($2)\""
+}
+
+# Fake master: binds $port so smoke-cluster's port-wait succeeds. $self=y 
prints
+# the self-join line. $trigger is "n" (never emit the joined line), a number N
+# (emit it N seconds after start), or "wait:<file>" (emit it only once <file>
+# exists -- used to couple the join to the worker actually running). $hold is 
how
+# long to hold the port (short => master dies mid-run).
+#   $1=path $2=port $3=self(y/n) $4=trigger $5=hold_secs
+make_master() {
+  local path="$1" port="$2" self="$3" trigger="$4" hold="$5"
+  {
+    echo '#!/usr/bin/env bash'
+    [[ "$self" == y ]] && echo "echo '$PHRASE_SELF'"
+    case "$trigger" in
+      n) : ;;
+      wait:*) echo "( while [ ! -f '${trigger#wait:}' ]; do sleep 0.2; done; 
echo '$PHRASE_JOINED' ) &" ;;
+      *) echo "( sleep $trigger; echo '$PHRASE_JOINED' ) &" ;;
+    esac
+    py_listen "$port" "$hold"
+  } >"$path"
+  chmod +x "$path"
+}
+
+# Fake worker: sleeps (healthy), crashes on boot, or touches a signal file (to
+# trigger a coupled master) then sleeps.  $1=path $2=sleep|crash|touch:<file>
+make_worker() {
+  case "$2" in
+    crash)   printf '#!/usr/bin/env bash\necho "worker boom" >&2\nexit 1\n' 
>"$1" ;;
+    touch:*) printf '#!/usr/bin/env bash\ntouch "%s"\nexec sleep 120\n' 
"${2#touch:}" >"$1" ;;
+    *)       printf '#!/usr/bin/env bash\nexec sleep 120\n' >"$1" ;;
+  esac
+  chmod +x "$1"
+}
+
+# Assert smoke-cluster FAILS and its output contains $needle (locks in the 
branch
+# that produces that message). $1=desc $2=needle, remaining args = 
smoke-cluster argv.
+assert_fail_msg() {
+  local desc="$1" needle="$2"; shift 2
+  local out
+  if out="$("$smoke" "$@" 2>&1)"; then
+    failed "$desc: expected FAIL but it passed"
+  elif printf '%s\n' "$out" | grep -Fq "$needle"; then
+    pass "$desc"
+  else
+    failed "$desc: failed but message missing '$needle'"; printf '%s\n' "$out" 
| tail -n3
+  fi
+}
+
+# --- happy path: the worker triggers the join; master observes 2 nodes -> 
PASS.
+# The joined line fires only after the worker touches the signal, so this 
cannot
+# pass if smoke-cluster never launches the worker. ---
+port="$(free_port)"; sig="$work/joined.signal"
+make_master "$work/m_ok" "$port" y "wait:$sig" 120
+make_worker "$work/w_ok" "touch:$sig"
+if "$smoke" "$work/m_ok" "$work/w_ok" "$port" 30 >/dev/null 2>&1; then
+  pass "worker joins cluster (master sees 2 nodes) -> OK"
+else
+  failed "worker join should be OK"
+fi
+
+# --- worker exits before joining -> FAIL fast (assert the fast-fail message) 
---
+port="$(free_port)"
+make_master "$work/m_nojoin" "$port" y n 120
+make_worker "$work/w_crash" crash
+assert_fail_msg \
+  "worker exits before join -> FAIL fast" "worker exited before joining" \
+  "$work/m_nojoin" "$work/w_crash" "$port" 15
+
+# --- worker never joins within the timeout -> FAIL (assert the timeout 
message) ---
+port="$(free_port)"
+make_master "$work/m_nojoin2" "$port" y n 120
+make_worker "$work/w_sleep" sleep
+assert_fail_msg \
+  "worker never joins -> FAIL (timeout)" "did not join the cluster within" \
+  "$work/m_nojoin2" "$work/w_sleep" "$port" 3
+
+# --- master dies during the join wait -> FAIL (assert the master-death 
message) ---
+port="$(free_port)"
+make_master "$work/m_die" "$port" y n 4
+make_worker "$work/w_sleep_b" sleep
+assert_fail_msg \
+  "master dies during join wait -> FAIL" "master exited before the worker 
joined" \
+  "$work/m_die" "$work/w_sleep_b" "$port" 20
+
+# --- master logs no "N nodes" line at all (readiness log drift) -> FAIL with 
a hint ---
+port="$(free_port)"
+make_master "$work/m_drift" "$port" n n 120
+make_worker "$work/w_sleep2" sleep
+assert_fail_msg \
+  "master readiness log drift -> FAIL (timeout + drift hint)" "readiness log 
may have changed" \
+  "$work/m_drift" "$work/w_sleep2" "$port" 3
+
+# --- master crashes before it listens -> FAIL (assert the pre-listen message) 
---

Review Comment:
   The master-alive-but-never-listening branch (smoke-cluster.sh:140, "did not 
listen ... within") is the only verdict branch without a message-asserted case, 
and the header's slow-timeout-masking argument applies to it too. A fake master 
that stays alive without binding anything (an `exec sleep 120` launcher, like 
the workers) plus a small `SMOKE_MASTER_READY_TIMEOUT` covers it in a few 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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to