Add a request_async_id to the async requests #842
Conversation
📝 WalkthroughWalkthroughThis PR extends FFI protocol messages with an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FfiServer
participant AsyncTask as Async Handler
Note over Client,AsyncTask: New Flow: Request Correlation
Client->>FfiServer: Request with request_async_id = 42
FfiServer->>FfiServer: resolve_async_id(42) → 42
FfiServer->>AsyncTask: Spawn task with async_id = 42
AsyncTask->>AsyncTask: Execute operation
AsyncTask->>FfiServer: Emit callback(async_id = 42)
FfiServer->>Client: Send callback with id = 42
Note over Client,AsyncTask: Old Flow: Auto-Generated IDs
Client->>FfiServer: Request
FfiServer->>FfiServer: next_id() → 99
FfiServer->>AsyncTask: Spawn task with async_id = 99
AsyncTask->>AsyncTask: Execute operation
AsyncTask->>FfiServer: Emit callback(async_id = 99)
FfiServer->>Client: Send callback with id = 99
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
🧰 Additional context used🧬 Code graph analysis (2)livekit-ffi/src/server/audio_source.rs (1)
livekit-ffi/src/server/data_stream.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (49)
✏️ Tip: You can disable this entire section by setting Comment |
ladvoc
left a comment
ladvoc
left a comment
There was a problem hiding this comment.
LGTM ✅, just a few unnecessary clones that should be removed prior to merging.
| let topic = publish.topic; | ||
| let destination_identities = publish.destination_identities; | ||
| let async_id = server.next_id(); | ||
| let topic = publish.topic.clone(); |
There was a problem hiding this comment.
suggestion: Cloning topic and destination_identities is not necessary.
| let digit = publish.digit; | ||
| let destination_identities = publish.destination_identities; | ||
| let async_id = server.next_id(); | ||
| let digit = publish.digit.clone(); |
This PR will allows client SDKs to generate an async id for the async requests, and start a listener before passing the requests, so that it could fix the race conditions.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.