-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(adrs): add draft ArNS cache behavior ADR #246
Conversation
Adds an ADR that defines the set of parameters that control ArNS cache refresh behavior.
44fae89
to
426dc02
Compare
📝 Walkthrough📝 WalkthroughWalkthroughThe changes introduce a new architectural decision record (ADR) titled "ArNS Cache Timing" that outlines the caching mechanisms for ArNS resolution. It describes the balance between response latency and the freshness of responses through configurable caching levels. The document includes a flowchart detailing the resolution process, cache hit/miss conditions, and defines various caching-related timeouts and limits to aid gateway operators in optimizing their caching strategies. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
docs/madr/002-arns-cache-timing.md (4)
13-14
: Fix typo and improve readabilityThere's a typo in "opimtize" and the sentence can be more concise.
-In order to allow gateway operators to opimtize +To allow gateway operators to optimize🧰 Tools
🪛 LanguageTool
[style] ~13-~13: Consider a shorter alternative to avoid wordiness.
Context: ...d) and the freshness of ArNS responses. In order to allow gateway operators to opimtize thi...(IN_ORDER_TO_PREMIUM)
22-58
: Consider enhancing flowchart clarityThe flowchart effectively shows the caching layers, but consider adding:
- Notes linking "Up-to-date" states with their corresponding TTL parameters
- Visual indicators for the debouncing and concurrency limiting mechanisms
Example enhancement for the TTL relationship:
StateStatus -->|Up-to-date (within ANT state TTL)| Resolution([Resolution Returned])
64-65
: Fix typo in timeout description-The miminum amount of time +The minimum amount of time
62-71
: Consider adding parameter guidanceThe timeout parameters would benefit from:
- Default values or recommended ranges
- Examples of how different values might impact system behavior
- Guidance on tuning these parameters based on gateway operator needs
This would help operators make informed decisions when configuring their gateways.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
docs/madr/002-arns-cache-timing.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/madr/002-arns-cache-timing.md
[style] ~13-~13: Consider a shorter alternative to avoid wordiness.
Context: ...d) and the freshness of ArNS responses. In order to allow gateway operators to opimtize thi...
(IN_ORDER_TO_PREMIUM)
🔇 Additional comments (2)
docs/madr/002-arns-cache-timing.md (2)
1-7
: LGTM! Metadata section follows MADR format.
73-76
: LGTM! References are properly formatted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
docs/madr/002-arns-cache-timing.md (5)
3-6
: Consider adding more context about the decision statusSince this is a proposed ADR, it would be helpful to include:
- Expected timeline for decision
- Prerequisites or dependencies for the decision
- Next steps in the decision process
14-16
: Improve concisenessConsider rephrasing for better clarity:
-In order to allow gateway operators to optimize this trade-off we need to define the +To help gateway operators optimize this trade-off, we need to define the🧰 Tools
🪛 LanguageTool
[style] ~14-~14: Consider a shorter alternative to avoid wordiness.
Context: ...d) and the freshness of ArNS responses. In order to allow gateway operators to optimize thi...(IN_ORDER_TO_PREMIUM)
23-59
: Enhance flowchart documentationThe flowchart effectively illustrates the resolution flow, but would benefit from:
- A legend explaining the color coding (pink/blue nodes)
- Documentation about the expected behavior at each decision point
- Explanation of failure scenarios and retry mechanisms
63-74
: Add operational context for timeout valuesConsider enhancing the timeout documentation with:
- Rationale behind the default values
- Impact of adjusting these values
- Recommendations for different deployment scenarios (e.g., high-traffic vs. low-traffic gateways)
- Monitoring recommendations for these parameters
1-79
: Consider adding security implicationsThe ADR would benefit from a section addressing security considerations:
- Cache poisoning prevention
- DoS protection mechanisms
- Impact of caching on privacy
- Security implications of cache invalidation
🧰 Tools
🪛 LanguageTool
[style] ~14-~14: Consider a shorter alternative to avoid wordiness.
Context: ...d) and the freshness of ArNS responses. In order to allow gateway operators to optimize thi...(IN_ORDER_TO_PREMIUM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/madr/002-arns-cache-timing.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/madr/002-arns-cache-timing.md
[style] ~14-~14: Consider a shorter alternative to avoid wordiness.
Context: ...d) and the freshness of ArNS responses. In order to allow gateway operators to optimize thi...
(IN_ORDER_TO_PREMIUM)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #246 +/- ##
========================================
Coverage 71.43% 71.43%
========================================
Files 35 35
Lines 8861 8861
Branches 519 519
========================================
Hits 6330 6330
Misses 2529 2529
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Adds an ADR that defines the set of parameters that control ArNS cache refresh behavior.