-
Notifications
You must be signed in to change notification settings - Fork 531
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: create rfc 008 Dataplane #2841
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
📝 WalkthroughWalkthroughThe pull request introduces a new RFC document titled "0008 Dataplane" in the engineering content directory. The document outlines a proposed globally distributed architecture for Unkey, focusing on a dataplane that operates independently from the primary database to enhance availability. It presents four architectural options for implementing the dataplane, each with distinct advantages and challenges related to complexity, cache management, and infrastructure requirements. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Dataplane
participant CacheLayer
participant PrimaryStorage
Client->>Dataplane: Request Data
Dataplane->>CacheLayer: Check Cache
alt Cache Hit
CacheLayer-->>Dataplane: Return Cached Data
Dataplane-->>Client: Respond with Cached Data
else Cache Miss
Dataplane->>PrimaryStorage: Fetch Data
PrimaryStorage-->>Dataplane: Return Data
Dataplane->>CacheLayer: Store in Cache
Dataplane-->>Client: Respond with Data
end
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 (
|
Thank you for following the naming conventions for pull request titles! 🙏 |
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
🧹 Nitpick comments (6)
apps/engineering/content/rfcs/0008-dataplane.mdx (6)
4-4
: Update the date to the current date.The document is dated
2025-01-26
, which is in the future. Consider updating it to the current date to avoid confusion.-date: 2025-01-26 +date: 2024-01-26
10-10
: Fix grammar in the introduction.The phrase "independently from" should be "independently of" for correct collocation.
-We need to design a globally distributed architecture for Unkey where the dataplane can operate independently from the primary database for improved availability. +We need to design a globally distributed architecture for Unkey where the dataplane can operate independently of the primary database for improved availability.🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: The usual collocation for “independently” is “of”, not “from”. Did you mean “independently of”?
Context: ...r Unkey where the dataplane can operate independently from the primary database for improved avail...(INDEPENDENTLY_FROM_OF)
66-66
: Add missing comma for better readability.Add a comma after "gossip" to improve sentence structure.
-Efficient cache invalidation through gossip allowing higher TTLs. +Efficient cache invalidation through gossip, allowing higher TTLs.🧰 Tools
🪛 LanguageTool
[uncategorized] ~66-~66: Possible missing comma found.
Context: ... - Efficient cache invalidation through gossip allowing higher TTLs. - Reduced load on...(AI_HYDRA_LEO_MISSING_COMMA)
99-102
: Fix formatting issues in the cache nodes description.There are a few formatting issues to address:
- Remove the double dots after "TBD"
- Consider expanding "TBD" for clarity
-gateways. TBD.. +gateways. To be determined.🧰 Tools
🪛 LanguageTool
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the s3 bucket,...(DOUBLE_PUNCTUATION)
103-104
: Capitalize S3 for consistency.Use consistent capitalization for AWS service names.
-Everything here, except the s3 bucket, would be duplicated per region. +Everything here, except the S3 bucket, would be duplicated per region.
82-96
: Consider addressing load balancer failure scenarios.The architecture diagram shows a load balancer as a critical component, but the pros/cons don't address its potential failure modes or redundancy strategy. Consider adding details about:
- Load balancer redundancy
- Failure detection and failover mechanisms
- Impact on gateway routing during load balancer failures
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/engineering/content/rfcs/0008-dataplane.mdx
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/engineering/content/rfcs/0008-dataplane.mdx
[grammar] ~10-~10: The usual collocation for “independently” is “of”, not “from”. Did you mean “independently of”?
Context: ...r Unkey where the dataplane can operate independently from the primary database for improved avail...
(INDEPENDENTLY_FROM_OF)
[uncategorized] ~66-~66: Possible missing comma found.
Context: ... - Efficient cache invalidation through gossip allowing higher TTLs. - Reduced load on...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the s3 bucket,...
(DOUBLE_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test GO API Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/engineering/content/rfcs/0008-dataplane.mdx (1)
14-14
: Consider clarifying the availability goal.The goal "100% dataplane availability" might be unrealistic as true 100% availability is generally unattainable. Consider specifying a more realistic target (e.g., "99.99% availability") or clarifying that this is an aspirational goal.
…into rfc-0008-dataplane
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
🧹 Nitpick comments (5)
apps/engineering/content/rfcs/0008-dataplane.mdx (5)
4-4
: Update the date to the current date.The document is dated January 26, 2025, which is in the future. Please update this to the current date.
-date: 2025-01-26 +date: 2024-01-26
14-14
: Consider a more realistic availability target.Stating "100% availability" might set unrealistic expectations. Even the most robust systems typically aim for "99.99%" or "five nines" (99.999%) availability. Consider rephrasing to reflect a more achievable target.
-Achieve 100% dataplane availability independent of primary database +Achieve high availability (99.99%+) for the dataplane independent of primary database
71-73
: Address network partition scenarios.The gossip protocol approach should include considerations for handling network partitions and split-brain scenarios. Consider adding details about:
- How the system maintains consistency during network partitions
- Recovery mechanisms when partitioned nodes rejoin
- Conflict resolution strategies
99-101
: Complete the TBD section and fix typographical error.The section about cache nodes needs completion. Also, there's a double dot after "TBD".
-gateways. TBD.. +gateways. [TODO: Elaborate on cache freshness strategy and potential optimizations]🧰 Tools
🪛 LanguageTool
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the S3 bucket,...(DOUBLE_PUNCTUATION)
111-111
: Consider adding recommendation and timeline sections.The RFC would benefit from:
- A clear recommendation among the three options with justification
- An implementation timeline or phased approach
- Success metrics and monitoring strategy
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/engineering/content/rfcs/0008-dataplane.mdx
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/engineering/content/rfcs/0008-dataplane.mdx
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the S3 bucket,...
(DOUBLE_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test GO API Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/engineering/content/rfcs/0008-dataplane.mdx (1)
39-39
: Specify recommended TTL ranges.The current description mentions "really low TTLs (<10s)" as a con. It would be helpful to specify what TTL ranges would be acceptable for different types of data to help with implementation decisions.
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
🧹 Nitpick comments (5)
apps/engineering/content/rfcs/0008-dataplane.mdx (5)
4-4
: Update the date to the current date.The document is dated in the future (2025-01-26). Please update it to the current date.
-date: 2025-01-26 +date: 2024-01-26
14-19
: Consider adding specific SLOs/SLIs for the goals.The goals would benefit from specific, measurable targets. For example:
- Define the target latency for "fast access"
- Specify the maximum acceptable propagation delay
- Set thresholds for storage load
142-144
: Add cost analysis comparison for Option 4B.The document mentions cost implications are debatable. Consider adding:
- Estimated cost comparison between DynamoDB direct access vs. cached approach
- Cache hit rate assumptions and their impact on cost
- Break-even point analysis
101-101
: Fix double punctuation.Replace the double dots with a single period.
-TBD.. +TBD.🧰 Tools
🪛 LanguageTool
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the S3 bucket,...(DOUBLE_PUNCTUATION)
173-176
: Consolidate duplicate points about replication.The cons section repeats points about replication costs and concerns. Consider consolidating these points:
- Vendor lock-in to AWS -> we need to have an abstraction - Higher storage cost compared to S3 due to replication - - Cost of replication - - Replication lag is controlled by AWS, not us + - Additional costs and lag from replication (controlled by AWS)🧰 Tools
🪛 LanguageTool
[duplication] ~175-~175: Possible typo: you repeated a word.
Context: ...ared to S3 due to replication - Cost of replication - Replication lag is controlled by AWS, not us(ENGLISH_WORD_REPEAT_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/engineering/content/rfcs/0008-dataplane.mdx
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/engineering/content/rfcs/0008-dataplane.mdx
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the S3 bucket,...
(DOUBLE_PUNCTUATION)
[duplication] ~175-~175: Possible typo: you repeated a word.
Context: ...ared to S3 due to replication - Cost of replication - Replication lag is controlled by AWS, not us
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Test Packages / Test ./packages/rbac
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test GO API Local / test_agent_local
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/engineering/content/rfcs/0008-dataplane.mdx (1)
99-101
: Complete the TBD section for Option 3.The discussion about cache nodes' freshness/staleness is incomplete. Consider elaborating on:
- Expected cache hit rates
- Cache invalidation strategies
- Impact on consistency guarantees
Would you like me to help draft a more detailed analysis of these aspects?
🧰 Tools
🪛 LanguageTool
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the S3 bucket,...(DOUBLE_PUNCTUATION)
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: 1
🧹 Nitpick comments (5)
apps/engineering/content/rfcs/0008-dataplane.mdx (5)
14-19
: Consider adding measurable metrics to goals.The goals are well-defined but could benefit from specific, measurable metrics. For example:
- What is the target latency for "fast access"?
- What is the acceptable propagation time for data?
- What is the target reduction in storage load?
99-101
: Fix the ellipsis in TBD note.There's a double period after "TBD" which should be a single period.
-TBD.. +TBD.🧰 Tools
🪛 LanguageTool
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the S3 bucket,...(DOUBLE_PUNCTUATION)
173-177
: Avoid duplicate points in cons list.The cost of replication is mentioned twice:
- "Higher storage cost compared to S3 due to replication"
- "Cost of replication"
Consider consolidating these points into a single, comprehensive point about replication costs.
🧰 Tools
🪛 LanguageTool
[duplication] ~175-~175: Possible typo: you repeated a word.
Context: ...ared to S3 due to replication - Cost of replication - Replication lag is controlled by AWS, not us - More...(ENGLISH_WORD_REPEAT_RULE)
113-177
: Consider adding a comparison table for DynamoDB options.To make it easier to compare Options 4A and 4B, consider adding a table that compares key metrics like:
- Cost implications
- Latency expectations
- Operational complexity
- Cache hit ratios
🧰 Tools
🪛 LanguageTool
[duplication] ~175-~175: Possible typo: you repeated a word.
Context: ...ared to S3 due to replication - Cost of replication - Replication lag is controlled by AWS, not us - More...(ENGLISH_WORD_REPEAT_RULE)
9-177
: Consider adding implementation timeline and migration strategy sections.The RFC would benefit from additional sections:
- Implementation Timeline: Phased approach for rolling out the chosen solution
- Migration Strategy: How to transition from the current architecture
- Monitoring & Metrics: Key metrics to track for the chosen solution
- Rollback Plan: Strategy for handling potential issues
🧰 Tools
🪛 LanguageTool
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the S3 bucket,...(DOUBLE_PUNCTUATION)
[duplication] ~175-~175: Possible typo: you repeated a word.
Context: ...ared to S3 due to replication - Cost of replication - Replication lag is controlled by AWS, not us - More...(ENGLISH_WORD_REPEAT_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/engineering/content/rfcs/0008-dataplane.mdx
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/engineering/content/rfcs/0008-dataplane.mdx
[typographical] ~101-~101: Two consecutive dots
Context: ...freshness/staleness as the gateways. TBD.. Everything here, except the S3 bucket,...
(DOUBLE_PUNCTUATION)
[duplication] ~175-~175: Possible typo: you repeated a word.
Context: ...ared to S3 due to replication - Cost of replication - Replication lag is controlled by AWS, not us - More...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test GO API Local / test_agent_local
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Packages / Test ./internal/hash
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
--- | ||
title: 0008 Dataplane | ||
description: Global Unkey Deployment Architecture | ||
date: 2025-01-26 |
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.
Update the date field.
The date is set to 2025-01-26, which is in the future. Please update it to the current date.
-date: 2025-01-26
+date: 2024-01-26
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
date: 2025-01-26 | |
date: 2024-01-26 |
Summary by CodeRabbit