indexer: filter query - fix cancel#2488
indexer: filter query - fix cancel#2488tuxcanfly wants to merge 5 commits intoethereum-optimism:developfrom
Conversation
🦋 Changeset detectedLatest commit: b6c89ec The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This PR changes implementation code, but doesn't include a changeset. Did you forget to add one? |
mslipper
left a comment
mslipper
left a comment
There was a problem hiding this comment.
We should manually call ‘cancel()’ since the ‘defer’ is in a loop. Using defer in a loop can lead to resource exhaustion in and of itself since the defers will stack up until the function that started the loop returns.
| End: &end, | ||
| Context: s.ctx, | ||
| Start: start, | ||
| End: &end, |
There was a problem hiding this comment.
Why don't we want to pass these Contexts in?
There was a problem hiding this comment.
This context was not being used at all. We're creating a new context for every request and setting it as opts.Context = ctxt in filter.go.
Inphi
left a comment
Inphi
left a comment
There was a problem hiding this comment.
Looks good. Just a minor comment about simplifying the code.
| logger.Error("Error fetching filter", "err", err) | ||
| return res, nil | ||
| } | ||
| cancel() |
There was a problem hiding this comment.
we're canceling on both code paths. can be simplified per my commend above
| logger.Error("Error fetching filter", "err", err) | ||
| return res, nil | ||
| } | ||
| cancel() |
There was a problem hiding this comment.
we're canceling on both code paths. It would be simpler to cancel right after L22 and remove the cancel() call here and on L24.
| logger.Error("Error fetching filter", "err", err) | ||
| return res, nil | ||
| } | ||
| cancel() |
There was a problem hiding this comment.
we're canceling on both code paths. can be simplified per my commend above
| logger.Error("Error fetching filter", "err", err) | ||
| return res, nil | ||
| } | ||
| cancel() |
There was a problem hiding this comment.
we're canceling on both code paths. can be simplified per my commend above
|
Holding out on this because I am to repro the context deadline issue locally. Should have another fix coming up so it should be easier to get both in together. |
|
Closing in favor of #2496 |
This PR adds comprehensive documentation for the Kona Node SDK runtime system in the `docs/` directory, addressing the need for clear, concise runtime documentation. ## Changes Made ### New Runtime Documentation (`docs/docs/pages/node/design/runtime.mdx`) - **Architecture Overview**: Detailed explanation of the three core components (RuntimeConfig, RuntimeLoader, RuntimeActor) - **Configuration Management**: How runtime parameters are loaded from L1 contracts and cached efficiently - **Integration Patterns**: How the runtime system integrates with the broader node service architecture - **Error Handling**: Comprehensive coverage of error scenarios and recovery mechanisms - **Monitoring & Observability**: Guidance on logging, metrics, and operational monitoring - **Best Practices**: Recommendations for configuration intervals and monitoring setup - **Troubleshooting**: Common issues and debugging guidance ### Documentation Fixes - Fixed broken internal links in `docs/docs/pages/node/design/intro.mdx` (removed `.md` extensions for proper routing) ## Key Features The documentation emphasizes: :::info[Textual Focus] Prioritizes clear textual explanations over code snippets, with strategic use of callouts for important information ::: - **Concise Coverage**: Comprehensive yet focused content covering all runtime system aspects - **Practical Guidance**: Real-world configuration recommendations and troubleshooting tips - **Integration Context**: Clear explanation of how runtime fits into the overall node architecture - **Operational Focus**: Emphasis on monitoring, error handling, and best practices for production use The runtime system documentation now provides developers with a complete understanding of: - How dynamic configuration management works - L1 contract integration for protocol version signaling - Caching strategies and performance optimization - Production deployment considerations This addresses the gap in SDK documentation and provides the foundation for developers building custom rollup nodes with Kona. <!-- START COPILOT CODING AGENT TIPS --> --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: refcell <21288394+refcell@users.noreply.github.com>
3 participants
Description
The indexer was frequently stalling with
Context deadline exceeded. I found that thecancelfunction wasn't always being executed. Simplified the err handling around Filter Query and replacedswitch/casewithdefer cancel()andif/err.