-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
chore(architecture): consolidate sink I/O driver logic into reusable component #9215
Merged
Merged
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
69c398a
chore(architecture): consolidate sink I/O driver logic into reusable …
tobz eec948f
Merge branch 'master' into tobz/streamify-new-sink-io-task
tobz c968754
cleaning up clippy nits
tobz ae2de65
temporary commit to avoid losing too much work before more futzing
tobz ef8dd9e
ahhh it compiles
tobz 7e239f6
Merge branch 'master' into tobz/streamify-new-sink-io-task
tobz 1bfd7d4
Update src/sinks/aws_s3/sink.rs
tobz 55f8062
PR feedback
tobz 45d95cd
formatting and spelling errors and what not
tobz 021a974
Merge branch 'master' of github.com:vectordotdev/vector into tobz/str…
tobz 05bc5f3
address PR feedback
tobz 101d461
fix more nits and lints
tobz f4c2196
fix nit
tobz 8957e8f
try it this way
tobz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
pub mod batcher; | ||
pub mod driver; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
use std::{collections::HashMap, fmt, marker::PhantomData}; | ||
|
||
use buffers::{Ackable, Acker}; | ||
use futures::{stream::FuturesUnordered, FutureExt, Stream, StreamExt, TryFutureExt}; | ||
use tokio::{pin, select, sync::oneshot}; | ||
use tower::{Service, ServiceExt}; | ||
use tracing::Instrument; | ||
|
||
use crate::event::{EventStatus, Finalizable}; | ||
|
||
/// Drives the interaction between a stream of items and a service which processes them | ||
/// asynchronously. | ||
/// | ||
/// `Driver`, as a high-level, facilitates taking items from an arbitrary `Stream` and pushing them | ||
/// through a `Service`, spawning each call to the service so that work can be run concurrently, | ||
/// managing waiting for the service to be ready before processing more items, and so on. | ||
/// | ||
/// Additionally, `Driver` handles two event-specific facilities: finalization and acknowledgement. | ||
/// | ||
/// This capability is parameterized so any implementation which can define how to interpret the | ||
/// response for each request, as well as define how many events a request is compromised of, can be | ||
/// used with `Driver`. | ||
pub struct Driver<St, Svc, Req> | ||
where | ||
Svc: Service<Req>, | ||
{ | ||
input: St, | ||
service: Svc, | ||
acker: Acker, | ||
_req: PhantomData<Req>, | ||
} | ||
|
||
impl<St, Svc, Req> Driver<St, Svc, Req> | ||
where | ||
Svc: Service<Req>, | ||
{ | ||
pub fn new(input: St, service: Svc, acker: Acker) -> Self { | ||
Self { | ||
input, | ||
service, | ||
acker, | ||
_req: PhantomData, | ||
} | ||
} | ||
} | ||
|
||
impl<St, Svc, Req> Driver<St, Svc, Req> | ||
where | ||
St: Stream<Item = Req>, | ||
Svc: Service<Req>, | ||
Svc::Error: fmt::Debug + 'static, | ||
Svc::Future: Send + 'static, | ||
Svc::Response: AsRef<EventStatus>, | ||
Req: Ackable + Finalizable, | ||
{ | ||
/// Runs the driver until the input stream is exhausted. | ||
/// | ||
/// All in-flight calls to the provided `service` will also be completed before `run` returns. | ||
/// | ||
/// # Errors | ||
/// | ||
/// No errors are currently returned. Te return type is purely to simplify caller code, but may | ||
/// return an error for a legitimate reason in the future. | ||
pub async fn run(self) -> Result<(), ()> { | ||
let in_flight = FuturesUnordered::new(); | ||
let mut pending_acks = HashMap::new(); | ||
let mut seq_head: u64 = 0; | ||
let mut seq_tail: u64 = 0; | ||
|
||
let Self { | ||
input, | ||
mut service, | ||
acker, | ||
.. | ||
} = self; | ||
|
||
pin!(input); | ||
pin!(in_flight); | ||
|
||
loop { | ||
select! { | ||
tobz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// We've received an item from the input stream. | ||
Some(req) = input.next() => { | ||
// Rebind the variable to avoid a bug with the pattern matching | ||
// in `select!`: https://github.com/tokio-rs/tokio/issues/4076 | ||
let mut req = req; | ||
let seqno = seq_head; | ||
seq_head += 1; | ||
|
||
let (tx, rx) = oneshot::channel(); | ||
tobz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
in_flight.push(rx); | ||
|
||
trace!( | ||
message = "Submitting service request.", | ||
in_flight_requests = in_flight.len() | ||
); | ||
let ack_size = req.ack_size(); | ||
let finalizers = req.take_finalizers(); | ||
|
||
let svc = service.ready().await.expect("should not get error when waiting for svc readiness"); | ||
let fut = svc.call(req) | ||
.err_into() | ||
.map(move |result: Result<Svc::Response, Svc::Error>| { | ||
let status = match result { | ||
Err(error) => { | ||
error!(message = "Service call failed.", ?error, seqno); | ||
EventStatus::Failed | ||
}, | ||
Ok(response) => { | ||
trace!(message = "Service call succeeded.", seqno); | ||
*response.as_ref() | ||
} | ||
}; | ||
finalizers.update_status(status); | ||
|
||
// The receiver could drop before we reach this point if Driver` | ||
// goes away as part of a sink closing. We can't do anything | ||
// about it, so just silently ignore the error. | ||
let _ = tx.send((seqno, ack_size)); | ||
}) | ||
.instrument(info_span!("request", request_id = %seqno)); | ||
tokio::spawn(fut); | ||
}, | ||
|
||
// One of our service calls has completed. | ||
Some(Ok((seqno, ack_size))) = in_flight.next() => { | ||
trace!(message = "Sending request.", seqno, ack_size); | ||
pending_acks.insert(seqno, ack_size); | ||
|
||
let mut num_to_ack = 0; | ||
while let Some(ack_size) = pending_acks.remove(&seq_tail) { | ||
num_to_ack += ack_size; | ||
seq_tail += 1; | ||
} | ||
|
||
if num_to_ack > 0 { | ||
trace!(message = "Acking events.", ack_size = num_to_ack); | ||
acker.ack(num_to_ack); | ||
} | ||
}, | ||
|
||
else => break | ||
} | ||
} | ||
|
||
Ok(()) | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Small note that we could probably parameterize this with twox hash or some such if we wanted.
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.
I had the exact same thought. There's actually a
nohash-hasher
crate since our keys are alreadyu64
. I guess if we've both had this thought, maybe I should just go ahead and do it.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.
TIL about nohash-hasher.
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.
This is almost a glorified
VecDeque
. I wonder if it could be emulated with a wrapper aroundVecDequeue<Option<(u64, u64)>>
where inserting inserts "blanks" if it's not actually sequentially next.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.
That's a good point regarding
VecDeque
. I'll have to think about your idea... off the top of my head, I can't visualize it, so might just be one of those things I need to write down first to grok.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.
In 05bc5f3, I reworked this as a
BinaryHeap
because I didn't like the idea of having to insert blanks that we would then had to do a linear scan to find when the call finishes and we want to mark the pending acknowledgement as completed.Admittedly, though, the code using
BinaryHeap
is longer than theHashMap
approach.