Repository navigation
perf(getlogs): reduce partition reads and disabled log conversion - #430
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
wanliqun
force-pushed
the
codex/getlogs-performance
branch
from
October 27, 2026 09:14
be32bbd to
f64678f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An ordered getLogs query spanning partitions could load almost another 10,000 payload rows just to discover the 10,001st result. Disabled Geth logs also converted fields before Logrus discarded them. This change limits each ordered partition read to the remaining result allowance and checks the live Logrus level before conversion.
Both Core and eSpace retain existing result limits, suggested ranges, error logging, and complete reads when bound checks are disabled. Suggestion retry configuration remains supported. Main already provides indexed-query contexts and shared retry budgets through #428; this PR adds regression coverage for those behaviors and preserves its single-unit complete-read policy.
Commit breakdown
Compatibility
10001 - len(result), retaining one row to detect overflow. With bound checks disabled, the supplied limit is ignored and all keyset pages are read, including more than 10,000 rows.Local measurements
Measured on macOS/arm64, Apple M3 Pro, Go 1.23.10, with SQLite in memory. Each payload is 256 bytes. Each benchmark used 20 iterations; timings are illustrative, not production MySQL throughput or P95/P99 measurements.
For the cross-partition case, allocated bytes decreased from 18,922,097 to 9,402,821 per operation, and allocations from 409,335 to 200,045. Disabled Geth Debug logs at Info level decreased from 2,944 bytes and 40 allocations per call to zero bytes and zero allocations (3,994 to 29.15 ns/op in this short sample). Small-query time increased in this sample despite unchanged reads and allocations; these short runs do not establish a latency regression or meet the production rollout criterion below.
The single-partition oversized benchmark still rejects through the existing lightweight precheck, so its payload count is zero before and after. This PR does not eliminate
OFFSET 10000validation scans or reduce SQL statement counts. Dense filtered queries can still be expensive; evaluate MySQL execution plans and scanned rows using representative production filters before changing indexes or query structure.Reproduce the benchmarks on the baseline-fixture commit and on the completed branch with:
Validation and rollout
Regression tests cover 0/9,999/10,000/10,001 results, empty partitions, overflow suggestions, topic filters, all three ordered routes, complete reads with bound checks disabled, cancellation during precheck/payload/connection-pool wait, suggestion retry counts of 0/1/3, and shared reorg deadlines. Main's existing integration/context tests also cover fullnode calls and request-local client copies. Logging tests cover live levels, skipped conversion, and grouped/connection fields.
Validation passed:
Deploy to one instance first. Compare the same request distribution and offered load with the old version: normal-request P95/P99 (target no more than 5% regression), RPC failures/timeouts, database connection-pool wait, CPU/GC, and MySQL execution counts/scanned rows. Confirm that overload subsides after traffic drops. Local SQLite benchmarks do not establish these production acceptance criteria.
Keep suggestion retries at 0 during overload where operationally acceptable. Confirm that the effective rate strategy contains
cfx_getLogs_qps/eth_getLogs_qps; these changes do not modify production rate rules. Revert the PR to roll back; no database migration is required.This change is