imbajin commented on code in PR #3105:
URL: https://github.com/apache/hugegraph/pull/3105#discussion_r3649547931
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugeCountStrategy.java:
##########
@@ -277,28 +272,10 @@ private boolean doStrategy(final Step step) {
return false;
}
- final P<?> predicate = ((IsStep<?>) step.getNextStep()).getPredicate();
- if (this.hasNestedConnectivePredicate(predicate)) {
- return false;
- }
-
final Step parent = step.getTraversal().getParent().asStep();
Review Comment:
‼️ This PR's sole commit is based directly on `e960cc5` (`fix(server): skip
unsafe count optm for nested predicates (#3100)`), but it removes that commit's
entire 23-line `ConnectiveP` safety guard and simultaneously deletes all seven
associated `CountStrategyCoreTest` regressions. Merging this shell-focused
change would therefore reopen the wrong-result cases fixed by #3100 while
hiding them from CI. Please remove both Java-file diffs from this PR and retain
the complete #3100 implementation and tests.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -79,15 +79,199 @@ function process_id() {
return "$pid"
}
-# check the port of rest server is occupied
+# check whether the REST server port is occupied
function check_port() {
- local port=$(echo "$1" | sed 's|.*:||' | sed 's|/.*||')
- if ! command_available "lsof"; then
- echo "Required lsof but it is unavailable"
- exit 1
+ local url="$1"
+ local host
+ local port
+
+ # Strip leading/trailing whitespace from URL (handles whitespace from
ServerOptions)
+ url="${url#"${url%%[![:space:]]*}"}"
+ url="${url%"${url##*[![:space:]]}"}"
+
+ # Extract port: last colon-delimited segment, strip trailing path
+ port=$(echo "$url" | sed 's|.*:||' | sed 's|/.*||')
+ if [[ -z "$port" ]]; then
+ return 0
fi
- lsof -i :"$port" >/dev/null
- if [ $? -eq 0 ]; then
+
+ if ! [[ "$port" =~ ^[0-9]+$ ]]; then
+ return 0
+ fi
+ port=$((10#$port))
+ if (( port < 1 || port > 65535 )); then
+ return 0
+ fi
+
+ # Extract host
+ if [[ "$url" =~ \[([^\]]*)\] ]]; then
+ host="${BASH_REMATCH[1]}"
+ else
+ host=$(echo "$url" | sed 's|.*://||' | sed 's|:.*||')
+ fi
+ # Strip any leading/trailing whitespace
+ host="${host#"${host%%[![:space:]]*}"}"
+ host="${host%"${host##*[![:space:]]}"}"
+
+ # Resolve hostname → numeric IPs so ss/netstat (which use -n) can match
them.
+ # A "hostname" is anything that is not blank, a wildcard, or already
numeric.
+ # If resolution fails, resolved_addrs stays empty and we fall back to
/dev/tcp probe.
+ local resolved_addrs=""
+ local is_hostname=0
+ if [[ -n "$host" && "$host" != "0.0.0.0" && "$host" != "::" && "$host" !=
"*" ]] \
+ && ! [[ "$host" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]] \
+ && ! [[ "$host" =~ ^[0-9a-fA-F:]*:[0-9a-fA-F:]*$ ]]; then
+ is_hostname=1
+ if command_available "getent"; then
+ resolved_addrs=$(getent hosts "$host" 2>/dev/null | awk '{print
$1}')
Review Comment:
⚠️ The new hostname-resolution phase runs synchronously without any
deadline, before either bounded `/dev/tcp` branch. A slow or stuck NSS/DNS/LDAP
lookup in `getent hosts` (and likewise `dscacheutil`) can therefore still hang
startup despite the later 1-2 second probe watchdog. Please apply a hard
deadline to resolution as well, fall through safely on timeout, and cover a
blocking resolver in the shell tests.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -79,15 +79,199 @@ function process_id() {
return "$pid"
}
-# check the port of rest server is occupied
+# check whether the REST server port is occupied
function check_port() {
- local port=$(echo "$1" | sed 's|.*:||' | sed 's|/.*||')
- if ! command_available "lsof"; then
- echo "Required lsof but it is unavailable"
- exit 1
+ local url="$1"
+ local host
+ local port
+
+ # Strip leading/trailing whitespace from URL (handles whitespace from
ServerOptions)
+ url="${url#"${url%%[![:space:]]*}"}"
+ url="${url%"${url##*[![:space:]]}"}"
+
+ # Extract port: last colon-delimited segment, strip trailing path
+ port=$(echo "$url" | sed 's|.*:||' | sed 's|/.*||')
Review Comment:
⚠️ This extracts the port from the last colon in the entire URL rather than
from its authority. `ServerOptionsTest.testUrlNormalizationPreservesPathCase`
explicitly preserves URL paths, so `http://127.0.0.1:8080/path:9090` is checked
as port 9090 and can miss an occupied 8080; a valid URL without an explicit
port is skipped entirely instead of checking its HTTP/HTTPS default. Please
parse the normalized URI authority, handle the scheme's default port
explicitly, and add regressions for a colon in the path and for an omitted port.
##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-check-port.sh:
##########
@@ -0,0 +1,682 @@
+#!/bin/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.
+#
+# test-check-port.sh — Unit tests for check_port() in util.sh
+#
+# Strategy: each test runs check_port in a subshell that overrides
+# command_available() to control which probe branch is taken, and
+# overrides the tool functions (ss, netstat, timeout) to control
+# what they return — no real network connections needed.
+#
+# check_port calls `exit 1` when the port is in use, so the subshell
+# exits 1; it returns normally (exit 0) when the port is free.
+#
+# Usage: ./test-check-port.sh [path-to-hugegraph-static-dir]
+# path-to-hugegraph-static-dir: directory containing bin/util.sh
+# Defaults to current directory.
+# In CI: $TRAVIS_DIR/test-check-port.sh
hugegraph-server/hugegraph-dist/src/assembly/static
+
+set -uo pipefail
+
+STATIC_DIR="${1:-$(pwd)}"
+UTIL_SH="$STATIC_DIR/bin/util.sh"
+
+REPO_ROOT="$(cd "$(dirname "$0")/../../../../.." && pwd)"
+PD_UTIL_SH="$REPO_ROOT/hugegraph-pd/hg-pd-dist/src/assembly/static/bin/util.sh"
+STORE_UTIL_SH="$REPO_ROOT/hugegraph-store/hg-store-dist/src/assembly/static/bin/util.sh"
+
+PASS=0
+FAIL=0
+ERRORS=()
+
+GREEN='\033[0;32m'
+RED='\033[0;31m'
+YELLOW='\033[1;33m'
+NC='\033[0m'
+
+pass() { echo -e "${GREEN} PASS${NC} $1"; PASS=$((PASS + 1)); }
+fail() { echo -e "${RED} FAIL${NC} $1"; ERRORS+=("$1"); FAIL=$((FAIL + 1)); }
+section() { echo ""; echo "── $1 ──"; }
+
+echo ""
+echo "check_port() unit test suite"
+echo "util.sh: $UTIL_SH"
+echo ""
+
+if [[ ! -f "$UTIL_SH" ]]; then
+ echo -e "${RED}ERROR:${NC} $UTIL_SH not found."
+ echo " Pass the HugeGraph static assembly dir as \$1"
+ exit 1
+fi
+
+# ── ss branch
─────────────────────────────────────────────────────────────────
+
+section "ss branch — IPv4"
+
+(
+ # shellcheck source=/dev/null
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 0.0.0.0:8080 0.0.0.0:*"; }
+ check_port "http://127.0.0.1:8080";
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: IPv4 port occupied → exit 1" \
+ || fail "ss: IPv4 port occupied → expected exit 1, got 0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 0.0.0.0:9090 0.0.0.0:*"; }
+ check_port "http://127.0.0.1:8080";
+)
+[[ $? -eq 0 ]] \
+ && pass "ss: IPv4 port free → exit 0" \
+ || fail "ss: IPv4 port free → expected exit 0, got 1"
+
+section "ss branch — IPv6 URL with scheme (http://[::1]:8080)"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:8080 [::]:*"; }
+ check_port "http://[::1]:8080";
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: http://[::1]:8080 occupied → exit 1" \
+ || fail "ss: http://[::1]:8080 occupied → expected exit 1, got 0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:9090 [::]:*"; }
+ check_port "http://[::1]:8080";
+)
+[[ $? -eq 0 ]] \
+ && pass "ss: http://[::1]:8080 free → exit 0" \
+ || fail "ss: http://[::1]:8080 free → expected exit 0, got 1"
+
+section "ss branch — IPv6 URL without scheme ([::1]:8080)"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:8080 [::]:*"; }
+ check_port "[::1]:8080"
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: [::1]:8080 (no scheme) occupied → exit 1" \
+ || fail "ss: [::1]:8080 (no scheme) occupied → expected exit 1, got 0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:9090 [::]:*"; }
+ check_port "[::1]:8080"
+)
+[[ $? -eq 0 ]] \
+ && pass "ss: [::1]:8080 (no scheme) free → exit 0" \
+ || fail "ss: [::1]:8080 (no scheme) free → expected exit 0, got 1"
+
+section "ss branch — wildcard 0.0.0.0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 0.0.0.0:8080 0.0.0.0:*"; }
+ check_port "http://0.0.0.0:8080";
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: 0.0.0.0:8080 occupied → exit 1" \
+ || fail "ss: 0.0.0.0:8080 occupied → expected exit 1, got 0"
+
+section "ss branch — wildcard ::"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:8080 [::]:*"; }
+ check_port "http://[::]:8080";
+)
+[[ $? -eq 1 ]] \
+ && pass "ss: [::]:8080 occupied → exit 1" \
+ || fail "ss: [::]:8080 occupied → expected exit 1, got 0"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 [::]:9090 [::]:*"; }
+ check_port "http://[::]:8080";
+)
+[[ $? -eq 0 ]] \
+ && pass "ss: [::]:8080 free → exit 0" \
+ || fail "ss: [::]:8080 free → expected exit 0, got 1"
+
+# ── netstat branch
────────────────────────────────────────────────────────────
+
+section "netstat branch — Linux format (-ltn), occupied"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "netstat" ]]; }
+ netstat() { echo "tcp 0 0 0.0.0.0:8080 0.0.0.0:* LISTEN"; }
+ check_port "http://127.0.0.1:8080";
+)
+[[ $? -eq 1 ]] \
+ && pass "netstat -ltn: port 8080 occupied → exit 1" \
+ || fail "netstat -ltn: port 8080 occupied → expected exit 1, got 0"
+
+section "netstat branch — Linux format (-ltn), free"
+
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "netstat" ]]; }
+ netstat() { echo "tcp 0 0 0.0.0.0:9090 0.0.0.0:* LISTEN"; }
+ check_port "http://127.0.0.1:8080";
+)
+[[ $? -eq 0 ]] \
+ && pass "netstat -ltn: port 8080 free → exit 0" \
+ || fail "netstat -ltn: port 8080 free → expected exit 0, got 1"
+
+section "netstat branch — BSD/macOS fallback (-an), occupied"
+
+# Simulate netstat that produces no output for -ltn (Linux flag unsupported)
+# but outputs BSD-format lines for -an
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "netstat" ]]; }
+ netstat() {
+ if [[ "$1" == "-ltn" ]]; then
+ return 1 # flag not supported on BSD
+ fi
+ echo "tcp4 0 0 *.8080 *.* LISTEN"
+ }
+ check_port "http://127.0.0.1:8080";
+)
+[[ $? -eq 1 ]] \
+ && pass "netstat -an BSD: port 8080 occupied → exit 1" \
+ || fail "netstat -an BSD: port 8080 occupied → expected exit 1, got 0"
+
+section "netstat branch — IP octet false-positive guard"
+
+# Port 80 check; netstat output contains 192.168.80.1:443
+# The .80 in the IP address must NOT match port 80
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "netstat" ]]; }
+ netstat() { echo "tcp 0 0 192.168.80.1:443 0.0.0.0:* LISTEN"; }
+ check_port "http://127.0.0.1:80";
+)
+[[ $? -eq 0 ]] \
+ && pass "netstat: IP octet .80 does not false-positive for port 80 → exit
0" \
+ || fail "netstat: IP octet .80 false-positived for port 80 → expected exit
0, got 1"
+
+section "ss branch — host dot-escaping guard"
+
+# Host 127.0.0.1 must be matched literally: a listener whose address merely
+# matches the pattern with '.' as a regex wildcard (127a0b0c1) must NOT count
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "ss" ]]; }
+ ss() { echo "tcp LISTEN 0 128 127a0b0c1:8080 0.0.0.0:*"; }
+ check_port "http://127.0.0.1:8080";
+)
+[[ $? -eq 0 ]] \
+ && pass "ss: unescaped-dot lookalike 127a0b0c1 does not false-positive →
exit 0" \
+ || fail "ss: unescaped-dot lookalike 127a0b0c1 false-positived → expected
exit 0, got 1"
+
+# ── /dev/tcp fallback branch
──────────────────────────────────────────────────
+
+section "/dev/tcp fallback — timeout available, port occupied"
+
+# timeout exits 0 → connection succeeded → port in use
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "timeout" ]]; }
+ timeout() {
+ # Assert correct invocation: timeout 1 bash -c SCRIPT _ HOST PORT
+ [[ "$1" == "1" && "$2" == "bash" && "$3" == "-c" && "$5" == "_" ]] \
+ || { echo "timeout mock: unexpected argv: $*"; return 2; }
+ return 0
+ }
+ check_port "http://127.0.0.1:8080";
+)
+[[ $? -eq 1 ]] \
+ && pass "/dev/tcp+timeout: connection succeeded (exit 0) → port occupied →
exit 1" \
+ || fail "/dev/tcp+timeout: connection succeeded → expected exit 1, got 0"
+
+section "/dev/tcp fallback — timeout available, port free"
+
+# timeout exits 1 → connection refused → port free
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "timeout" ]]; }
+ timeout() {
+ [[ "$1" == "1" && "$2" == "bash" && "$3" == "-c" && "$5" == "_" ]] \
+ || { echo "timeout mock: unexpected argv: $*"; return 2; }
+ return 1
+ }
+ check_port "http://127.0.0.1:8080";
+)
+[[ $? -eq 0 ]] \
+ && pass "/dev/tcp+timeout: connection refused (exit 1) → port free → exit
0" \
+ || fail "/dev/tcp+timeout: connection refused → expected exit 0, got 1"
+
+section "/dev/tcp fallback — real loopback (ephemeral port)"
+
+# Start Python server on ephemeral port 0, capture actual port from child
stdout
+(
+ source "$UTIL_SH"
+ command_available() { [[ "$1" == "timeout" ]]; }
+ # Mock timeout: on hosts without timeout (macOS), run the probe directly.
+ # The probe args are: timeout 1 bash -c SCRIPT _ HOST PORT
+ timeout() {
+ [[ "$1" == "1" && "$2" == "bash" && "$3" == "-c" && "$5" == "_" ]] \
+ || { echo "timeout mock: unexpected argv: $*" >&2; return 2; }
+ # Run the probe with a 2-second hard deadline via background + watchdog
+ bash -c "$4" "$5" "$6" "$7" 2>/dev/null &
+ local probe_pid=$!
+ (sleep 2; kill -9 "$probe_pid" 2>/dev/null) &
+ local watchdog_pid=$!
+ wait "$probe_pid" 2>/dev/null
+ local rc=$?
+ kill -9 "$watchdog_pid" 2>/dev/null
+ wait "$watchdog_pid" 2>/dev/null
+ return $rc
+ }
+ # Use a temp file to capture the bound port from child
+ port_file=$(mktemp)
+ trap "rm -f $port_file" EXIT
+
+ # Start Python server that prints the bound port to stdout
+ python3 -c "
+import socket
+s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+s.bind(('127.0.0.1', 0))
+s.listen(1) # actually listen so /dev/tcp can connect
+port = s.getsockname()[1]
+print(port, flush=True)
+import time
+time.sleep(30) # keep server alive
+" > "$port_file" 2>/dev/null &
+ PY_PID=$!
+ # Poll for port file up to ~4s (Python cold-start can exceed 0.5s on busy
CI)
+ bound_port=""
+ for _ in $(seq 1 40); do
+ bound_port=$(head -1 "$port_file" 2>/dev/null)
+ [[ -n "$bound_port" ]] && break
+ kill -0 $PY_PID 2>/dev/null || break
+ sleep 0.1
+ done
+ if [[ -z "$bound_port" || ! "$bound_port" =~ ^[0-9]+$ ]]; then
+ echo "SKIP: failed to get bound port"
+ kill -9 $PY_PID 2>/dev/null || true
+ exit 77
+ fi
+ # Verify child is alive
+ if ! kill -0 $PY_PID 2>/dev/null; then
+ echo "SKIP: Python child died"
+ exit 77
+ fi
+ check_port "http://127.0.0.1:$bound_port";
Review Comment:
⚠️ On the expected occupied-port path, `check_port` executes `exit 1`, so
the following `rc`, `kill`, and `wait` cleanup is unreachable. The EXIT trap
only removes the temporary file, leaving the Python listener alive for up to 30
seconds after every successful real-probe test. Please register cleanup of
`PY_PID` in the EXIT trap (including `wait`), or invoke `check_port` in a
nested subshell so the parent can always reap the child.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
