Visitar URL original
Merge pull request #2125 from KazukiKandaKK/fix-pinot-minmax-string-w… · ClickHouse/ClickBench@007e002 · GitHub
Skip to content

Commit 007e002

Browse files
alexey-milovidovalexey-milovidov
authored
Merge pull request #2125 from KazukiKandaKK/fix-pinot-minmax-string-workaround
pinot: work around STRING MIN/MAX limitation in queries.sql (apache/pinot#19603)
2 parents b605e6f + ecd0e2d commit 007e002

7 files changed

Lines changed: 171 additions & 16 deletions

File tree

‎pinot/benchmark.sh‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,29 @@ export BENCH_DOWNLOAD_SCRIPT="download-hits-tsv"
44
# inside one JVM and takes longer than the lib's 300 s default to be
55
# query-ready on a cold instance. 900 s clears the observed cold start.
66
export BENCH_CHECK_TIMEOUT=900
7+
# Pinot QuickStart's -dataDir persistence is broken in 1.5.0/1.5.1: the
8+
# second QuickStart invocation against an existing -dataDir always dies
9+
# with IllegalStateException from Quickstart.java's
10+
# Preconditions.checkState(quickstartRunnerDir.mkdirs()) — mkdirs() returns
11+
# false because the directory from the first run already exists (verified
12+
# by reproducing this locally in Docker and reading the Pinot source).
13+
# A stop/start cycle therefore always wipes the in-memory table/segment
14+
# registration and there is no supported way to bring the daemon back up
15+
# against its previous state directory.
16+
#
17+
# What *does* survive the cycle is the segment tar files QuickStart wrote
18+
# to a plain local directory (batch/hits/segments/, not process memory —
19+
# verified locally: kill -9 the JVM, restart it, the .tar.gz files are
20+
# still on disk). BENCH_DURABLE=no makes the shared driver re-run ./load
21+
# after every restart; our ./load exploits this by re-pushing those
22+
# existing tars straight to the controller's segment-upload API
23+
# (https://docs.pinot.apache.org/build-with-pinot/ingestion/batch-ingestion/segment-upload)
24+
# instead of re-parsing hits.tsv, so the "reload" on cold tries 2-129
25+
# (43 queries x 3 tries) takes seconds instead of the ~78 min a real
26+
# from-source reload would need. This keeps the real stop/start/
27+
# drop_caches cycle other systems get, rather than skipping it via
28+
# BENCH_RESTARTABLE=no.
29+
export BENCH_DURABLE=no
730
# Skip the pre-snapshot ./stop+./start cycle: the loaded
831
# state lives only in the daemon's process memory (in-process
932
# DataFrame, JVM heap caches) and stopping wipes it. The

‎pinot/check‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,28 @@
11
#!/bin/bash
22
set -e
33

4-
# Pinot is responsive once both the controller and the broker accept queries.
5-
RES=$(curl -sf -o /dev/null -w '%{http_code}' \
4+
# The QuickStart process only registers its clean-shutdown hook after
5+
# ./start's launched process finishes starting every component AND
6+
# bootstrapping every sample table (including waiting for each table's
7+
# async minion segment-generation-and-push task to finish). If ./stop
8+
# sends SIGTERM any earlier than that, either shutdown-hook registration
9+
# itself throws (component startup still in flight) or an in-flight
10+
# minion task gets killed (bootstrap still in flight) -- either way the
11+
# process dies without a clean stop and corrupts the next ./start.
12+
#
13+
# Broker readiness alone is not a safe proxy for "fully done starting":
14+
# it comes up long before bootstrap finishes. The only reliable signal is
15+
# the "Quick start setup complete" line QuickStart itself prints right
16+
# after runner.bootstrapTable() + waitForBootstrapToComplete() return
17+
# (see pinot-tools Quickstart.java, method execute()). Since pinot.log is
18+
# appended across restarts, only look at what was appended after the
19+
# offset ./start recorded for this invocation.
20+
BROKER_RES=$(curl -sf -o /dev/null -w '%{http_code}' \
621
-XPOST -H'Content-Type: application/json' \
722
http://localhost:8000/query/sql/ \
8-
-d '{"sql":"SELECT 1"}')
23+
-d '{"sql":"SELECT 1"}' 2>/dev/null || echo "000")
924

10-
[ "$RES" = "200" ]
25+
OFFSET=$(cat pinot.log.offset 2>/dev/null || echo 0)
26+
SETUP_DONE=$(tail -c +"$((OFFSET + 1))" pinot.log 2>/dev/null | grep -c 'Quick start setup complete' || true)
27+
28+
[ "$BROKER_RES" = "200" ] && [ "${SETUP_DONE:-0}" -ge 1 ]

‎pinot/load‎

Lines changed: 84 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ set -e
33

44
PINOT_VERSION=1.5.1
55
PINOT_DIR="apache-pinot-$PINOT_VERSION-bin"
6+
SEGMENTS_DIR="batch/hits/segments"
67

78
# Wait for the controller's REST API to be live. `start` only waits for
89
# the broker; the controller can lag, and AddTable below talks to the
@@ -32,15 +33,68 @@ for _ in $(seq 1 60); do
3233
sleep 1
3334
done
3435

35-
# Register the schema and table. Previously trailed `|| true`, which
36-
# masked genuine failures (controller not up, bad JSON, etc.) and let
37-
# LaunchDataIngestionJob run against a missing table — the symptom
38-
# was a misleading "Failed to fetch hits/schema" deep in the ingestion
39-
# job. Fail loudly here.
40-
"./${PINOT_DIR}/bin/pinot-admin.sh" AddTable \
36+
# Register the schema and table. AddTable fails with "already exists" on
37+
# every call after the first (BENCH_DURABLE=no re-runs ./load on every
38+
# cold cycle; QuickStart's own restart wipes the table registration but
39+
# not the segment tar files on disk — see the fast path below). Treat
40+
# that specific failure as success; anything else should still fail loudly
41+
# as before.
42+
add_table_out=$("./${PINOT_DIR}/bin/pinot-admin.sh" AddTable \
4143
-tableConfigFile offline_table.json \
42-
-schemaFile schema.json -exec
44+
-schemaFile schema.json -exec 2>&1) && add_table_rc=0 || add_table_rc=$?
45+
if [ "$add_table_rc" -ne 0 ]; then
46+
if ! printf '%s\n' "$add_table_out" | grep -q 'already exists'; then
47+
printf '%s\n' "$add_table_out" >&2
48+
exit "$add_table_rc"
49+
fi
50+
fi
51+
52+
# Fast path: segment tar files from a previous ./load already exist on
53+
# disk. QuickStart -type batch writes them under $SEGMENTS_DIR (a plain
54+
# local directory, not process memory) and a stop/start cycle wipes only
55+
# the in-memory table/segment registration, not these files (verified by
56+
# reproducing this locally in Docker: kill -9 the QuickStart JVM, restart
57+
# it, curl POST each existing .tar.gz straight to the controller's
58+
# /segments endpoint — official "tar push" upload API, see
59+
# https://docs.pinot.apache.org/build-with-pinot/ingestion/batch-ingestion/segment-upload
60+
# — and the table is fully queryable again, star-tree index included,
61+
# without re-parsing hits.tsv). Re-pushing ~100 existing tars takes
62+
# seconds; re-running LaunchDataIngestionJob from hits.tsv takes ~78 min.
63+
if [ -d "$SEGMENTS_DIR" ] && ls "$SEGMENTS_DIR"/*.tar.gz >/dev/null 2>&1; then
64+
for f in "$SEGMENTS_DIR"/*.tar.gz; do
65+
curl -sS -X POST -H 'Content-Type: multipart/form-data' \
66+
-F "file=@${f}" \
67+
'http://localhost:9000/segments?tableName=hits&tableType=OFFLINE' \
68+
| grep -q 'Successfully uploaded' || {
69+
echo "bench: failed to re-push ${f}" >&2
70+
exit 1
71+
}
72+
done
73+
74+
# Re-pushed segments take a few seconds to show up in the external
75+
# view (Helix propagation), during which queries return a transient
76+
# "N segments unavailable" error. Poll until every expected segment
77+
# is ONLINE instead of racing the very first query against it
78+
# (observed locally: up to ~20s for 5 segments; sized generously
79+
# for ~100).
80+
expected=$(ls "$SEGMENTS_DIR"/*.tar.gz | wc -l | tr -d ' ')
81+
for _ in $(seq 1 120); do
82+
online=$(curl -sf http://localhost:9000/tables/hits/externalview 2>/dev/null \
83+
| python3 -c 'import json,sys
84+
d=json.load(sys.stdin)
85+
ov=d.get("OFFLINE") or {}
86+
print(sum(1 for st in ov.values() if list(st.values()) == ["ONLINE"]))' \
87+
2>/dev/null || echo 0)
88+
if [ "$online" -ge "$expected" ]; then
89+
break
90+
fi
91+
sleep 1
92+
done
4393

94+
exit 0
95+
fi
96+
97+
# Slow path (first ./load only): generate segments from hits.tsv.
4498
# Pinot was unable to load data as a single file without errors. Split.
4599
split -d --additional-suffix .tsv -n l/100 hits.tsv parts
46100

@@ -51,7 +105,30 @@ sed parts93.tsv -e 's/"tatuirovarki_redmond/tatuirovarki_redmond/g' -i
51105
sed splitted.yaml -e "s|PWD_DIR_PLACEHOLDER|$PWD|g" -i
52106
sed local.yaml -e "s|PWD_DIR_PLACEHOLDER|$PWD|g" -i
53107

108+
# pinot-admin.sh's own default is -Xms4G with NO -Xmx (falls back to the
109+
# JVM's default of ~25% of physical RAM). On a real ~74GB/100-segment
110+
# ClickBench hits.tsv this OOMed mid-job around segment 60/100 on a
111+
# 30GB-RAM c6a.4xlarge (OutOfMemoryError in SegmentDictionaryCreator) —
112+
# and LaunchDataIngestionJob still exited 0, silently leaving only 60/100
113+
# segments on disk. Force a real heap ceiling sized for this instance.
114+
export JAVA_OPTS="-Xms4G -Xmx20G"
115+
54116
"./${PINOT_DIR}/bin/pinot-admin.sh" LaunchDataIngestionJob -jobSpecFile splitted.yaml
55117

118+
# Defense against the exact failure this instance just hit: Pinot's own
119+
# ingestion job runner can swallow a per-file OutOfMemoryError (or other
120+
# per-task exception) and still exit 0, leaving fewer segment tars on
121+
# disk than input splits. Fail loudly instead of letting a partial load
122+
# silently pass bench_load's >5GB data-size check downstream.
123+
actual_segments=$(ls "$SEGMENTS_DIR"/*.tar.gz 2>/dev/null | wc -l | tr -d ' ')
124+
expected_segments=$(ls parts*.tsv 2>/dev/null | wc -l | tr -d ' ')
125+
if [ "$expected_segments" -gt 0 ] && [ "$actual_segments" -ne "$expected_segments" ]; then
126+
echo "bench: LaunchDataIngestionJob produced ${actual_segments}/${expected_segments} segments — partial load (likely OOM swallowed per-task)" >&2
127+
exit 1
128+
fi
129+
130+
# Keep hits.tsv's derived parts only as long as needed; the source file
131+
# itself and the split parts are no longer needed once segments exist in
132+
# $SEGMENTS_DIR (used by the fast path above on every later cold cycle).
56133
rm -f hits.tsv parts*.tsv
57134
sync

‎pinot/queries.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ SELECT SearchEngineID, SearchPhrase, COUNT(*) AS c FROM hits WHERE SearchPhrase
1616
SELECT UserID, COUNT(*) FROM hits GROUP BY UserID ORDER BY COUNT(*) DESC LIMIT 10;
1717
SELECT UserID, SearchPhrase, COUNT(*) FROM hits GROUP BY UserID, SearchPhrase ORDER BY COUNT(*) DESC LIMIT 10;
1818
SELECT UserID, SearchPhrase, COUNT(*) FROM hits GROUP BY UserID, SearchPhrase LIMIT 10;
19-
SELECT UserID, extract(minute FROM EventTime) AS m, SearchPhrase, COUNT(*) FROM hits GROUP BY UserID, m, SearchPhrase ORDER BY COUNT(*) DESC LIMIT 10;
19+
SELECT UserID, extract(minute FROM CAST(EventTime AS TIMESTAMP)) AS m, SearchPhrase, COUNT(*) FROM hits GROUP BY UserID, m, SearchPhrase ORDER BY COUNT(*) DESC LIMIT 10;
2020
SELECT UserID FROM hits WHERE UserID = 435090932899640449;
2121
SELECT COUNT(*) FROM hits WHERE URL LIKE '%google%';
2222
SELECT SearchPhrase, MIN(URL), COUNT(*) AS c FROM hits WHERE URL LIKE '%google%' AND SearchPhrase <> '' GROUP BY SearchPhrase ORDER BY c DESC LIMIT 10;
@@ -40,4 +40,4 @@ SELECT URL, COUNT(*) AS PageViews FROM hits WHERE CounterID = 62 AND EventDate >
4040
SELECT TraficSourceID, SearchEngineID, AdvEngineID, CASE WHEN (SearchEngineID = 0 AND AdvEngineID = 0) THEN Referer ELSE '' END AS Src, URL AS Dst, COUNT(*) AS PageViews FROM hits WHERE CounterID = 62 AND EventDate >= '2013-07-01' AND EventDate <= '2013-07-31' AND IsRefresh = 0 GROUP BY TraficSourceID, SearchEngineID, AdvEngineID, Src, Dst ORDER BY PageViews DESC LIMIT 10 OFFSET 1000;
4141
SELECT URLHash, EventDate, COUNT(*) AS PageViews FROM hits WHERE CounterID = 62 AND EventDate >= '2013-07-01' AND EventDate <= '2013-07-31' AND IsRefresh = 0 AND TraficSourceID IN (-1, 6) AND RefererHash = 3594120000172545465 GROUP BY URLHash, EventDate ORDER BY PageViews DESC LIMIT 10 OFFSET 100;
4242
SELECT WindowClientWidth, WindowClientHeight, COUNT(*) AS PageViews FROM hits WHERE CounterID = 62 AND EventDate >= '2013-07-01' AND EventDate <= '2013-07-31' AND IsRefresh = 0 AND DontCountHits = 0 AND URLHash = 2868770270353813622 GROUP BY WindowClientWidth, WindowClientHeight ORDER BY PageViews DESC LIMIT 10 OFFSET 10000;
43-
SELECT DATE_TRUNC('minute', EventTime) AS M, COUNT(*) AS PageViews FROM hits WHERE CounterID = 62 AND EventDate >= '2013-07-14' AND EventDate <= '2013-07-15' AND IsRefresh = 0 AND DontCountHits = 0 GROUP BY DATE_TRUNC('minute', EventTime) ORDER BY DATE_TRUNC('minute', EventTime) LIMIT 10 OFFSET 1000;
43+
SELECT DATE_TRUNC('minute', CAST(EventTime AS TIMESTAMP)) AS M, COUNT(*) AS PageViews FROM hits WHERE CounterID = 62 AND EventDate >= '2013-07-14' AND EventDate <= '2013-07-15' AND IsRefresh = 0 AND DontCountHits = 0 GROUP BY DATE_TRUNC('minute', CAST(EventTime AS TIMESTAMP)) ORDER BY DATE_TRUNC('minute', CAST(EventTime AS TIMESTAMP)) LIMIT 10 OFFSET 1000;

‎pinot/query‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ query=$(printf '%s' "$query" | tr -d ';')
1212
req=$(printf '%s' "$query" | python3 -c '
1313
import json, sys
1414
q = sys.stdin.read()
15-
print(json.dumps({"sql": q + " option(timeoutMs=300000)"}))
15+
print(json.dumps({"sql": q + " option(timeoutMs=300000,autoRewriteAggregationType=true)"}))
1616
')
1717

1818
resp=$(curl -sS -XPOST -H'Content-Type: application/json' \

‎pinot/start‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,30 @@ set -e
44
PINOT_VERSION=1.5.1
55
PINOT_DIR="apache-pinot-$PINOT_VERSION-bin"
66

7-
# Idempotent: if broker query endpoint is up, do nothing.
7+
# Idempotent: if broker query endpoint is up, do nothing (leave the
8+
# existing pinot.log offset marker as-is; ./check will find the earlier
9+
# "setup complete" marker in the untouched log).
810
if curl -sf -o /dev/null -w '%{http_code}' \
911
-XPOST -H'Content-Type: application/json' \
1012
http://localhost:8000/query/sql/ \
1113
-d '{"sql":"SELECT 1"}' 2>/dev/null | grep -q '^200'; then
1214
exit 0
1315
fi
1416

17+
# Record the current end of pinot.log so ./check can look only at what
18+
# this invocation appends, not a "setup complete" line left over from an
19+
# earlier run of the log-appending (>>) QuickStart process.
20+
touch pinot.log
21+
wc -c < pinot.log | tr -d ' ' > pinot.log.offset
22+
23+
# pinot-admin.sh's own default is -Xms4G with NO -Xmx (falls back to the
24+
# JVM's default of ~25% of physical RAM). On a 100-segment/~74GB ClickBench
25+
# run this OOMed mid-LaunchDataIngestionJob around segment 60/100 on a
26+
# 30GB-RAM c6a.4xlarge (observed: OutOfMemoryError in
27+
# SegmentDictionaryCreator, job silently reported success anyway).
28+
# Force a real heap ceiling sized for this instance.
29+
export JAVA_OPTS="-Xms4G -Xmx20G"
30+
1531
nohup "./${PINOT_DIR}/bin/pinot-admin.sh" QuickStart -type batch \
1632
>> pinot.log 2>&1 < /dev/null &
1733
disown

‎pinot/stop‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,26 @@
33
pkill -f 'pinot-admin' 2>/dev/null || true
44
pkill -f 'pinot.tools.admin' 2>/dev/null || true
55
pkill -f 'org.apache.pinot' 2>/dev/null || true
6-
sleep 2
6+
7+
# pkill only sends SIGTERM; the actual teardown (JVM shutdown hook
8+
# stopping controller/broker/server, then the embedded Zookeeper
9+
# releasing its port) is asynchronous and takes several seconds. Two
10+
# things were previously observed to still be alive well after a fixed
11+
# short sleep or even after the broker itself stopped responding:
12+
# - the broker itself (was seen answering 200 for ~7-8s after SIGTERM)
13+
# - the embedded Zookeeper's port 2123, which a fixed sleep or a
14+
# broker-only check does not account for; the next ./start's fresh
15+
# Zookeeper can then fail to bind/connect on the still-held port,
16+
# surfacing as "ZkTimeoutException: Unable to connect to zookeeper
17+
# server within timeout" in the next start's controller.
18+
# The only reliable signal that teardown is actually finished is that
19+
# the pinot-admin JVM process itself is gone. Poll for that (up to 30s)
20+
# instead of guessing a delay or checking a single port.
21+
for _ in $(seq 1 30); do
22+
if ! pgrep -f 'org.apache.pinot.tools.admin.PinotAdministrator' > /dev/null 2>&1; then
23+
break
24+
fi
25+
sleep 1
26+
done
27+
728
exit 0

0 commit comments

Comments
 (0)