Repository navigation
perf(bigquery): remove tokio::spawn from AppendWithOffset - #6937
Conversation
product-auto-label
Bot
added
the
api: bigquery
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors AppendFuture to support wrapping either a oneshot receiver or a boxed future via an internal Inner enum, introducing a new from_future constructor. Consequently, the send method in AppendWithOffset is updated to return an AppendFuture constructed directly from an async block instead of spawning a background Tokio task. This change allows polling the future outside of a Tokio runtime, which is verified by new unit tests. There are no review comments, so no additional feedback is provided.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6937 +/- ##
=======================================
Coverage 97.18% 97.18%
=======================================
Files 325 325
Lines 110565 110619 +54
=======================================
+ Hits 107452 107506 +54
Misses 3113 3113 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
dbolduc
left a comment
dbolduc
left a comment
There was a problem hiding this comment.
I had to think about whether behavior has changed and I think the answer is no. The request is immediately sent on the channel, serviced by a Runner (an already spawned task). So we still get the eager execution without needing to await the future.
So I think this is just a win. Thanks!
| use crate::model::append_rows_request::MissingValueInterpretation; | ||
|
|
||
| let (req_tx, _req_rx) = mpsc::unbounded_channel(); | ||
| let invalid: MissingValueInterpretation = serde_json::from_str("\"INVALID\"").unwrap(); |
There was a problem hiding this comment.
comment: Interesting. I never thought about how to force this error.
| async move { Err(AppendError::UnexpectedEndOfStream) }, | ||
| ); | ||
| } | ||
| AppendFuture::from_future(async move { |
In
AppendWithOffset::send(), eliminate the intermediate oneshot channel and backgroundtokio::spawntask by returning an inline async future viaAppendFuture::from_future.Fixes #6898