*: update rocksdb to downgrade log-level of some noisy logs (#440) - #860
Conversation
Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
📝 WalkthroughWalkthroughThe pull request updates the ChangesRocksDB submodule update
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: 🟡 Moderate · up to This change reduces default visibility into PrefetchTail behavior without adding the proposed replacement statistics, which could make tail-prefetch issues harder to diagnose in production. The PR should not merge until the replacement metrics are added or the observability tradeoff is explicitly accepted. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: glorv, overvenus The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aa78fbc7-6a8b-412a-9539-a9872495352e
📒 Files selected for processing (1)
librocksdb_sys/rocksdb
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.
| @@ -1 +1 @@ | |||
| Subproject commit 0da2397eeb4b40b534ed586c13fb0b795438b2ce | |||
| Subproject commit 37ab6137347bb125d17723a180fde4d1e1e0c3d4 | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rocksdb=librocksdb_sys/rocksdb
commit=37ab6137347bb125d17723a180fde4d1e1e0c3d4
git -C "$rocksdb" cat-file -e "${commit}^{commit}"
git -C "$rocksdb" show --format= "$commit" -- \
table/block_based/block_based_table_reader.cc
if ! git -C "$rocksdb" grep -nE \
'TAIL_PREFETCH_SIZE_(FROM_MANIFEST|FROM_STATS|FROM_HEURISTICS)' \
"$commit" -- \
include/rocksdb/statistics.h \
table/block_based/block_based_table_reader.cc; then
echo "Replacement tail-prefetch tickers are missing"
exit 1
fiRepository: tikv/rust-rocksdb
Length of output: 331
🏁 Script executed:
#!/usr/bin/env bash
set -u
rocksdb=librocksdb_sys/rocksdb
commit=37ab6137347bb125d17723a180fde4d1e1e0c3d4
printf '%s\n' '== submodule state =='
git submodule status -- "$rocksdb" || true
git -C "$rocksdb" rev-parse HEAD 2>&1 || true
git -C "$rocksdb" cat-file -t "$commit" 2>&1 || true
git -C "$rocksdb" show-ref | grep -E '37ab6137347bb125d17723a180fde4d1e1e0c3d4|refs/(heads|remotes|tags)' | head -40 || true
printf '%s\n' '== GitHub commit API =='
curl -LfsS --max-time 20 \
-H 'Accept: application/vnd.github+json' \
'https://api.github.com/repos/tikv/rocksdb/commits/37ab6137347bb125d17723a180fde4d1e1e0c3d4' |
python3 -c '
import json, sys
try:
d=json.load(sys.stdin)
print(json.dumps({
"sha": d.get("sha"),
"message": d.get("commit", {}).get("message"),
"files": [
{"filename": f.get("filename"), "patch": f.get("patch")}
for f in d.get("files", [])
if f.get("filename") in {
"table/block_based/block_based_table_reader.cc",
"include/rocksdb/statistics.h"
}
]
}, indent=2))
except Exception as e:
print(e)
sys.exit(1)
' 2>&1 || trueRepository: tikv/rust-rocksdb
Length of output: 1703
🏁 Script executed:
#!/usr/bin/env bash
set -u
owner=tikv
repo=rocksdb
commit=37ab6137347bb125d17723a180fde4d1e1e0c3d4
printf '%s\n' '== commit files =='
curl -LfsS --max-time 30 \
-H 'Accept: application/vnd.github+json' \
"https://api.github.com/repos/$owner/$repo/commits/$commit" |
python3 -c '
import json, sys
d=json.load(sys.stdin)
print("sha:", d.get("sha"))
for f in d.get("files", []):
print(f["filename"])
' 2>&1 || true
printf '%s\n' '== matching tree paths =='
curl -LfsS --max-time 30 \
-H 'Accept: application/vnd.github+json' \
"https://api.github.com/repos/$owner/$repo/git/trees/$commit?recursive=1" |
python3 -c '
import json, sys
d=json.load(sys.stdin)
for x in d.get("tree", []):
p=x.get("path","")
if any(s in p.lower() for s in ("statistics", "block_based_table_reader")):
print(p)
' 2>&1 || true
printf '%s\n' '== issue 439 =='
curl -LfsS --max-time 30 \
-H 'Accept: application/vnd.github+json' \
"https://api.github.com/repos/$owner/$repo/issues/439" |
python3 -c '
import json, sys
d=json.load(sys.stdin)
print("title:", d.get("title"))
print("state:", d.get("state"))
print(d.get("body") or "")
' 2>&1 || true
printf '%s\n' '== source identifiers at commit =='
for path in \
include/rocksdb/statistics.h \
table/block_based/block_based_table_reader.cc \
include/rocksdb/statistics.h.in \
monitoring/statistics.h
do
printf '%s\n' "--- $path ---"
curl -LfsS --max-time 30 \
"https://raw.githubusercontent.com/$owner/$repo/$commit/$path" |
grep -nE 'TAIL_PREFETCH|Tail prefetch|PrefetchTail|ROCKS_LOG_(WARN|DEBUG)' || true
doneRepository: tikv/rust-rocksdb
Length of output: 7026
Preserve the tail-prefetch signal in Statistics before demoting these logs.
The commit demotes the heuristic and TailPrefetchStats messages to ROCKS_LOG_DEBUG, so the default INFO_LEVEL no longer exposes this diagnostic signal. Issue #439 proposes TAIL_PREFETCH_SIZE_FROM_MANIFEST, TAIL_PREFETCH_SIZE_FROM_STATS, and TAIL_PREFETCH_SIZE_FROM_HEURISTICS, but this commit defines or increments none. Add and increment all three tickers, or land the replacement separately before this submodule update.
Source: MCP tools
There was a problem hiding this comment.
Since this logging is not helpful for the performance diagnosis or debugging bugs, we've determined to downgrade the logging level in warning level directly.
Descriptions
Ref tikv/rocksdb#439
Downgrade the log-level of noisy logs when calling
PrefetchTailwhere the inner updates of rocksdb can be reviewed in tikv/rocksdb#440Summary by CodeRabbit