chore: update brpc for eventfd worker wakeup - #540
Conversation
WalkthroughThe pull request updates the brpc dependency manifest and submodule pointer to commit Changesbrpc dependency update
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Poem
🚥 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 |
021556b to
8d0f325
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@third_party/manifest.yml`:
- Around line 46-47: Update the brpc revision in third_party/manifest.yml from
c12822d4931a59a9c2cc42df061975e114e93590 to
a192af98010f873f669652aa5d07c31ceee19521, and align the corresponding brpc
gitlink in .gitmodules with the same commit so scripts/third_party/fetch.sh
checks out the intended worker-wakeup implementation.
- Around line 46-47: Document the compatibility constraint for the pinned bRPC
commit c12822d4931a59a9c2cc42df061975e114e93590 in the manifest or its brpc
documentation entry. State that it enables io_uring eventfd worker wakeups,
while non-io_uring processes retain the legacy polling/condition-variable path,
and include the amd64 and arm64 CI matrices validating both paths.
In `@third_party/src/brpc`:
- Line 1: Update the brpc gitlink at third_party/src/brpc and its corresponding
pin in manifest.yml to the authoritative SHA
a192af98010f873f669652aa5d07c31ceee19521, replacing
c12822d4931a59a9c2cc42df061975e114e93590, then verify both locations match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c0977e7-4be9-41b5-a6a3-e739af3e76bd
📒 Files selected for processing (2)
third_party/manifest.ymlthird_party/src/brpc
| @@ -1 +1 @@ | |||
| Subproject commit 342603c95c33b8e03a89ca222c62d0e5e1263713 | |||
| Subproject commit c12822d4931a59a9c2cc42df061975e114e93590 | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
expected='a192af98010f873f669652aa5d07c31ceee19521'
test "$(git rev-parse HEAD:third_party/src/brpc)" = "$expected"
test "$(git -C third_party/src/brpc rev-parse HEAD)" = "$expected"
git -C third_party/src/brpc cat-file -e "$expected^{commit}"
rg -n -C 4 'brpc|a192af98010f873f669652aa5d07c31ceee19521|c12822d4931a59a9c2cc42df061975e114e93590' third_party/manifest.ymlRepository: eloqdata/tx_service
Length of output: 157
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Gitlink in tree:\n'
git ls-tree HEAD third_party/src/brpc || true
printf '\nBare gitlink from current index:\n'
git cat-file -t HEAD:third_party/src/brpc && git cat-file -p HEAD:third_party/src/brpc
printf '\nSubmodule working tree gitlink status:\n'
(test -d .git/modules/third_party/src/brpc && git -C .git/modules/third_party/src/brpc rev-parse HEAD) || true
(test -d .git/modules/third_party/src/brpc && git -C .git/modules/third_party/src/brpc rev-parse HEAD 2>/dev/null | tee /tmp/actual_brpc_sha) || true
printf '\nCommit existence checks:\n'
for sha in c12822d4931a59a9c2cc42df061975e114e93590 a192af98010f873f669652aa5d07c31ceee19521; do
printf '%s: ' "$sha"
git cat-file -e "$sha^{commit}" 2>/dev/null && echo existing || echo missing
done
printf '\nManifest occurrences:\n'
git ls-files | rg '(^|/)third_party/manifest\.ya?ml$|brpc' || true
if [ -f third_party/manifest.yml ]; then
rg -n -C 3 'brpc|a192af98010f873f669652aa5d07c31ceee19521|c12822d4931a59a9c2cc42df061975e114e93590' third_party/manifest.yml || true
fiRepository: eloqdata/tx_service
Length of output: 1611
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Submodule remote config:\n'
git submodule status --init 2>&1 || true
git config --file .gitmodules --get-regexp 'submodule\.third_party/src/brpc\..*' || true
git config --show-proxy .remote.origin.url || true
printf '\nFetch upstream brpc refs for target commits if reachable:\n'
git ls-remote --exit-code https://github.com/eloqdata/brpc.git \
c12822d4931a59a9c2cc42df061975e114e93590 \
a192af98010f873f669652aa5d07c31ceee19521 \
refs/heads/master \
refs/tags/* 2>&1 || true
printf '\nShort submodule diff around third_party/src:\n'
git diff --shortstat HEAD^..HEAD 2>/dev/null || true
git diff --stat -- third_party/src/brpc third_party/manifest.yml 2>/dev/null || trueRepository: eloqdata/tx_service
Length of output: 4713
Align the brpc pin with the PR objective before merge.
The gitlink at third_party/src/brpc and third_party/manifest.yml:47 both record c12822d4931a59a9c2cc42df061975e114e93590, while the PR objective uses a192af98010f873f669652aa5d07c31ceee19521. Use the authoritive SHA across both locations, then re-check the manifest pin.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@third_party/src/brpc` at line 1, Update the brpc gitlink at
third_party/src/brpc and its corresponding pin in manifest.yml to the
authoritative SHA a192af98010f873f669652aa5d07c31ceee19521, replacing
c12822d4931a59a9c2cc42df061975e114e93590, then verify both locations match.
Context
Consume the eventfd worker-wakeup support merged in eloqdata/brpc#25 and run it through Data Substrate's CI matrices.
Changes
third_party/src/brpctoc12822d4931a59a9c2cc42df061975e114e93590on brpcmaster.third_party/manifest.yml;.gitmodulesandgitlinks are now their repository and version sources of truth.
third_party/srcentries for mode160000and include thosegitlink SHAs in the CI third-party cache key.
There is no Data Substrate API, transaction, durability, recovery, or data-format change.
Validation
The full build and test matrix is running in this PR's CI. Parent integration runs in EloqKV #554 and EloqDoc #487.
Risk and rollback
Risk is limited to brpc worker idle waiting and shutdown when io_uring is enabled. Non-io_uring processes retain the condition-variable path. Revert this PR to restore the previous brpc gitlink and manifest pin.
Reviewer guide
Verify
third_party/src/brpcat brpcmastercommitc12822d4931a59a9c2cc42df061975e114e93590, and review.github/actions/setup-third-party/action.ymlfor mode-160000gitlink hashing. Functional review is in the merged brpc PR #25.