From 6159c1330fb3a499157508b690f9a0ef2a8e80af Mon Sep 17 00:00:00 2001 From: Jansen Date: Wed, 2 Sep 2026 11:05:01 -0400 Subject: [PATCH 1/5] fix(buffers): respect when_full on disk exhaustion --- Cargo.lock | 1 + lib/vector-buffers/Cargo.toml | 3 + lib/vector-buffers/src/internal_events.rs | 45 +- .../src/topology/channel/sender.rs | 695 ++++++++++++-- .../src/topology/channel/tests.rs | 666 +++++++++++++- .../src/variants/disk_v2/common.rs | 2 +- lib/vector-buffers/src/variants/disk_v2/io.rs | 330 ++++++- .../src/variants/disk_v2/ledger.rs | 32 +- .../src/variants/disk_v2/mod.rs | 5 +- .../src/variants/disk_v2/tests/mod.rs | 209 ++++- .../variants/disk_v2/tests/model/common.rs | 3 +- .../disk_v2/tests/model/filesystem.rs | 220 ++++- .../src/variants/disk_v2/tests/model/mod.rs | 6 +- .../disk_v2/tests/runtime_capacity.rs | 725 +++++++++++++++ .../src/variants/disk_v2/writer.rs | 848 +++++++++++++++--- 15 files changed, 3531 insertions(+), 259 deletions(-) create mode 100644 lib/vector-buffers/src/variants/disk_v2/tests/runtime_capacity.rs diff --git a/Cargo.lock b/Cargo.lock index 2a0b822d35376..ca41ddc7aabf3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13506,6 +13506,7 @@ dependencies = [ "fslock", "futures", "hdrhistogram", + "libc", "memmap2", "metrics", "metrics-tracing-context", diff --git a/lib/vector-buffers/Cargo.toml b/lib/vector-buffers/Cargo.toml index 9baa08fb1e430..b0e71b101c2ca 100644 --- a/lib/vector-buffers/Cargo.toml +++ b/lib/vector-buffers/Cargo.toml @@ -20,6 +20,7 @@ crossbeam-utils.workspace = true derivative.workspace = true fslock = { version = "0.2.1", default-features = false, features = ["std"] } futures.workspace = true +libc.workspace = true memmap2 = { version = "0.9.10", default-features = false } metrics.workspace = true num-traits = { version = "0.2.19", default-features = false } @@ -51,9 +52,11 @@ rand.workspace = true serde_json.workspace = true serde_yaml.workspace = true temp-dir = "0.2.0" +tokio = { workspace = true, features = ["test-util"] } tokio-test.workspace = true tracing-fluent-assertions = { version = "0.3" } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt", "registry", "std", "ansi"] } +vector-common = { path = "../vector-common", default-features = false, features = ["test"] } [[bench]] name = "sized_records" diff --git a/lib/vector-buffers/src/internal_events.rs b/lib/vector-buffers/src/internal_events.rs index e05ecd182afa6..5613e91ee9afb 100644 --- a/lib/vector-buffers/src/internal_events.rs +++ b/lib/vector-buffers/src/internal_events.rs @@ -1,4 +1,4 @@ -use std::time::Duration; +use std::{path::Path, time::Duration}; use metrics::Histogram; use vector_common::NamedInternalEvent; @@ -8,6 +8,49 @@ use vector_common::{ registered_event, }; +#[derive(Debug, NamedInternalEvent)] +pub struct DiskBufferBackpressure<'a> { + pub operation: &'static str, + pub error: &'a std::io::Error, + pub retry_delay: Duration, + pub buffer_path: &'a Path, +} + +impl InternalEvent for DiskBufferBackpressure<'_> { + fn emit(self) { + warn!( + message = "Disk buffer is waiting for filesystem capacity.", + operation = self.operation, + error = %self.error, + error_kind = ?self.error.kind(), + retry_delay_ms = self.retry_delay.as_millis(), + buffer_path = %self.buffer_path.display(), + internal_log_rate_limit = false, + ); + } +} + +#[derive(Debug, NamedInternalEvent)] +pub struct DiskBufferBackpressureRecovered<'a> { + pub operation: &'static str, + pub retries: u64, + pub duration: Duration, + pub buffer_path: &'a Path, +} + +impl InternalEvent for DiskBufferBackpressureRecovered<'_> { + fn emit(self) { + info!( + message = "Disk buffer filesystem capacity recovered.", + operation = self.operation, + retries = self.retries, + duration_ms = self.duration.as_millis(), + buffer_path = %self.buffer_path.display(), + internal_log_rate_limit = false, + ); + } +} + #[derive(NamedInternalEvent)] pub struct BufferCreated { pub buffer_id: String, diff --git a/lib/vector-buffers/src/topology/channel/sender.rs b/lib/vector-buffers/src/topology/channel/sender.rs index 1735cee9785ae..5e67044bf7751 100644 --- a/lib/vector-buffers/src/topology/channel/sender.rs +++ b/lib/vector-buffers/src/topology/channel/sender.rs @@ -1,12 +1,21 @@ // Derivative's Debug impl generates 'let _ = field.fmt(f)' which triggers this lint. #![allow(clippy::let_underscore_must_use)] -use std::{sync::Arc, time::Instant}; +use std::{ + fmt, io, + sync::{Arc, Mutex as StdMutex}, + time::{Duration, Instant}, +}; use async_recursion::async_recursion; use derivative::Derivative; -use tokio::sync::Mutex; +use futures::future::BoxFuture; +use tokio::{ + sync::{Mutex, Notify}, + time::sleep, +}; use tracing::Span; +use vector_common::finalization::EventStatus; use vector_common::internal_event::{InternalEventHandle, Registered, register}; use super::limited_queue::LimitedSender; @@ -14,9 +23,544 @@ use crate::{ BufferInstrumentation, Bufferable, WhenFull, buffer_usage_data::BufferUsageHandle, internal_events::BufferSendDuration, - variants::disk_v2::{self, ProductionFilesystem, TryWriteOutcome}, + variants::disk_v2::{ + self, CapacityProgress, Filesystem, ProductionFilesystem, TryWriteOutcome, + }, }; +#[cfg(test)] +use crate::variants::disk_v2::tests::model::filesystem::TestFilesystem; + +const CAPACITY_RETRY_INITIAL: Duration = Duration::from_millis(100); +const CAPACITY_RETRY_MAX: Duration = Duration::from_secs(5); + +#[derive(Debug)] +enum CapacityState { + Ready, + Retrying, + Failed(String), +} + +#[cfg(test)] +#[derive(Debug, Default)] +pub(crate) struct CapacityBlockedHook { + pub(crate) detected: Notify, + pub(crate) resume: Notify, +} + +struct UnownedFinalizers(Option); + +impl UnownedFinalizers { + fn take(item: &mut T) -> Self { + Self(Some(item.take_finalizer_groups())) + } + + fn restore(mut self, item: &mut T) { + item.merge_finalizer_groups(self.0.take().expect("finalizers must exist")); + } +} + +impl Drop for UnownedFinalizers { + fn drop(&mut self) { + if let Some(finalizers) = self.0.as_mut() { + finalizers.update_status(EventStatus::Errored); + } + } +} + +trait SenderFilesystem: Filesystem + fmt::Debug + Clone + Sized + 'static +where + T: Bufferable, + Self::File: Unpin, +{ + fn try_write_record( + writer: &mut disk_v2::BufferWriter, + item: T, + ) -> BoxFuture<'_, Result, disk_v2::WriterError>>; + + fn try_flush( + writer: &mut disk_v2::BufferWriter, + ) -> BoxFuture<'_, io::Result>; + + fn retry_capacity( + writer: &mut disk_v2::BufferWriter, + ) -> BoxFuture<'_, io::Result>; +} + +macro_rules! impl_sender_filesystem { + ($filesystem:ty) => { + impl SenderFilesystem for $filesystem { + fn try_write_record( + writer: &mut disk_v2::BufferWriter, + item: T, + ) -> BoxFuture<'_, Result, disk_v2::WriterError>> { + Box::pin(writer.try_write_record(item)) + } + + fn try_flush( + writer: &mut disk_v2::BufferWriter, + ) -> BoxFuture<'_, io::Result> { + Box::pin(writer.try_flush()) + } + + fn retry_capacity( + writer: &mut disk_v2::BufferWriter, + ) -> BoxFuture<'_, io::Result> { + Box::pin(writer.retry_capacity()) + } + } + }; +} + +impl_sender_filesystem!(ProductionFilesystem); +#[cfg(test)] +impl_sender_filesystem!(TestFilesystem); + +#[derive(Debug)] +pub struct DiskV2Sender +where + T: Bufferable, + FS: Filesystem, + FS::File: fmt::Debug + Unpin, +{ + writer: Mutex>, + usage: BufferUsageHandle, + capacity_state: StdMutex, + capacity_notify: Notify, + #[cfg(test)] + capacity_blocked_hook: StdMutex>>, +} + +struct WriterProgressOnCancel<'a, T, FS> +where + T: Bufferable, + FS: SenderFilesystem, + FS::File: fmt::Debug + Unpin, +{ + state: &'a Arc>, + armed: bool, +} + +impl<'a, T, FS> WriterProgressOnCancel<'a, T, FS> +where + T: Bufferable, + FS: SenderFilesystem, + FS::File: fmt::Debug + Unpin, +{ + fn new(state: &'a Arc>) -> Self { + Self { state, armed: true } + } + + fn disarm(&mut self) { + self.armed = false; + } +} + +impl Drop for WriterProgressOnCancel<'_, T, FS> +where + T: Bufferable, + FS: SenderFilesystem, + FS::File: fmt::Debug + Unpin, +{ + fn drop(&mut self) { + if self.armed { + self.state.start_cancellation_retry(); + } + } +} + +#[allow(private_bounds)] +impl DiskV2Sender +where + T: Bufferable, + FS: SenderFilesystem, + FS::File: fmt::Debug + Unpin, +{ + fn new(writer: disk_v2::BufferWriter) -> Self { + let usage = writer.usage_handle(); + Self { + writer: Mutex::new(writer), + usage, + capacity_state: StdMutex::new(CapacityState::Ready), + capacity_notify: Notify::new(), + #[cfg(test)] + capacity_blocked_hook: StdMutex::new(None), + } + } + + #[cfg(test)] + async fn pause_after_capacity_blocked_for_test(&self) { + let hook = self.capacity_blocked_hook.lock().expect("poisoned").clone(); + if let Some(hook) = hook { + hook.detected.notify_waiters(); + hook.resume.notified().await; + } + } + + fn check_terminal_error(&self) -> crate::Result<()> { + if let CapacityState::Failed(error) = &*self.capacity_state.lock().expect("poisoned") { + return Err(io::Error::other(error.clone()).into()); + } + Ok(()) + } + + fn is_capacity_blocked(&self) -> bool { + matches!( + *self.capacity_state.lock().expect("poisoned"), + CapacityState::Retrying + ) + } + + fn start_capacity_retry(self: &Arc) { + let should_start = { + let mut capacity_state = self.capacity_state.lock().expect("poisoned"); + match *capacity_state { + CapacityState::Ready => { + *capacity_state = CapacityState::Retrying; + true + } + CapacityState::Retrying | CapacityState::Failed(_) => false, + } + }; + if should_start { + self.capacity_notify.notify_waiters(); + self.spawn_capacity_retry_driver(None); + } + } + + fn start_cancellation_retry(self: &Arc) { + let (retained_driver, classifier) = { + let mut capacity_state = self.capacity_state.lock().expect("poisoned"); + match *capacity_state { + CapacityState::Ready => { + *capacity_state = CapacityState::Retrying; + (Some(Arc::clone(self)), None) + } + CapacityState::Retrying => (None, Some(Arc::clone(self))), + CapacityState::Failed(_) => return, + } + }; + if let Some(state) = retained_driver { + self.capacity_notify.notify_waiters(); + self.spawn_capacity_retry_driver(Some(state)); + } else if let Some(state) = classifier { + Self::classify_cancelled_retry(state); + } + } + + fn classify_cancelled_retry(state: Arc) { + tokio::spawn(async move { + let result = { + let mut writer = state.writer.lock().await; + state.retry_capacity_once(&mut writer).await + }; + + let _ = result; + }); + } + + async fn retry_capacity_once( + &self, + writer: &mut disk_v2::BufferWriter, + ) -> Option> { + if !matches!( + *self.capacity_state.lock().expect("poisoned"), + CapacityState::Retrying + ) { + return None; + } + + let result = FS::retry_capacity(writer).await; + match &result { + Ok(CapacityProgress::Ready) if !writer.is_capacity_blocked() => { + let mut capacity_state = self.capacity_state.lock().expect("poisoned"); + if matches!(*capacity_state, CapacityState::Retrying) { + *capacity_state = CapacityState::Ready; + self.capacity_notify.notify_waiters(); + } + } + Err(error) => { + writer.fail_pending_write(); + error!(%error, "Disk buffer capacity retry failed."); + let mut capacity_state = self.capacity_state.lock().expect("poisoned"); + if matches!(*capacity_state, CapacityState::Retrying) { + *capacity_state = CapacityState::Failed(error.to_string()); + self.capacity_notify.notify_waiters(); + } + } + Ok( + CapacityProgress::Blocked + | CapacityProgress::WaitingForReader + | CapacityProgress::Ready, + ) => {} + } + Some(result) + } + + fn spawn_capacity_retry_driver(self: &Arc, mut retained_state: Option>) { + let weak_state = Arc::downgrade(self); + tokio::spawn(async move { + let mut delay = CAPACITY_RETRY_INITIAL; + loop { + let Some(state) = retained_state.clone().or_else(|| weak_state.upgrade()) else { + return; + }; + let reader_progress = { + let writer = state.writer.lock().await; + writer.reader_progress_waiter() + }; + drop(state); + tokio::select! { + () = sleep(delay) => {} + () = reader_progress => {} + } + let Some(state) = retained_state.clone().or_else(|| weak_state.upgrade()) else { + return; + }; + let result = { + let mut writer = state.writer.lock().await; + state.retry_capacity_once(&mut writer).await + }; + let Some(result) = result else { + return; + }; + // A cancellation can leave a production syscall running after its caller drops. + // Keep its sender alive through the first retry result, then return to weak ownership. + retained_state = None; + match result { + Ok(CapacityProgress::Ready) => { + if state.is_capacity_blocked() { + continue; + } + return; + } + Ok(CapacityProgress::Blocked) => { + delay = delay.saturating_mul(2).min(CAPACITY_RETRY_MAX); + } + Ok(CapacityProgress::WaitingForReader) => {} + Err(_) => { + return; + } + } + } + }); + } + + async fn wait_for_capacity(&self) -> crate::Result<()> { + let notified = self.capacity_notify.notified(); + tokio::pin!(notified); + loop { + notified.as_mut().enable(); + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => return Ok(()), + CapacityState::Failed(error) => { + return Err(io::Error::other(error.clone()).into()); + } + CapacityState::Retrying => {} + } + notified.as_mut().await; + notified.set(self.capacity_notify.notified()); + } + } + + async fn try_send_record(self: &Arc, item: T) -> crate::Result> { + self.check_terminal_error()?; + let pre_count = item.event_count() as u64; + let pre_size = item.size_of() as u64; + let Some(mut item) = item.filter_unencodable() else { + self.usage + .increment_received_event_count_and_byte_size(pre_count, pre_size); + self.usage + .increment_dropped_event_count_and_byte_size(pre_count, pre_size, false); + return Ok(TryWriteOutcome::Dropped); + }; + if item.event_count() as u64 != pre_count { + let dropped_events = pre_count - item.event_count() as u64; + let dropped_bytes = pre_size.saturating_sub(item.size_of() as u64); + self.usage + .increment_received_event_count_and_byte_size(dropped_events, dropped_bytes); + self.usage.increment_dropped_event_count_and_byte_size( + dropped_events, + dropped_bytes, + false, + ); + } + + let finalizers = UnownedFinalizers::take(&mut item); + let notified = self.capacity_notify.notified(); + tokio::pin!(notified); + let mut writer = loop { + notified.as_mut().enable(); + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => {} + CapacityState::Retrying => { + finalizers.restore(&mut item); + return Ok(TryWriteOutcome::Full(item)); + } + CapacityState::Failed(error) => { + return Err(io::Error::other(error.clone()).into()); + } + } + + tokio::select! { + biased; + () = notified.as_mut() => { + notified.set(self.capacity_notify.notified()); + } + writer = self.writer.lock() => { + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => break writer, + CapacityState::Retrying => { + drop(writer); + finalizers.restore(&mut item); + return Ok(TryWriteOutcome::Full(item)); + } + CapacityState::Failed(error) => { + return Err(io::Error::other(error.clone()).into()); + } + } + } + } + }; + if writer.is_capacity_blocked() { + #[cfg(test)] + self.pause_after_capacity_blocked_for_test().await; + self.start_capacity_retry(); + drop(writer); + finalizers.restore(&mut item); + return Ok(TryWriteOutcome::Full(item)); + } + + finalizers.restore(&mut item); + let mut progress_on_cancel = WriterProgressOnCancel::new(self); + let result = FS::try_write_record(&mut writer, item).await; + progress_on_cancel.disarm(); + let outcome = result.inspect_err(|_error| { + error!("Disk buffer writer has encountered an unrecoverable error."); + })?; + let blocked = writer.is_capacity_blocked(); + if blocked { + #[cfg(test)] + self.pause_after_capacity_blocked_for_test().await; + self.start_capacity_retry(); + } + drop(writer); + Ok(outcome) + } + + async fn send_record(self: &Arc, item: T) -> crate::Result> { + let mut item = item; + loop { + match self.try_send_record(item).await? { + TryWriteOutcome::Written => return Ok(TryWriteOutcome::Written), + TryWriteOutcome::Dropped => return Ok(TryWriteOutcome::Dropped), + TryWriteOutcome::Pending => { + self.wait_for_capacity().await?; + return Ok(TryWriteOutcome::Written); + } + TryWriteOutcome::Full(mut returned) => { + let finalizers = UnownedFinalizers::take(&mut returned); + item = returned; + if self.is_capacity_blocked() { + self.wait_for_capacity().await?; + } else { + let writer = self.writer.lock().await; + let reader_progress = writer.reader_progress_waiter(); + drop(writer); + reader_progress.await; + } + finalizers.restore(&mut item); + } + } + } + } + + async fn flush(self: &Arc, block: bool) -> crate::Result<()> { + loop { + let retrying = { + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => false, + CapacityState::Retrying => true, + CapacityState::Failed(error) => { + return Err(io::Error::other(error.clone()).into()); + } + } + }; + if retrying { + if !block { + return Ok(()); + } + self.wait_for_capacity().await?; + continue; + } + + let mut writer = self.writer.lock().await; + let retrying = { + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => false, + CapacityState::Retrying => true, + CapacityState::Failed(error) => { + return Err(io::Error::other(error.clone()).into()); + } + } + }; + if retrying { + drop(writer); + if !block { + return Ok(()); + } + self.wait_for_capacity().await?; + continue; + } + + // This guard only covers filesystem I/O. Cancelling while waiting for capacity must not + // start a second capacity retry driver. + let mut progress_on_cancel = WriterProgressOnCancel::new(self); + let result = FS::try_flush(&mut writer).await; + progress_on_cancel.disarm(); + let progress = result.map_err(|error| -> Box { + error!("Disk buffer writer has encountered an unrecoverable error."); + error.into() + })?; + + if matches!(progress, CapacityProgress::Blocked) { + // Publish Retrying before releasing the writer, so another flush cannot race in + // and issue another filesystem call before observing the capacity episode. + self.start_capacity_retry(); + } + drop(writer); + + if matches!(progress, CapacityProgress::Blocked) && block { + self.wait_for_capacity().await?; + continue; + } + return Ok(()); + } + } + + #[cfg(test)] + pub(crate) fn start_normal_capacity_retry_for_test(self: &Arc) { + self.start_capacity_retry(); + } + + #[cfg(test)] + pub(crate) fn start_cancellation_retry_for_test(self: &Arc) { + self.start_cancellation_retry(); + } + + #[cfg(test)] + pub(crate) fn set_capacity_blocked_hook(&self, hook: Arc) { + *self.capacity_blocked_hook.lock().expect("poisoned") = Some(hook); + } +} + +#[cfg(test)] +impl DiskV2Sender { + pub(crate) async fn next_writer_data_file_path(&self) -> std::path::PathBuf { + self.writer.lock().await.next_writer_data_file_path() + } +} + /// Adapter for papering over various sender backends. #[derive(Clone, Debug)] pub enum SenderAdapter { @@ -24,7 +568,10 @@ pub enum SenderAdapter { InMemory(LimitedSender), /// The disk v2 buffer. - DiskV2(Arc>>), + DiskV2(Arc>), + + #[cfg(test)] + DiskV2Test(Arc>), } impl From> for SenderAdapter { @@ -35,7 +582,14 @@ impl From> for SenderAdapter { impl From> for SenderAdapter { fn from(v: disk_v2::BufferWriter) -> Self { - Self::DiskV2(Arc::new(Mutex::new(v))) + Self::DiskV2(Arc::new(DiskV2Sender::new(v))) + } +} + +#[cfg(test)] +impl From> for SenderAdapter { + fn from(v: disk_v2::BufferWriter) -> Self { + Self::DiskV2Test(Arc::new(DiskV2Sender::new(v))) } } @@ -56,6 +610,8 @@ where match self { Self::InMemory(_) => false, Self::DiskV2(_) => true, + #[cfg(test)] + Self::DiskV2Test(_) => true, } } @@ -66,37 +622,9 @@ where .await .map(|()| TryWriteOutcome::Written) .map_err(Into::into), - Self::DiskV2(writer) => { - let pre_count = item.event_count() as u64; - let pre_size = item.size_of() as u64; - let mut writer = writer.lock().await; - - let Some(item) = item.filter_unencodable() else { - // The whole item was filtered out (e.g. every sub-item over the - // protobuf nesting budget). Report the drop directly via the - // ledger's usage handle so it shows up in the disk-v2 stage's - // `received` / `dropped` metrics — `BufferSender` does not carry - // its own handle for backends that `provides_instrumentation()`. - writer.track_dropped(pre_count, pre_size); - return Ok(TryWriteOutcome::Dropped); - }; - if item.event_count() as u64 != pre_count { - let dropped_events = pre_count - item.event_count() as u64; - let dropped_bytes = pre_size.saturating_sub(item.size_of() as u64); - writer.track_dropped(dropped_events, dropped_bytes); - } - - writer.write_record_outcome(item).await.map_err(|e| { - // Record-level failures that can never succeed (a record too large to encode - // within the max record size) are handled inside the writer and surfaced as a - // dropped outcome. Anything that reaches this point -- I/O errors, - // serialization failures, an inconsistent writer state -- is genuinely - // unrecoverable. - error!("Disk buffer writer has encountered an unrecoverable error."); - - e.into() - }) - } + Self::DiskV2(writer) => Arc::clone(writer).send_record(item).await, + #[cfg(test)] + Self::DiskV2Test(writer) => Arc::clone(writer).send_record(item).await, } } @@ -106,55 +634,18 @@ where .try_send(item) .map(|()| TryWriteOutcome::Written) .or_else(|e| Ok(TryWriteOutcome::Full(e.into_inner()))), - Self::DiskV2(writer) => { - let mut writer = writer.lock().await; - - // Filtering here is unconditional and independent of current occupancy. - // Whether an unencodable item should be dropped or handed to an overflow - // stage is a `WhenFull` policy decision, so it is made in `BufferSender` - // before the item ever reaches this backend: `WhenFull::Overflow` diverts - // items failing `is_fully_encodable` straight to the overflow stage, and - // anything arriving here is therefore expected to be persistable. Keeping - // the filter unconditional means a given item is treated the same at 99% - // full as at 100% full. - let pre_count = item.event_count() as u64; - let pre_size = item.size_of() as u64; - let Some(item) = item.filter_unencodable() else { - writer.track_dropped(pre_count, pre_size); - return Ok(TryWriteOutcome::Dropped); - }; - if item.event_count() as u64 != pre_count { - let dropped_events = pre_count - item.event_count() as u64; - let dropped_bytes = pre_size.saturating_sub(item.size_of() as u64); - writer.track_dropped(dropped_events, dropped_bytes); - } - - writer.try_write_record(item).await.map_err(|e| { - // Record-level failures that can never succeed (a record too large to encode - // within the max record size) are handled inside the writer and surfaced as a - // dropped outcome. Anything that reaches this point -- I/O errors, - // serialization failures, an inconsistent writer state -- is genuinely - // unrecoverable. - error!("Disk buffer writer has encountered an unrecoverable error."); - - e.into() - }) - } + Self::DiskV2(writer) => Arc::clone(writer).try_send_record(item).await, + #[cfg(test)] + Self::DiskV2Test(writer) => Arc::clone(writer).try_send_record(item).await, } } - pub(crate) async fn flush(&mut self) -> crate::Result<()> { + pub(crate) async fn flush(&mut self, block: bool) -> crate::Result<()> { match self { Self::InMemory(_) => Ok(()), - Self::DiskV2(writer) => { - let mut writer = writer.lock().await; - writer.flush().await.map_err(|e| { - // Errors on the I/O path, which is all that flushing touches, are never recoverable. - error!("Disk buffer writer has encountered an unrecoverable error."); - - e.into() - }) - } + Self::DiskV2(writer) => Arc::clone(writer).flush(block).await, + #[cfg(test)] + Self::DiskV2Test(writer) => Arc::clone(writer).flush(block).await, } } @@ -162,6 +653,28 @@ where match self { Self::InMemory(tx) => Some(tx.available_capacity()), Self::DiskV2(_) => None, + #[cfg(test)] + Self::DiskV2Test(_) => None, + } + } + + fn track_dropped_newest(&self, item_count: usize, item_size: usize) -> bool { + let usage = match self { + Self::DiskV2(state) => Some(&state.usage), + #[cfg(test)] + Self::DiskV2Test(state) => Some(&state.usage), + Self::InMemory(_) => None, + }; + if let Some(usage) = usage { + usage.increment_received_event_count_and_byte_size(item_count as u64, item_size as u64); + usage.increment_dropped_event_count_and_byte_size( + item_count as u64, + item_size as u64, + true, + ); + true + } else { + false } } } @@ -309,11 +822,23 @@ impl BufferSender { WhenFull::Block => match self.base.send(item).await? { TryWriteOutcome::Written => UsageAccounting::Accepted, TryWriteOutcome::Full(_) => unreachable!("blocking sends wait until space exists"), + TryWriteOutcome::Pending => { + unreachable!("blocking sends wait for pending writes") + } TryWriteOutcome::Dropped => UsageAccounting::NotAccepted, }, WhenFull::DropNewest => match self.base.try_send(item).await? { - TryWriteOutcome::Written => UsageAccounting::Accepted, - TryWriteOutcome::Full(_) => UsageAccounting::DroppedNewest, + TryWriteOutcome::Written | TryWriteOutcome::Pending => UsageAccounting::Accepted, + TryWriteOutcome::Full(item) => { + if self + .base + .track_dropped_newest(item.event_count(), item.size_of()) + { + UsageAccounting::NotAccepted + } else { + UsageAccounting::DroppedNewest + } + } TryWriteOutcome::Dropped => UsageAccounting::NotAccepted, }, WhenFull::Overflow => { @@ -337,7 +862,9 @@ impl BufferSender { UsageAccounting::NotAccepted } else { match self.base.try_send(item).await? { - TryWriteOutcome::Written => UsageAccounting::Accepted, + TryWriteOutcome::Written | TryWriteOutcome::Pending => { + UsageAccounting::Accepted + } TryWriteOutcome::Full(item) => { self.overflow .as_mut() @@ -373,7 +900,7 @@ impl BufferSender { #[async_recursion] pub async fn flush(&mut self) -> crate::Result<()> { - self.base.flush().await?; + self.base.flush(self.when_full == WhenFull::Block).await?; if let Some(overflow) = self.overflow.as_mut() { overflow.flush().await?; } diff --git a/lib/vector-buffers/src/topology/channel/tests.rs b/lib/vector-buffers/src/topology/channel/tests.rs index 7bee7e475c2b1..0d27867ee86be 100644 --- a/lib/vector-buffers/src/topology/channel/tests.rs +++ b/lib/vector-buffers/src/topology/channel/tests.rs @@ -1,18 +1,38 @@ use std::{ + io, sync::Arc, time::{Duration, Instant}, }; -use tokio::{pin, sync::Barrier, time::sleep}; +use temp_dir::TempDir; +use tokio::{ + pin, + sync::Barrier, + time::{advance, sleep, timeout}, +}; +use vector_common::finalization::{AddBatchNotifier, BatchNotifier, BatchStatus}; use crate::{ Bufferable, WhenFull, + buffer_usage_data::BufferUsageHandle, topology::{ - channel::{BufferReceiver, BufferSender}, + channel::{BufferReceiver, BufferSender, CapacityBlockedHook, SenderAdapter}, test_util::{assert_current_send_capacity, build_buffer}, }, + variants::disk_v2::{ + Buffer, DiskBufferConfigBuilder, ProductionFilesystem, StalledWrites, TestWriteGate, + tests::model::{filesystem::TestFilesystem, record::Record}, + }, }; +struct WriteGateCleanup(Arc); + +impl Drop for WriteGateCleanup { + fn drop(&mut self) { + self.0.release(); + } +} + async fn assert_send_ok_with_capacities( sender: &mut BufferSender, value: impl Into, @@ -270,3 +290,645 @@ async fn test_buffer_metrics_overflow_block() { assert_eq!(4, snapshot.sent_event_count); assert_eq!(0, snapshot.dropped_event_count_intentional); } + +// Physical disk exhaustion acts like a full buffer for records not yet owned by disk_v2. These +// tests verify each policy while partially written records recover exactly once. +async fn build_policy_disk( + filesystem: TestFilesystem, + write_buffer_size: usize, + rotating: bool, + when_full: WhenFull, +) -> ( + BufferSender, + crate::variants::disk_v2::BufferReader, + BufferUsageHandle, +) { + let directory = + std::env::temp_dir().join(format!("vector-policy-disk-{}", rand::random::())); + let mut builder = DiskBufferConfigBuilder::from_path(directory) + .write_buffer_size(write_buffer_size) + .filesystem(filesystem); + if rotating { + builder = builder.max_data_file_size(256).max_record_size(256); + } + let usage = BufferUsageHandle::noop(); + let (writer, reader, _) = Buffer::from_config_inner(builder.build().unwrap(), usage.clone()) + .await + .unwrap(); + ( + BufferSender::new(SenderAdapter::from(writer), when_full), + reader, + usage, + ) +} + +#[tokio::test] +async fn disk_capacity_block_retries_owned_record() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, mut reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::Block).await; + let record = Record::new(100, 256, 1); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + let mut send = Box::pin(sender.send(record.clone(), None)); + assert!( + timeout(Duration::from_millis(20), &mut send).await.is_err(), + "block policy must backpressure while the owned record retries" + ); + filesystem.restore_data_writes(); + timeout(Duration::from_secs(2), send) + .await + .expect("block send should recover") + .unwrap(); + sender.flush().await.unwrap(); + assert_eq!(reader.next().await.unwrap(), Some(record)); +} + +#[tokio::test] +async fn disk_capacity_drop_newest_is_prompt_and_counted_once() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, mut reader, usage) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let outer_usage = BufferUsageHandle::noop(); + sender.with_usage_instrumentation(outer_usage.clone()); + let owned = Record::new(101, 256, 1); + let dropped = Record::new(102, 64, 1); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + timeout(Duration::from_secs(1), sender.send(owned.clone(), None)) + .await + .expect("the owned drop-newest record should be accepted") + .unwrap(); + timeout(Duration::from_millis(20), sender.send(dropped, None)) + .await + .expect("drop-newest send must not wait for the retry driver") + .unwrap(); + assert_eq!(usage.snapshot().dropped_event_count_intentional, 1); + assert_eq!(outer_usage.snapshot().dropped_event_count_intentional, 0); + + filesystem.restore_data_writes(); + timeout(Duration::from_secs(2), async { + while usage.snapshot().received_event_count < 2 { + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("owned record should complete in the background"); + assert_eq!(reader.next().await.unwrap(), Some(owned)); + let snapshot = usage.snapshot(); + assert_eq!(snapshot.received_event_count, 2); + assert_eq!(snapshot.dropped_event_count_intentional, 1); +} + +#[tokio::test(start_paused = true)] +async fn disk_capacity_drop_newest_flush_skips_writer_while_retrying() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, _reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + sender.send(Record::new(120, 256, 1), None).await.unwrap(); + tokio::task::yield_now().await; + let write_attempts = filesystem.data_write_attempts(); + + for id in 121..124 { + sender.send(Record::new(id, 64, 1), None).await.unwrap(); + sender.flush().await.unwrap(); + } + assert_eq!(filesystem.data_write_attempts(), write_attempts); + + // The retry driver is the only path allowed to make another filesystem attempt, and it + // retains its exponential-backoff schedule while sends and flushes are dropped. + advance(Duration::from_millis(99)).await; + tokio::task::yield_now().await; + assert_eq!(filesystem.data_write_attempts(), write_attempts); + advance(Duration::from_millis(1)).await; + tokio::task::yield_now().await; + assert_eq!(filesystem.data_write_attempts(), write_attempts + 1); +} + +#[tokio::test] +async fn disk_capacity_send_publishes_retrying_before_releasing_writer() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, _reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match sender.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let hook = Arc::new(CapacityBlockedHook::default()); + state.set_capacity_blocked_hook(Arc::clone(&hook)); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + let detected = hook.detected.notified(); + tokio::pin!(detected); + detected.as_mut().enable(); + let send = tokio::spawn(async move { sender.send(Record::new(127, 256, 1), None).await }); + detected.await; + let write_attempts = filesystem.data_write_attempts(); + + let mut flusher = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::DropNewest); + let flush = tokio::spawn(async move { flusher.flush().await }); + tokio::task::yield_now().await; + hook.resume.notify_waiters(); + + send.await.unwrap().unwrap(); + flush.await.unwrap().unwrap(); + assert_eq!(filesystem.data_write_attempts(), write_attempts); +} + +#[tokio::test] +async fn disk_capacity_overflow_routes_from_cloned_sender() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, mut base_reader, base_usage) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let (overflow_sender, mut overflow_reader, overflow_usage) = + build_policy_disk(TestFilesystem::default(), 1024, false, WhenFull::Block).await; + sender.switch_to_overflow(overflow_sender); + let mut clone = sender.clone(); + let owned = Record::new(103, 256, 1); + let overflowed = Record::new(104, 64, 1); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + timeout(Duration::from_secs(1), sender.send(owned.clone(), None)) + .await + .expect("the owned overflow record should be accepted by the base") + .unwrap(); + timeout( + Duration::from_millis(20), + clone.send(overflowed.clone(), None), + ) + .await + .expect("cloned overflow sender must not wait for the base retry") + .unwrap(); + clone.flush().await.unwrap(); + timeout(Duration::from_secs(1), async { + while overflow_usage.snapshot().received_event_count < 1 { + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("overflowed record should be flushed"); + assert_eq!(overflow_reader.next().await.unwrap(), Some(overflowed)); + + filesystem.restore_data_writes(); + timeout(Duration::from_secs(2), async { + while base_usage.snapshot().received_event_count < 1 { + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("base owned record should recover"); + assert_eq!(base_reader.next().await.unwrap(), Some(owned)); +} + +#[tokio::test(start_paused = true)] +async fn disk_capacity_overflow_flushes_base_without_retrying_writer() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, _base_reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let (overflow_sender, mut overflow_reader, overflow_usage) = + build_policy_disk(TestFilesystem::default(), 1024, false, WhenFull::Block).await; + sender.switch_to_overflow(overflow_sender); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + sender.send(Record::new(124, 256, 1), None).await.unwrap(); + tokio::task::yield_now().await; + let write_attempts = filesystem.data_write_attempts(); + let overflowed = Record::new(125, 64, 1); + sender.send(overflowed.clone(), None).await.unwrap(); + sender.flush().await.unwrap(); + + assert_eq!(filesystem.data_write_attempts(), write_attempts); + assert_eq!(overflow_usage.snapshot().received_event_count, 1); + assert_eq!(overflow_reader.next().await.unwrap(), Some(overflowed)); +} + +#[tokio::test(start_paused = true)] +async fn disk_capacity_blocking_flush_waits_then_recovers() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut owner, mut reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match owner.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let mut blocker = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::Block); + let record = Record::new(126, 256, 1); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + owner.send(record.clone(), None).await.unwrap(); + tokio::task::yield_now().await; + let write_attempts = filesystem.data_write_attempts(); + + let mut flush = Box::pin(blocker.flush()); + tokio::select! { + result = &mut flush => panic!("capacity-blocked flush unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + assert_eq!(filesystem.data_write_attempts(), write_attempts); + + filesystem.restore_data_writes(); + advance(Duration::from_millis(100)).await; + flush.await.unwrap(); + assert_eq!(reader.next().await.unwrap(), Some(record)); +} + +#[tokio::test] +async fn disk_retry_waiting_for_reader_releases_sender_mutex() { + let filesystem = TestFilesystem::default(); + let (mut sender, _reader, _) = + build_policy_disk(filesystem.clone(), 1024, true, WhenFull::DropNewest).await; + for id in 105..107 { + sender.send(Record::new(id, 64, 1), None).await.unwrap(); + sender.flush().await.unwrap(); + } + let state = match sender.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let next_path = state.next_writer_data_file_path().await; + filesystem.fail_data_file_open(io::ErrorKind::StorageFull); + sender.send(Record::new(107, 64, 1), None).await.unwrap(); + filesystem.restore_data_file_open(); + filesystem.create_data_file_with_data(&next_path, b"occupied"); + sleep(Duration::from_millis(150)).await; + + let mut clone = sender.clone(); + timeout( + Duration::from_millis(20), + clone.send(Record::new(108, 64, 1), None), + ) + .await + .expect("retry waiting for reader must not hold the sender mutex") + .unwrap(); +} + +#[tokio::test] +async fn ordinary_capacity_retry_does_not_retain_dropped_sender() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let weak = match sender.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::downgrade(state), + _ => unreachable!("test disk sender expected"), + }; + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + sender.send(Record::new(109, 256, 1), None).await.unwrap(); + + drop(sender); + drop(reader); + timeout(Duration::from_millis(100), async { + while weak.upgrade().is_some() { + tokio::task::yield_now().await; + } + }) + .await + .expect("ordinary retry task must not retain the disk sender or writer"); +} + +#[tokio::test] +async fn cancelling_unowned_block_send_does_not_detach_or_duplicate_record() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut owner, mut reader, usage) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match owner.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let weak = Arc::downgrade(&state); + let mut blocker = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::Block); + let pending_record = Record::new(110, 256, 1); + let mut cancelled_record = Record::new(111, 64, 1); + let (batch, finalizer) = BatchNotifier::new_with_receiver(); + cancelled_record.add_batch_notifier(batch); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + owner.send(pending_record.clone(), None).await.unwrap(); + let send = tokio::spawn(async move { blocker.send(cancelled_record, None).await }); + sleep(Duration::from_millis(20)).await; + assert!( + !send.is_finished(), + "block send should be waiting for capacity" + ); + send.abort(); + assert!(send.await.unwrap_err().is_cancelled()); + assert_eq!( + timeout(Duration::from_secs(1), finalizer).await.unwrap(), + BatchStatus::Errored, + "cancelling an unowned send must nack it for source redelivery" + ); + + filesystem.restore_data_writes(); + timeout(Duration::from_secs(2), async { + while usage.snapshot().received_event_count < 1 { + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("writer-owned record should complete in the background"); + assert_eq!(reader.next().await.unwrap(), Some(pending_record)); + assert!( + timeout(Duration::from_millis(50), reader.next()) + .await + .is_err(), + "the cancelled unowned record must not be written later" + ); + + drop(owner); + drop(reader); + timeout(Duration::from_millis(100), async { + while weak.upgrade().is_some() { + tokio::task::yield_now().await; + } + }) + .await + .expect("cancelled block send must not retain the disk sender or writer"); +} + +#[tokio::test] +async fn terminal_capacity_retry_wakes_waiters_and_stays_failed() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut owner, reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match owner.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let mut blocker = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::Block); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + owner.send(Record::new(112, 256, 1), None).await.unwrap(); + + let waiting = tokio::spawn(async move { blocker.send(Record::new(113, 64, 1), None).await }); + sleep(Duration::from_millis(20)).await; + assert!(!waiting.is_finished(), "block send should await capacity"); + filesystem.fail_data_writes_after(0, io::ErrorKind::PermissionDenied); + assert!( + timeout(Duration::from_secs(1), waiting) + .await + .expect("terminal retry must wake the waiter") + .unwrap() + .is_err() + ); + assert!( + owner.send(Record::new(114, 64, 1), None).await.is_err(), + "terminal retry state must reject subsequent sends" + ); + drop(owner); + drop(reader); +} + +#[tokio::test] +async fn terminal_cancellation_retry_is_not_overwritten_by_queued_ready_retry() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, _reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match sender.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let mut record = Record::new(114, 256, 1); + let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); + record.add_batch_notifier(batch); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + timeout(Duration::from_secs(1), sender.send(record, None)) + .await + .expect("drop-newest should accept the writer-owned record") + .unwrap(); + + // Let the ordinary weak retry observe StorageFull and schedule its later retry. + sleep(Duration::from_millis(150)).await; + filesystem.fail_data_writes_after(0, io::ErrorKind::PermissionDenied); + state.start_cancellation_retry_for_test(); + assert_eq!( + timeout(Duration::from_secs(1), &mut finalizer) + .await + .expect("terminal classification should resolve the owned record once"), + BatchStatus::Errored + ); + + // The queued ordinary driver would get Ready from the restored filesystem if it did not + // recheck the terminal state while holding the writer mutex. + filesystem.restore_data_writes(); + sleep(Duration::from_millis(250)).await; + assert!( + sender.send(Record::new(115, 64, 1), None).await.is_err(), + "a stale Ready retry must not overwrite Failed" + ); +} + +#[tokio::test] +async fn cancelled_production_send_from_ready_retains_owner_until_syscall_is_classified() { + let directory = TempDir::with_prefix("vector-buffer-adapter-cancel").unwrap(); + let data_dir = directory.path().to_path_buf(); + let StalledWrites { filesystem, gate } = ProductionFilesystem::with_stalled_writes(); + let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); + let config = DiskBufferConfigBuilder::from_path(&data_dir) + .write_buffer_size(16) + .filesystem(filesystem.clone()) + .build() + .unwrap(); + let (writer, reader, _) = Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + let mut sender = BufferSender::new(SenderAdapter::from(writer), WhenFull::Block); + let weak = match sender.get_base_ref() { + SenderAdapter::DiskV2(state) => Arc::downgrade(state), + _ => unreachable!("production disk sender expected"), + }; + let mut record = Record::new(115, 256, 1); + let expected = record.clone(); + let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); + record.add_batch_notifier(batch); + + let mut send = Box::pin(sender.send(record, None)); + tokio::select! { + result = &mut send => panic!("gated send unexpectedly completed: {result:?}"), + () = gate.wait_until_started() => {} + } + drop(send); + // Cancellation registers its retained driver before a normal starter can observe Retrying. + match sender.get_base_ref() { + SenderAdapter::DiskV2(state) => state.start_normal_capacity_retry_for_test(), + _ => unreachable!("production disk sender expected"), + } + drop(sender); + drop(reader); + assert!( + weak.upgrade().is_some(), + "the cancellation retry must retain the sender while its syscall is gated" + ); + assert!( + timeout(Duration::from_millis(20), &mut finalizer) + .await + .is_err(), + "writer-owned finalizers must remain unresolved while the syscall is pending" + ); + + gate.release(); + assert_eq!( + timeout(Duration::from_secs(2), &mut finalizer) + .await + .expect("the recovered write should resolve its finalizer"), + BatchStatus::Delivered + ); + timeout(Duration::from_millis(100), async { + while weak.upgrade().is_some() { + tokio::task::yield_now().await; + } + }) + .await + .expect("the retry must release the sender after classifying the syscall"); + + let config = DiskBufferConfigBuilder::from_path(&data_dir) + .write_buffer_size(16) + .filesystem(filesystem) + .build() + .unwrap(); + let (recovered_writer, mut recovered_reader, _) = + Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + drop(recovered_writer); + assert_eq!( + timeout(Duration::from_secs(2), recovered_reader.next()) + .await + .expect("the recovered record should be readable") + .unwrap(), + Some(expected) + ); + assert_eq!( + timeout(Duration::from_secs(2), recovered_reader.next()) + .await + .expect("the recovered buffer should reach EOF") + .unwrap(), + None + ); +} + +#[tokio::test] +async fn cancelled_production_flush_while_retrying_retains_owner_until_classified() { + let directory = TempDir::with_prefix("vector-buffer-adapter-retrying-cancel").unwrap(); + let data_dir = directory.path().to_path_buf(); + let StalledWrites { filesystem, gate } = ProductionFilesystem::with_stalled_writes(); + let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); + let config = DiskBufferConfigBuilder::from_path(&data_dir) + .write_buffer_size(1024) + .filesystem(filesystem.clone()) + .build() + .unwrap(); + let (writer, reader, _) = Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + let mut sender = BufferSender::new(SenderAdapter::from(writer), WhenFull::Block); + let weak = match sender.get_base_ref() { + SenderAdapter::DiskV2(state) => Arc::downgrade(state), + _ => unreachable!("production disk sender expected"), + }; + let mut record = Record::new(119, 256, 1); + let expected = record.clone(); + let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); + record.add_batch_notifier(batch); + sender.send(record, None).await.unwrap(); + match sender.get_base_ref() { + SenderAdapter::DiskV2(state) => state.start_normal_capacity_retry_for_test(), + _ => unreachable!("production disk sender expected"), + } + + let mut flush = Box::pin(sender.flush()); + tokio::select! { + result = &mut flush => panic!("gated flush unexpectedly completed: {result:?}"), + () = gate.wait_until_started() => {} + } + drop(flush); + drop(sender); + drop(reader); + assert!( + weak.upgrade().is_some(), + "the cancellation classifier must retain the sender while its syscall is gated" + ); + + gate.release(); + assert_eq!( + timeout(Duration::from_secs(2), &mut finalizer) + .await + .expect("the accepted record should resolve its finalizer"), + BatchStatus::Delivered + ); + timeout(Duration::from_secs(2), async { + while weak.upgrade().is_some() { + tokio::task::yield_now().await; + } + }) + .await + .expect("the classifier must release the sender after classifying the syscall"); + + let config = DiskBufferConfigBuilder::from_path(&data_dir) + .write_buffer_size(1024) + .filesystem(filesystem) + .build() + .unwrap(); + let (recovered_writer, mut recovered_reader, _) = + Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + drop(recovered_writer); + assert_eq!( + timeout(Duration::from_secs(2), recovered_reader.next()) + .await + .expect("the recovered record should be readable") + .unwrap(), + Some(expected) + ); + assert_eq!( + timeout(Duration::from_secs(2), recovered_reader.next()) + .await + .expect("the recovered buffer should reach EOF") + .unwrap(), + None + ); +} + +#[tokio::test] +async fn cancelled_production_flush_recovers_accepted_record() { + let directory = TempDir::with_prefix("vector-buffer-adapter-flush-cancel").unwrap(); + let StalledWrites { filesystem, gate } = ProductionFilesystem::with_stalled_writes(); + let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); + let config = DiskBufferConfigBuilder::from_path(directory.path()) + .write_buffer_size(1024) + .filesystem(filesystem) + .build() + .unwrap(); + let (writer, mut reader, _) = Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + let mut sender = BufferSender::new(SenderAdapter::from(writer), WhenFull::Block); + let record = Record::new(118, 256, 1); + + sender.send(record.clone(), None).await.unwrap(); + let mut flush = Box::pin(sender.flush()); + tokio::select! { + result = &mut flush => panic!("gated flush unexpectedly completed: {result:?}"), + () = gate.wait_until_started() => {} + } + drop(flush); + gate.release(); + + assert_eq!( + timeout(Duration::from_secs(2), reader.next()) + .await + .expect("cancelled flush should be recovered in the background") + .unwrap(), + Some(record) + ); +} diff --git a/lib/vector-buffers/src/variants/disk_v2/common.rs b/lib/vector-buffers/src/variants/disk_v2/common.rs index 177f0f5d58f4a..fee4cb2a0f9a0 100644 --- a/lib/vector-buffers/src/variants/disk_v2/common.rs +++ b/lib/vector-buffers/src/variants/disk_v2/common.rs @@ -214,7 +214,7 @@ impl DiskBufferConfigBuilder { max_record_size: None, write_buffer_size: None, flush_interval: None, - filesystem: ProductionFilesystem, + filesystem: ProductionFilesystem::default(), } } } diff --git a/lib/vector-buffers/src/variants/disk_v2/io.rs b/lib/vector-buffers/src/variants/disk_v2/io.rs index c081765803975..a294437143626 100644 --- a/lib/vector-buffers/src/variants/disk_v2/io.rs +++ b/lib/vector-buffers/src/variants/disk_v2/io.rs @@ -1,14 +1,70 @@ use std::{ + fmt, future::Future, - io, + io::{self, Write as _}, path::{Path, PathBuf}, + pin::Pin, + sync::{Arc, Weak}, + task::{Context, Poll}, }; use tokio::{ fs::OpenOptions, - io::{AsyncRead, AsyncWrite}, + io::{AsyncRead, AsyncWrite, AsyncWriteExt, ReadBuf}, }; +#[cfg(test)] +use std::sync::Mutex; + +const RESUMABLE_WRITE_CHUNK_SIZE: usize = 256 * 1024; + +/// Returns whether an I/O error indicates exhausted filesystem capacity or quota. +pub(crate) fn is_filesystem_full(error: &io::Error) -> bool { + if matches!( + error.kind(), + io::ErrorKind::StorageFull | io::ErrorKind::QuotaExceeded + ) { + return true; + } + + #[cfg(unix)] + if matches!(error.raw_os_error(), Some(libc::ENOSPC | libc::EDQUOT)) { + return true; + } + + false +} + +#[cfg(test)] +mod tests { + use super::is_filesystem_full; + use std::io; + + #[test] + fn classifies_storage_full() { + assert!(is_filesystem_full(&io::Error::from( + io::ErrorKind::StorageFull + ))); + assert!(!is_filesystem_full(&io::Error::from( + io::ErrorKind::PermissionDenied + ))); + assert!(is_filesystem_full(&io::Error::from( + io::ErrorKind::QuotaExceeded + ))); + } + + #[cfg(unix)] + #[test] + fn classifies_unix_capacity_errors() { + assert!(is_filesystem_full(&io::Error::from_raw_os_error( + libc::ENOSPC + ))); + assert!(is_filesystem_full(&io::Error::from_raw_os_error( + libc::EDQUOT + ))); + } +} + #[cfg(unix)] const FILE_MODE_OWNER_RW_GROUP_RO: u32 = 0o640; @@ -119,9 +175,30 @@ pub trait Filesystem: Send + Sync { fn supports_background_cleanup(&self) -> bool { true } + + /// Binds files opened through this value to the current disk-buffer session. + /// + /// Test filesystems do not launch detached writes and therefore need no session binding. + fn bind_buffer_session(&mut self, _session_guard: Weak) {} } -pub trait AsyncFile: AsyncRead + AsyncWrite + Send + Sync { +pub trait AsyncFile: AsyncRead + AsyncWrite + Send + Sync + Unpin { + /// Whether a cancelled write may still commit bytes in the background. + fn has_pending_write(&self) -> bool { + false + } + + /// Writes at most one chunk that the caller can account for and resume after a short write. + /// + /// Implementations used for production data files must not report bytes as written until the + /// underlying filesystem write has completed. A blocking kernel write cannot be cancelled + /// portably, so production implementations keep a cancelled syscall owned by the buffer + /// session until it returns. The default is suitable for test files and other `AsyncWrite` + /// implementations that already provide that guarantee. + async fn write_resumable(&mut self, buf: &[u8]) -> io::Result { + self.write(buf).await + } + /// Queries metadata about the underlying file. /// /// # Errors @@ -163,30 +240,198 @@ pub trait WritableMemoryMap: ReadableMemoryMap { /// A normal filesystem used for production operations. /// /// Uses Tokio's `File` for asynchronous file reading/writing, and `memmap2` for memory-mapped files. -#[derive(Clone, Debug)] -pub struct ProductionFilesystem; +#[derive(Clone, Default)] +pub struct ProductionFilesystem { + session_guard: Option>, + #[cfg(test)] + write_gate: Option>, +} + +/// Production file handle with session-owned blocking data writes. +pub struct ProductionFile { + inner: tokio::fs::File, + blocking_writer: Option>, + pending_write: Option, + session_guard: Option>, + #[cfg(test)] + write_gate: Option>, +} + +struct PendingWrite { + task: tokio::task::JoinHandle>, +} + +impl ProductionFile { + fn new( + inner: tokio::fs::File, + session_guard: Option>, + #[cfg(test)] write_gate: Option>, + ) -> Self { + Self { + inner, + blocking_writer: None, + pending_write: None, + session_guard, + #[cfg(test)] + write_gate, + } + } + + async fn blocking_writer(&mut self) -> io::Result> { + if let Some(file) = &self.blocking_writer { + return Ok(Arc::clone(file)); + } + + let file = Arc::new(self.inner.try_clone().await?.into_std().await); + self.blocking_writer = Some(Arc::clone(&file)); + Ok(file) + } + + #[cfg(test)] + pub(crate) fn has_blocking_writer(&self) -> bool { + self.blocking_writer.is_some() + } +} + +impl fmt::Debug for ProductionFilesystem { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ProductionFilesystem") + .finish_non_exhaustive() + } +} + +impl ProductionFilesystem { + fn production_file(&self, file: tokio::fs::File) -> ProductionFile { + ProductionFile::new( + file, + self.session_guard.clone(), + #[cfg(test)] + self.write_gate.clone(), + ) + } +} + +#[cfg(test)] +pub(crate) struct TestWriteGate { + released: Mutex, + release: std::sync::Condvar, + started: std::sync::atomic::AtomicBool, + started_notify: tokio::sync::Notify, +} + +#[cfg(test)] +pub(crate) struct StalledWrites { + pub(crate) filesystem: ProductionFilesystem, + pub(crate) gate: Arc, +} + +#[cfg(test)] +impl TestWriteGate { + fn new() -> Self { + Self { + released: Mutex::new(false), + release: std::sync::Condvar::new(), + started: std::sync::atomic::AtomicBool::new(false), + started_notify: tokio::sync::Notify::new(), + } + } + + fn wait(&self) { + self.started + .store(true, std::sync::atomic::Ordering::Release); + self.started_notify.notify_waiters(); + let mut released = self.released.lock().unwrap(); + while !*released { + released = self.release.wait(released).unwrap(); + } + } + + pub(crate) async fn wait_until_started(&self) { + while !self.started.load(std::sync::atomic::Ordering::Acquire) { + self.started_notify.notified().await; + } + } + + pub(crate) fn release(&self) { + *self.released.lock().unwrap() = true; + self.release.notify_all(); + } +} + +#[cfg(test)] +impl ProductionFilesystem { + pub(crate) fn with_stalled_writes() -> StalledWrites { + let gate = Arc::new(TestWriteGate::new()); + StalledWrites { + filesystem: Self { + write_gate: Some(Arc::clone(&gate)), + ..Self::default() + }, + gate, + } + } +} + +impl fmt::Debug for ProductionFile { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ProductionFile") + .field("inner", &self.inner) + .finish_non_exhaustive() + } +} + +impl AsyncRead for ProductionFile { + fn poll_read( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + Pin::new(&mut self.inner).poll_read(cx, buf) + } +} + +impl AsyncWrite for ProductionFile { + fn poll_write( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + Pin::new(&mut self.inner).poll_write(cx, buf) + } + + fn poll_flush(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.inner).poll_flush(cx) + } + + fn poll_shutdown(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.inner).poll_shutdown(cx) + } +} impl Filesystem for ProductionFilesystem { - type File = tokio::fs::File; + type File = ProductionFile; type MemoryMap = memmap2::Mmap; type MutableMemoryMap = memmap2::MmapMut; async fn open_file_writable(&self, path: &Path) -> io::Result { - create_writable_file_options(false) + let file = create_writable_file_options(false) .append(true) .open(path) - .await + .await?; + Ok(self.production_file(file)) } async fn open_file_writable_atomic(&self, path: &Path) -> io::Result { - create_writable_file_options(true) + let file = create_writable_file_options(true) .append(true) .open(path) - .await + .await?; + Ok(self.production_file(file)) } async fn open_file_readable(&self, path: &Path) -> io::Result { - open_readable_file_options().open(path).await + let file = open_readable_file_options().open(path).await?; + Ok(self.production_file(file)) } async fn open_mmap_readable(&self, path: &Path) -> io::Result { @@ -235,6 +480,10 @@ impl Filesystem for ProductionFilesystem { Ok(files) } } + + fn bind_buffer_session(&mut self, session_guard: Weak) { + self.session_guard = Some(session_guard); + } } /// Builds a set of `OpenOptions` for opening a file as readable/writable. @@ -284,6 +533,65 @@ fn open_readable_file_options() -> OpenOptions { open_options } +impl AsyncFile for ProductionFile { + fn has_pending_write(&self) -> bool { + self.pending_write.is_some() + } + + async fn write_resumable(&mut self, buf: &[u8]) -> io::Result { + let write_len = buf.len().min(RESUMABLE_WRITE_CHUNK_SIZE); + + if self.pending_write.is_none() { + let file = self.blocking_writer().await?; + let session_guard = self.session_guard.as_ref().and_then(Weak::upgrade); + let buf = buf[..write_len].to_vec(); + #[cfg(test)] + let write_gate = self.write_gate.clone(); + let task = tokio::task::spawn_blocking(move || { + // Keep the buffer session locked until a detached syscall has actually returned. + let _session_guard = session_guard; + #[cfg(test)] + if let Some(gate) = write_gate { + gate.wait(); + } + let mut file = &*file; + #[allow(clippy::disallowed_methods)] + file.write(&buf) + }); + self.pending_write = Some(PendingWrite { task }); + } + + let result = (&mut self.pending_write.as_mut().unwrap().task) + .await + .map_err(io::Error::other)?; + self.pending_write = None; + result + } + + async fn metadata(&self) -> io::Result { + let metadata = self.inner.metadata().await?; + Ok(Metadata { + len: metadata.len(), + }) + } + + async fn truncate(&self, size: u64) -> io::Result<()> { + let current_size = self.inner.metadata().await?.len(); + if size > current_size { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "cannot extend a file through the truncation API", + )); + } + + self.inner.set_len(size).await + } + + async fn sync_all(&self) -> io::Result<()> { + self.inner.sync_all().await + } +} + impl AsyncFile for tokio::fs::File { async fn metadata(&self) -> io::Result { let metadata = self.metadata().await?; diff --git a/lib/vector-buffers/src/variants/disk_v2/ledger.rs b/lib/vector-buffers/src/variants/disk_v2/ledger.rs index 8c8eba76234b3..3110923f59429 100644 --- a/lib/vector-buffers/src/variants/disk_v2/ledger.rs +++ b/lib/vector-buffers/src/variants/disk_v2/ledger.rs @@ -236,13 +236,13 @@ where config: DiskBufferConfig, // Advisory lock for this buffer directory. #[allow(dead_code)] - lock: LockFile, + lock: Arc, // Ledger state. state: BackedArchive, // The total size, in bytes, of all unread records in the buffer. total_buffer_size: AtomicU64, // Notifier for reader-related progress. - reader_notify: Notify, + reader_notify: Arc, // Notifier for writer-related progress. writer_notify: Notify, // Tracks when writer has fully shutdown. @@ -500,6 +500,10 @@ where self.reader_notify.notified().await; } + pub fn reader_notifier(&self) -> Arc { + Arc::clone(&self.reader_notify) + } + /// Waits for a signal from the writer that progress has been made. /// /// This will occur when a record is written, or when a new data file is created. @@ -534,18 +538,8 @@ where next_record_id } - /// Tracks events that arrived at the buffer but were rejected before being - /// persisted (e.g. `Bufferable::filter_unencodable` dropping over-budget - /// sub-items). Bumps both `received` and the unintentional-`dropped` counter - /// on the usage handle so `buffer_size = received - sent - dropped` stays - /// consistent and operators can see the rejection in buffer-usage metrics. - /// `total_buffer_size` is intentionally left alone — these events never - /// reached disk. - pub fn track_dropped(&self, event_count: u64, byte_size: u64) { - self.usage_handle - .increment_received_event_count_and_byte_size(event_count, byte_size); - self.usage_handle - .increment_dropped_event_count_and_byte_size(event_count, byte_size, false); + pub fn usage_handle(&self) -> BufferUsageHandle { + self.usage_handle.clone() } /// Tracks the statistics of multiple successful reads. @@ -731,7 +725,7 @@ where /// operations, an error variant will be returned describing the error. #[cfg_attr(test, instrument(skip_all, level = "trace"))] pub(super) async fn load_or_create( - config: DiskBufferConfig, + mut config: DiskBufferConfig, usage_handle: BufferUsageHandle, ) -> Result, LedgerLoadCreateError> { // Create our containing directory if it doesn't already exist. @@ -751,6 +745,12 @@ where if !lock.try_lock().context(IoSnafu)? { return Err(LedgerLoadCreateError::LedgerLockAlreadyHeld); } + let lock = Arc::new(lock); + // Production blocking writes take a lease on this lock before leaving the async runtime. + let session_guard: Arc = lock.clone(); + config + .filesystem + .bind_buffer_session(Arc::downgrade(&session_guard)); // Open the ledger file, which may involve creating it if it doesn't yet exist. let ledger_path = config.data_dir.join("buffer.db"); @@ -816,7 +816,7 @@ where lock, state: ledger_state, total_buffer_size: AtomicU64::new(0), - reader_notify: Notify::new(), + reader_notify: Arc::new(Notify::new()), writer_notify: Notify::new(), writer_done: AtomicBool::new(false), pending_acks: AtomicU64::new(0), diff --git a/lib/vector-buffers/src/variants/disk_v2/mod.rs b/lib/vector-buffers/src/variants/disk_v2/mod.rs index 970a416ba9e5a..548b3dc049dbe 100644 --- a/lib/vector-buffers/src/variants/disk_v2/mod.rs +++ b/lib/vector-buffers/src/variants/disk_v2/mod.rs @@ -193,9 +193,12 @@ mod ser; mod writer; #[cfg(test)] -mod tests; +pub(crate) mod tests; +#[cfg(test)] +pub(crate) use self::io::{StalledWrites, TestWriteGate}; use self::ledger::Ledger; +pub(crate) use self::writer::CapacityProgress; pub use self::{ common::{DiskBufferConfig, DiskBufferConfigBuilder}, io::{Filesystem, ProductionFilesystem}, diff --git a/lib/vector-buffers/src/variants/disk_v2/tests/mod.rs b/lib/vector-buffers/src/variants/disk_v2/tests/mod.rs index 6f7f20bf4f1d4..ba253ce1dcfe2 100644 --- a/lib/vector-buffers/src/variants/disk_v2/tests/mod.rs +++ b/lib/vector-buffers/src/variants/disk_v2/tests/mod.rs @@ -1,7 +1,11 @@ use std::{ io::{self, Cursor}, path::Path, - sync::Arc, + sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + }, + time::{Duration, Instant}, }; use tokio::{ @@ -22,14 +26,63 @@ use crate::{ type FilesystemUnderTest = ProductionFilesystem; +struct WriteGateCleanup(Arc); + +impl Drop for WriteGateCleanup { + fn drop(&mut self) { + self.0.release(); + } +} + +async fn open_production_ledger( + data_dir: &Path, + filesystem: ProductionFilesystem, +) -> Result, super::ledger::LedgerLoadCreateError> { + let config = DiskBufferConfigBuilder::from_path(data_dir) + .filesystem(filesystem) + .build() + .unwrap(); + Ledger::load_or_create(config, BufferUsageHandle::noop()).await +} + +async fn reopen_production_ledger( + data_dir: &Path, +) -> Result, super::ledger::LedgerLoadCreateError> { + open_production_ledger(data_dir, ProductionFilesystem::default()).await +} + +async fn assert_session_locked(data_dir: &Path) { + assert!(matches!( + reopen_production_ledger(data_dir).await, + Err(super::ledger::LedgerLoadCreateError::LedgerLockAlreadyHeld) + )); +} + +async fn wait_for_session_unlock(data_dir: &Path) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + match reopen_production_ledger(data_dir).await { + Ok(_) => break, + Err(super::ledger::LedgerLoadCreateError::LedgerLockAlreadyHeld) => { + tokio::task::yield_now().await; + } + Err(error) => panic!("buffer reopen failed: {error}"), + } + } + }) + .await + .expect("session lock should be released after the detached write completes"); +} + mod acknowledgements; mod basic; mod filter_metrics; mod initialization; mod invariants; mod known_errors; -mod model; +pub(crate) mod model; mod record; +mod runtime_capacity; mod size_limits; impl AsyncFile for DuplexStream { @@ -493,7 +546,7 @@ async fn production_filesystem_truncates_with_append_handle_open() { let path = dir.join("truncate-with-append-handle"); async move { - let filesystem = ProductionFilesystem; + let filesystem = ProductionFilesystem::default(); filesystem .truncate_file(&path, 0) .await @@ -531,3 +584,153 @@ async fn production_filesystem_truncates_with_append_handle_open() { }) .await; } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn production_write_completes() { + with_temp_dir(|dir| { + let path = dir.join("single-poll-write"); + + async move { + let filesystem = ProductionFilesystem::default(); + let mut file = filesystem + .open_file_writable(&path) + .await + .expect("file should open"); + + assert_eq!( + file.write_resumable(b"test") + .await + .expect("write should succeed"), + 4 + ); + assert_eq!( + file.metadata().await.expect("metadata should load").len(), + 4 + ); + } + }) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn cancelled_stalled_write_does_not_block_runtime_and_retains_session_lock() { + with_temp_dir(|dir| { + let data_dir = dir.to_path_buf(); + + async move { + let super::io::StalledWrites { filesystem, gate } = + ProductionFilesystem::with_stalled_writes(); + let ledger = open_production_ledger(&data_dir, filesystem.clone()) + .await + .unwrap(); + let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); + let path = data_dir.join("buffer-data-final"); + let mut file = ledger.filesystem().open_file_writable(&path).await.unwrap(); + let mut write = Box::pin(file.write_resumable(b"test")); + + tokio::select! { + result = &mut write => panic!("gated write unexpectedly completed: {result:?}"), + () = gate.wait_until_started() => {} + } + + let runtime_progressed = Arc::new(AtomicBool::new(false)); + let watchdog_progress = Arc::clone(&runtime_progressed); + let watchdog_gate = Arc::clone(&gate); + let watchdog = std::thread::spawn(move || { + let deadline = Instant::now() + Duration::from_secs(5); + while !watchdog_progress.load(Ordering::Acquire) && Instant::now() < deadline { + std::thread::park_timeout(deadline.saturating_duration_since(Instant::now())); + } + let runtime_blocked = !watchdog_progress.load(Ordering::Acquire); + if runtime_blocked { + watchdog_gate.release(); + } + runtime_blocked + }); + + drop(write); + drop(file); + drop(ledger); + tokio::task::yield_now().await; + runtime_progressed.store(true, Ordering::Release); + watchdog.thread().unpark(); + + assert_session_locked(&data_dir).await; + + gate.release(); + assert!( + !watchdog.join().unwrap(), + "the watchdog had to release a write that parked the current-thread runtime" + ); + wait_for_session_unlock(&data_dir).await; + } + }) + .await; +} + +#[tokio::test] +async fn readable_production_file_does_not_create_blocking_writer() { + with_temp_dir(|dir| { + let path = dir.join("readable-file"); + + async move { + tokio::fs::write(&path, b"test").await.unwrap(); + let filesystem = ProductionFilesystem::default(); + let file = filesystem.open_file_readable(&path).await.unwrap(); + + assert!(!file.has_blocking_writer()); + } + }) + .await; +} + +#[tokio::test] +async fn cloned_production_filesystems_keep_session_locks_independent() { + with_temp_dir(|dir| { + let first_dir = dir.join("first"); + let second_dir = dir.join("second"); + + async move { + let super::io::StalledWrites { filesystem, gate } = + ProductionFilesystem::with_stalled_writes(); + let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); + let first = open_production_ledger(&first_dir, filesystem.clone()) + .await + .unwrap(); + let second = open_production_ledger(&second_dir, filesystem) + .await + .unwrap(); + + let mut first_file = first + .filesystem() + .open_file_writable(&first_dir.join("buffer-data-final")) + .await + .unwrap(); + let mut write = Box::pin(first_file.write_resumable(b"test")); + tokio::select! { + result = &mut write => panic!("gated write unexpectedly completed: {result:?}"), + () = gate.wait_until_started() => {} + } + drop(write); + drop(first); + + assert_session_locked(&first_dir).await; + + gate.release(); + assert_eq!(first_file.write_resumable(b"test").await.unwrap(), 4); + drop(first_file); + + reopen_production_ledger(&first_dir) + .await + .expect("first session lock should be released independently"); + + assert_session_locked(&second_dir).await; + drop(second); + + reopen_production_ledger(&second_dir) + .await + .expect("second session lock should be released independently"); + } + }) + .await; +} diff --git a/lib/vector-buffers/src/variants/disk_v2/tests/model/common.rs b/lib/vector-buffers/src/variants/disk_v2/tests/model/common.rs index abbe785b9caef..424e15d488c92 100644 --- a/lib/vector-buffers/src/variants/disk_v2/tests/model/common.rs +++ b/lib/vector-buffers/src/variants/disk_v2/tests/model/common.rs @@ -14,8 +14,7 @@ pub type WriterResult = Result>; // This is specifically set at 60KB because we allow a maximum record size of up to 64KB, and so // we'd likely to occasionally encounter a record that, when encoded, is larger than the write -// buffer overall, which exercises the "write this record directly to the wrapped writer" logic that -// exists in `tokio::io::BufWriter` itself. +// buffer overall, which exercises the resumable direct-write path. pub const TEST_WRITE_BUFFER_SIZE: usize = 60 * 1024; /// Result of applying an action to the model. diff --git a/lib/vector-buffers/src/variants/disk_v2/tests/model/filesystem.rs b/lib/vector-buffers/src/variants/disk_v2/tests/model/filesystem.rs index fbbd70d3aaded..7857341451897 100644 --- a/lib/vector-buffers/src/variants/disk_v2/tests/model/filesystem.rs +++ b/lib/vector-buffers/src/variants/disk_v2/tests/model/filesystem.rs @@ -33,6 +33,55 @@ struct FileInner { buf: Option>, } +#[derive(Debug, Default)] +struct FaultState { + data_write_error: Option, + data_write_attempts: usize, + bytes_until_error: Option, + max_write_size: Option, + data_open: OpenFault, + data_fallback_open: OpenFault, + data_sync_error: Option, + data_syncs_until_error: usize, +} + +#[derive(Debug, Default)] +struct OpenFault { + error: Option, + attempts: usize, +} + +impl OpenFault { + fn fail(&mut self, kind: io::ErrorKind) { + self.error = Some(kind); + self.attempts = 0; + } + + fn attempt(&mut self) -> Option { + self.attempts += 1; + self.error + } +} + +#[derive(Clone, Copy, Debug)] +enum FaultError { + #[cfg(not(unix))] + Kind(io::ErrorKind), + #[cfg(unix)] + RawOs(i32), +} + +impl FaultError { + fn into_error(self) -> io::Error { + match self { + #[cfg(not(unix))] + Self::Kind(kind) => kind.into(), + #[cfg(unix)] + Self::RawOs(raw) => io::Error::from_raw_os_error(raw), + } + } +} + impl FileInner { fn consume_buf(&mut self) -> Vec { self.buf.take().expect("tried to consume buf, but empty") @@ -68,14 +117,18 @@ impl fmt::Debug for FileInner { #[derive(Clone)] pub struct TestFile { inner: Arc>, + faults: Arc>, + is_data_file: bool, is_writable: bool, read_pos: usize, } impl TestFile { - fn new() -> Self { + fn new(path: &Path, faults: Arc>) -> Self { Self { inner: Arc::new(Mutex::new(FileInner::default())), + faults, + is_data_file: path.extension().is_some_and(|extension| extension == "dat"), is_writable: false, read_pos: 0, } @@ -102,7 +155,7 @@ impl fmt::Debug for TestFile { .field("data", &inner) .field("writable", &self.is_writable) .field("read_pos", &self.read_pos) - .finish() + .finish_non_exhaustive() } } @@ -183,11 +236,34 @@ impl AsyncWrite for TestFile { return Err(io_err_permission_denied()).into(); } + let write_len = if self.is_data_file { + let mut faults = self.faults.lock().expect("poisoned"); + faults.data_write_attempts += 1; + if faults.bytes_until_error == Some(0) { + return Err(io::Error::from( + faults + .data_write_error + .unwrap_or(io::ErrorKind::StorageFull), + )) + .into(); + } + + let until_error = faults.bytes_until_error.unwrap_or(usize::MAX); + let max_write_size = faults.max_write_size.unwrap_or(usize::MAX); + let write_len = buf.len().min(until_error).min(max_write_size); + if let Some(bytes_until_error) = faults.bytes_until_error.as_mut() { + *bytes_until_error -= write_len; + } + write_len + } else { + buf.len() + }; + let mut inner = self.inner.lock().expect("poisoned"); let dst = inner.buf.as_mut().expect("file buf consumed"); - dst.extend_from_slice(buf); + dst.extend_from_slice(&buf[..write_len]); - Poll::Ready(Ok(buf.len())) + Poll::Ready(Ok(write_len)) } fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { @@ -239,6 +315,15 @@ impl AsyncFile for TestFile { } async fn sync_all(&self) -> io::Result<()> { + if self.is_data_file { + let mut faults = self.faults.lock().expect("poisoned"); + if let Some(error) = faults.data_sync_error { + if faults.data_syncs_until_error == 0 { + return Err(error.into_error()); + } + faults.data_syncs_until_error -= 1; + } + } Ok(()) } } @@ -247,6 +332,7 @@ impl AsyncFile for TestFile { #[derive(Debug, Default)] struct FilesystemInner { files: HashMap, + faults: Arc>, } impl FilesystemInner { @@ -255,7 +341,7 @@ impl FilesystemInner { let file = self .files .entry(path.to_owned()) - .or_insert_with(TestFile::new); + .or_insert_with(|| TestFile::new(path, Arc::clone(&self.faults))); let mut new_file = file.clone(); new_file.set_writable(); @@ -267,7 +353,7 @@ impl FilesystemInner { if self.files.contains_key(path) { None } else { - let mut new_file = TestFile::new(); + let mut new_file = TestFile::new(path, Arc::clone(&self.faults)); new_file.set_writable(); self.files.insert(path.to_owned(), new_file.clone()); @@ -320,7 +406,9 @@ impl fmt::Debug for TestFilesystem { impl Clone for TestFilesystem { fn clone(&self) -> Self { - Self::default() + Self { + inner: Arc::clone(&self.inner), + } } } @@ -332,17 +420,135 @@ impl Default for TestFilesystem { } } +impl TestFilesystem { + fn with_faults(&self, f: impl FnOnce(&mut FaultState) -> R) -> R { + let inner = self.inner.lock().expect("poisoned"); + let mut faults = inner.faults.lock().expect("poisoned"); + f(&mut faults) + } + + pub(crate) fn fail_data_writes_after(&self, bytes: usize, kind: io::ErrorKind) { + self.with_faults(|faults| { + faults.data_write_error = Some(kind); + faults.bytes_until_error = Some(bytes); + }); + } + + pub(crate) fn restore_data_writes(&self) { + self.with_faults(|faults| { + faults.data_write_error = None; + faults.bytes_until_error = None; + }); + } + + pub(crate) fn data_write_attempts(&self) -> usize { + self.with_faults(|faults| faults.data_write_attempts) + } + + pub(crate) fn set_max_write_size(&self, size: Option) { + self.with_faults(|faults| faults.max_write_size = size); + } + + pub(crate) fn fail_data_file_open(&self, kind: io::ErrorKind) { + self.with_faults(|faults| { + faults.data_open.fail(kind); + }); + } + + pub(crate) fn restore_data_file_open(&self) { + self.with_faults(|faults| faults.data_open.error = None); + } + + pub(crate) fn data_file_open_attempts(&self) -> usize { + self.with_faults(|faults| faults.data_open.attempts) + } + + pub(crate) fn create_data_file(&self, path: &Path) { + let mut inner = self.inner.lock().expect("poisoned"); + inner.open_file_writable(path); + } + + pub(crate) fn create_data_file_with_data(&self, path: &Path, data: &[u8]) { + let mut inner = self.inner.lock().expect("poisoned"); + let file = inner.open_file_writable(path); + file.inner + .lock() + .expect("poisoned") + .buf + .as_mut() + .expect("file buffer must exist") + .extend_from_slice(data); + } + + pub(crate) fn fail_data_file_fallback_open(&self, kind: io::ErrorKind) { + self.with_faults(|faults| faults.data_fallback_open.fail(kind)); + } + + pub(crate) fn restore_data_file_fallback_open(&self) { + self.with_faults(|faults| faults.data_fallback_open.error = None); + } + + pub(crate) fn data_file_fallback_open_attempts(&self) -> usize { + self.with_faults(|faults| faults.data_fallback_open.attempts) + } + + #[cfg(not(unix))] + pub(crate) fn fail_data_file_sync_after(&self, successful_syncs: usize, kind: io::ErrorKind) { + self.with_faults(|faults| { + faults.data_sync_error = Some(FaultError::Kind(kind)); + faults.data_syncs_until_error = successful_syncs; + }); + } + + #[cfg(unix)] + pub(crate) fn fail_data_file_sync_after_raw_os_error( + &self, + successful_syncs: usize, + raw_os_error: i32, + ) { + self.with_faults(|faults| { + faults.data_sync_error = Some(FaultError::RawOs(raw_os_error)); + faults.data_syncs_until_error = successful_syncs; + }); + } + + pub(crate) fn restore_data_file_sync(&self) { + self.with_faults(|faults| faults.data_sync_error = None); + } + + pub(crate) fn data(&self, path: &Path) -> Vec { + let inner = self.inner.lock().expect("poisoned"); + let file = inner.files.get(path).expect("file should exist"); + file.inner + .lock() + .expect("poisoned") + .buf + .clone() + .expect("file buffer should be available") + } +} + impl Filesystem for TestFilesystem { type File = TestFile; type MemoryMap = TestMmap; type MutableMemoryMap = TestMmap; async fn open_file_writable(&self, path: &Path) -> io::Result { + if path.extension().is_some_and(|extension| extension == "dat") + && let Some(kind) = self.with_faults(|faults| faults.data_fallback_open.attempt()) + { + return Err(io::Error::from(kind)); + } let mut inner = self.inner.lock().expect("poisoned"); Ok(inner.open_file_writable(path)) } async fn open_file_writable_atomic(&self, path: &Path) -> io::Result { + if path.extension().is_some_and(|extension| extension == "dat") + && let Some(kind) = self.with_faults(|faults| faults.data_open.attempt()) + { + return Err(io::Error::from(kind)); + } let mut inner = self.inner.lock().expect("poisoned"); match inner.open_file_writable_atomic(path) { Some(file) => Ok(file), diff --git a/lib/vector-buffers/src/variants/disk_v2/tests/model/mod.rs b/lib/vector-buffers/src/variants/disk_v2/tests/model/mod.rs index 5e0b18acaee3c..50c4beeff6a24 100644 --- a/lib/vector-buffers/src/variants/disk_v2/tests/model/mod.rs +++ b/lib/vector-buffers/src/variants/disk_v2/tests/model/mod.rs @@ -30,10 +30,10 @@ use self::action::{Action, arb_actions}; mod common; use self::common::{Progress, arb_buffer_config}; -mod filesystem; +pub(crate) mod filesystem; use self::filesystem::TestFilesystem; -mod record; +pub(crate) mod record; use self::record::Record; mod sequencer; @@ -140,7 +140,7 @@ impl FileModel { } if record_len >= self.write_buffer_size { - // The record is bigger the write buffer itself, so just write it immediately: + // Records at least as large as the write buffer bypass it. flushed_events += record.event_count(); flushed_bytes += record_len; diff --git a/lib/vector-buffers/src/variants/disk_v2/tests/runtime_capacity.rs b/lib/vector-buffers/src/variants/disk_v2/tests/runtime_capacity.rs new file mode 100644 index 0000000000000..586dae10a8826 --- /dev/null +++ b/lib/vector-buffers/src/variants/disk_v2/tests/runtime_capacity.rs @@ -0,0 +1,725 @@ +use std::{io, time::Duration}; + +use tokio::time::timeout; +use vector_common::finalization::{AddBatchNotifier, BatchNotifier, BatchStatus}; + +use super::{ + Buffer, DiskBufferConfigBuilder, + model::{filesystem::TestFilesystem, record::Record}, +}; +use crate::buffer_usage_data::BufferUsageHandle; +use crate::variants::disk_v2::{CapacityProgress, TryWriteOutcome, WriterError}; + +struct CapacityBuffer { + writer: super::BufferWriter, + reader: super::BufferReader, + ledger: std::sync::Arc>, + usage: BufferUsageHandle, +} + +async fn build_buffer_with_rotation( + filesystem: TestFilesystem, + write_buffer_size: usize, + rotating: bool, +) -> CapacityBuffer { + let directory = + std::env::temp_dir().join(format!("vector-disk-v2-capacity-{}", rand::random::())); + let mut builder = DiskBufferConfigBuilder::from_path(directory) + .write_buffer_size(write_buffer_size) + .filesystem(filesystem); + if rotating { + builder = builder.max_data_file_size(256).max_record_size(256); + } + let config = builder.build().expect("configuration should be valid"); + let usage = BufferUsageHandle::noop(); + let (writer, reader, ledger) = Buffer::from_config_inner(config, usage.clone()) + .await + .expect("buffer should initialize"); + CapacityBuffer { + writer, + reader, + ledger, + usage, + } +} + +async fn build_buffer(filesystem: TestFilesystem, write_buffer_size: usize) -> CapacityBuffer { + build_buffer_with_rotation(filesystem, write_buffer_size, false).await +} + +async fn build_rotating_buffer(filesystem: TestFilesystem) -> CapacityBuffer { + build_buffer_with_rotation(filesystem, 1024, true).await +} + +async fn assert_flush_pending(writer: &mut super::BufferWriter) { + assert!( + timeout(Duration::from_millis(20), writer.flush()) + .await + .is_err(), + "flush should remain pending while persistence is capacity-blocked" + ); +} + +async fn assert_written(writer: &mut super::BufferWriter, record: Record) { + assert_eq!( + writer.try_write_record(record).await.unwrap(), + TryWriteOutcome::Written + ); +} + +async fn cancel_capacity_write( + writer: &mut super::BufferWriter, + filesystem: &TestFilesystem, + record: Record, + bytes_until_error: usize, +) { + filesystem.fail_data_writes_after(bytes_until_error, io::ErrorKind::StorageFull); + assert_eq!( + writer.try_write_record(record).await.unwrap(), + TryWriteOutcome::Pending, + "a partially written record must remain owned by the writer" + ); + filesystem.restore_data_writes(); +} + +async fn fill_for_rotation( + writer: &mut super::BufferWriter, + first_id: u32, +) { + for id in first_id..first_id + 2 { + assert_written(writer, Record::new(id, 64, 1)).await; + writer.flush().await.unwrap(); + } +} + +#[derive(Clone, Copy, Debug)] +enum OpenPath { + Atomic, + Fallback, +} + +impl OpenPath { + fn prepare(self, filesystem: &TestFilesystem, ledger: &super::Ledger) { + if matches!(self, Self::Fallback) { + filesystem.create_data_file(&ledger.get_next_writer_data_file_path()); + } + } + + fn fail(self, filesystem: &TestFilesystem, kind: io::ErrorKind) { + match self { + Self::Atomic => filesystem.fail_data_file_open(kind), + Self::Fallback => filesystem.fail_data_file_fallback_open(kind), + } + } + + fn restore(self, filesystem: &TestFilesystem) { + match self { + Self::Atomic => filesystem.restore_data_file_open(), + Self::Fallback => filesystem.restore_data_file_fallback_open(), + } + } + + fn attempts(self, filesystem: &TestFilesystem) -> usize { + match self { + Self::Atomic => filesystem.data_file_open_attempts(), + Self::Fallback => filesystem.data_file_fallback_open_attempts(), + } + } +} + +#[tokio::test] +async fn partial_write_then_storage_full_resumes_without_duplication() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, + mut reader, + ledger, + usage, + } = build_buffer(filesystem.clone(), 1024).await; + let record = Record::new(1, 128, 1); + + filesystem.fail_data_writes_after(7, io::ErrorKind::StorageFull); + assert_written(&mut writer, record.clone()).await; + assert_flush_pending(&mut writer).await; + assert_eq!(ledger.get_total_records(), 0); + assert_eq!(ledger.get_total_buffer_size(), 0); + assert_eq!(usage.snapshot().received_event_count, 0); + + let path = ledger.get_current_writer_data_file_path(); + assert_eq!(filesystem.data(&path).len(), 7); + + filesystem.restore_data_writes(); + writer.flush().await.unwrap(); + let written_len = filesystem.data(&path).len(); + assert_eq!(ledger.get_total_records(), 1); + assert_eq!(ledger.get_total_buffer_size(), written_len as u64); + assert_eq!(usage.snapshot().received_event_count, 1); + assert_eq!(reader.next().await.unwrap(), Some(record)); +} + +#[tokio::test] +async fn partial_write_leaves_subsequent_record_available_for_drop_newest() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let CapacityBuffer { + mut writer, + mut reader, + .. + } = build_buffer(filesystem.clone(), 16).await; + let owned = Record::new(60, 256, 1); + let newest = Record::new(61, 64, 1); + + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + assert_eq!( + writer.try_write_record(owned.clone()).await.unwrap(), + TryWriteOutcome::Pending + ); + assert_eq!( + timeout( + Duration::from_millis(20), + writer.try_write_record(newest.clone()) + ) + .await + .expect("a subsequent nonblocking send must return promptly") + .unwrap(), + TryWriteOutcome::Full(newest) + ); + + filesystem.restore_data_writes(); + writer.flush().await.unwrap(); + assert_eq!(reader.next().await.unwrap(), Some(owned)); +} + +#[tokio::test] +async fn partial_write_leaves_subsequent_record_available_for_overflow() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let CapacityBuffer { + mut writer, + mut reader, + .. + } = build_buffer(filesystem.clone(), 16).await; + let CapacityBuffer { + writer: mut overflow_writer, + reader: mut overflow_reader, + .. + } = build_buffer(TestFilesystem::default(), 1024).await; + let owned = Record::new(62, 256, 1); + let overflowed = Record::new(63, 64, 1); + + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + assert_eq!( + writer.try_write_record(owned.clone()).await.unwrap(), + TryWriteOutcome::Pending + ); + let TryWriteOutcome::Full(returned) = + writer.try_write_record(overflowed.clone()).await.unwrap() + else { + panic!("the subsequent record must remain available for overflow"); + }; + assert_written(&mut overflow_writer, returned).await; + + filesystem.restore_data_writes(); + writer.flush().await.unwrap(); + overflow_writer.flush().await.unwrap(); + assert_eq!(reader.next().await.unwrap(), Some(owned)); + assert_eq!(overflow_reader.next().await.unwrap(), Some(overflowed)); +} + +#[tokio::test] +async fn partial_write_retains_finalizer_until_owned_record_completes() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let CapacityBuffer { mut writer, .. } = build_buffer(filesystem.clone(), 16).await; + let mut record = Record::new(64, 256, 1); + let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); + record.add_batch_notifier(batch); + + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + assert_eq!( + writer.try_write_record(record).await.unwrap(), + TryWriteOutcome::Pending + ); + assert!( + timeout(Duration::from_millis(20), &mut finalizer) + .await + .is_err(), + "an owned record must retain its finalizer while capacity is exhausted" + ); + + filesystem.restore_data_writes(); + writer.flush().await.unwrap(); + assert_eq!(finalizer.await, BatchStatus::Delivered); +} + +#[tokio::test] +async fn unowned_capacity_full_record_keeps_finalizer_for_policy() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { mut writer, .. } = build_rotating_buffer(filesystem.clone()).await; + fill_for_rotation(&mut writer, 70).await; + filesystem.fail_data_file_open(io::ErrorKind::StorageFull); + let mut record = Record::new(72, 64, 1); + let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); + record.add_batch_notifier(batch); + + let TryWriteOutcome::Full(returned) = writer.try_write_record(record).await.unwrap() else { + panic!("open ENOSPC must return the unowned record to policy"); + }; + assert!( + timeout(Duration::from_millis(20), &mut finalizer) + .await + .is_err(), + "returning an unowned record must not resolve its finalizer" + ); + drop(returned); + assert_eq!(finalizer.await, BatchStatus::Delivered); +} + +#[tokio::test] +async fn large_record_uses_resumable_flush_path() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let CapacityBuffer { + mut writer, + mut reader, + ledger, + .. + } = build_buffer(filesystem.clone(), 16).await; + let record = Record::new(2, 256, 1); + + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + assert_eq!( + writer.try_write_record(record.clone()).await.unwrap(), + TryWriteOutcome::Pending + ); + filesystem.restore_data_writes(); + writer.flush().await.unwrap(); + + assert_eq!( + filesystem + .data(&ledger.get_current_writer_data_file_path()) + .len() as u64, + ledger.get_total_buffer_size() + ); + assert_eq!(reader.next().await.unwrap(), Some(record)); +} + +#[tokio::test] +async fn cancelled_large_write_finishes_before_next_record() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let CapacityBuffer { + mut writer, + mut reader, + .. + } = build_buffer(filesystem.clone(), 16).await; + let interrupted = Record::new(8, 256, 1); + let subsequent = Record::new(9, 256, 1); + + cancel_capacity_write(&mut writer, &filesystem, interrupted.clone(), 10).await; + assert_written(&mut writer, subsequent.clone()).await; + writer.flush().await.unwrap(); + + assert_eq!(reader.next().await.unwrap(), Some(interrupted)); + assert_eq!(reader.next().await.unwrap(), Some(subsequent)); +} + +#[tokio::test] +async fn flush_finishes_cancelled_large_write() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, + mut reader, + .. + } = build_buffer(filesystem.clone(), 16).await; + let interrupted = Record::new(13, 256, 1); + + cancel_capacity_write(&mut writer, &filesystem, interrupted.clone(), 10).await; + writer.flush().await.unwrap(); + + assert_eq!(reader.next().await.unwrap(), Some(interrupted)); +} + +#[tokio::test] +async fn cancelled_large_write_preserves_implicit_flush_accounting() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, + mut reader, + ledger, + .. + } = build_buffer(filesystem.clone(), 128).await; + let buffered = Record::new(10, 16, 1); + let interrupted = Record::new(11, 256, 1); + let subsequent = Record::new(12, 16, 1); + + assert_written(&mut writer, buffered.clone()).await; + cancel_capacity_write(&mut writer, &filesystem, interrupted.clone(), 200).await; + + assert_eq!(ledger.get_total_records(), 1); + assert_eq!(reader.next().await.unwrap(), Some(buffered)); + + assert_written(&mut writer, subsequent.clone()).await; + writer.flush().await.unwrap(); + + assert_eq!(ledger.get_total_records(), 3); + assert_eq!(reader.next().await.unwrap(), Some(interrupted)); + assert_eq!(reader.next().await.unwrap(), Some(subsequent)); +} + +#[tokio::test(start_paused = true)] +async fn runtime_block_waits_and_recovers_from_timer() { + vector_common::event_test_util::clear_recorded_events(); + let filesystem = TestFilesystem::default(); + let CapacityBuffer { mut writer, .. } = build_buffer(filesystem.clone(), 1024).await; + filesystem.fail_data_writes_after(0, io::ErrorKind::StorageFull); + assert_written(&mut writer, Record::new(3, 64, 1)).await; + + let mut flush = Box::pin(writer.flush()); + tokio::select! { + result = &mut flush => panic!("flush unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + assert_eq!(filesystem.data_write_attempts(), 1); + vector_common::event_test_util::contains_name_once("DiskBufferBackpressure").unwrap(); + assert!( + vector_common::event_test_util::contains_name_once("DiskBufferBackpressureRecovered") + .is_err() + ); + + tokio::time::advance(Duration::from_millis(100)).await; + tokio::select! { + result = &mut flush => panic!("flush unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + assert_eq!(filesystem.data_write_attempts(), 2); + filesystem.restore_data_writes(); + tokio::time::advance(Duration::from_millis(200)).await; + timeout(Duration::from_secs(1), flush) + .await + .expect("flush should recover without reader notification") + .expect("flush should succeed"); + vector_common::event_test_util::contains_name_once("DiskBufferBackpressureRecovered").unwrap(); +} + +#[tokio::test(start_paused = true)] +async fn cancelled_flush_backpressures_small_subsequent_write() { + vector_common::event_test_util::clear_recorded_events(); + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, + mut reader, + ledger, + .. + } = build_buffer(filesystem.clone(), 1024).await; + let accepted = Record::new(15, 32, 1); + let subsequent = Record::new(16, 32, 1); + assert_written(&mut writer, accepted.clone()).await; + + filesystem.fail_data_writes_after(0, io::ErrorKind::StorageFull); + let mut flush = Box::pin(writer.flush()); + tokio::select! { + result = &mut flush => panic!("flush unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + drop(flush); + assert_eq!(filesystem.data_write_attempts(), 1); + + assert_eq!( + writer.try_write_record(subsequent.clone()).await.unwrap(), + TryWriteOutcome::Full(subsequent.clone()) + ); + assert_eq!(filesystem.data_write_attempts(), 2); + vector_common::event_test_util::contains_name_once("DiskBufferBackpressure").unwrap(); + assert!( + vector_common::event_test_util::contains_name_once("DiskBufferBackpressureRecovered") + .is_err() + ); + + filesystem.restore_data_writes(); + ledger.notify_reader_waiters(); + assert_written(&mut writer, subsequent.clone()).await; + vector_common::event_test_util::contains_name_once("DiskBufferBackpressureRecovered").unwrap(); + + writer.flush().await.unwrap(); + assert_eq!(reader.next().await.unwrap(), Some(accepted)); + assert_eq!(reader.next().await.unwrap(), Some(subsequent)); +} + +#[tokio::test(start_paused = true)] +async fn runtime_block_reacts_to_reader_notification() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, ledger, .. + } = build_buffer(filesystem.clone(), 1024).await; + filesystem.fail_data_writes_after(0, io::ErrorKind::StorageFull); + assert_written(&mut writer, Record::new(4, 64, 1)).await; + let mut flush = Box::pin(writer.flush()); + tokio::select! { + result = &mut flush => panic!("flush unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + filesystem.restore_data_writes(); + ledger.notify_reader_waiters(); + timeout(Duration::from_secs(1), flush) + .await + .expect("reader progress should wake the writer") + .expect("flush should succeed"); +} + +#[tokio::test(start_paused = true)] +async fn runtime_rotation_capacity_faults_wait_without_spinning_and_recover() { + for path in [OpenPath::Atomic, OpenPath::Fallback] { + vector_common::event_test_util::clear_recorded_events(); + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, ledger, .. + } = build_rotating_buffer(filesystem.clone()).await; + fill_for_rotation(&mut writer, 20).await; + path.prepare(&filesystem, &ledger); + path.fail(&filesystem, io::ErrorKind::StorageFull); + + let current = Record::new(22, 64, 1); + let outcome = writer.try_write_record(current.clone()).await.unwrap(); + assert_eq!(outcome, TryWriteOutcome::Full(current.clone())); + assert_eq!(path.attempts(&filesystem), 1); + assert_eq!( + writer.retry_capacity().await.unwrap(), + CapacityProgress::Blocked + ); + assert_eq!(path.attempts(&filesystem), 2); + + path.restore(&filesystem); + assert_eq!( + writer.retry_capacity().await.unwrap(), + CapacityProgress::Ready + ); + assert_written(&mut writer, current).await; + assert!(path.attempts(&filesystem) <= 3); + vector_common::event_test_util::contains_name_once("DiskBufferBackpressure").unwrap(); + vector_common::event_test_util::contains_name_once("DiskBufferBackpressureRecovered") + .unwrap(); + } +} + +#[tokio::test(start_paused = true)] +async fn successful_flush_does_not_close_cancelled_open_capacity_episode() { + vector_common::event_test_util::clear_recorded_events(); + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, ledger, .. + } = build_rotating_buffer(filesystem.clone()).await; + fill_for_rotation(&mut writer, 40).await; + let path = OpenPath::Atomic; + path.prepare(&filesystem, &ledger); + path.fail(&filesystem, io::ErrorKind::StorageFull); + + assert!(matches!( + writer + .try_write_record(Record::new(42, 64, 1)) + .await + .unwrap(), + TryWriteOutcome::Full(_) + )); + writer.flush().await.unwrap(); + assert!( + vector_common::event_test_util::contains_name_once("DiskBufferBackpressureRecovered") + .is_err(), + "a successful data flush must not close an open-file capacity episode" + ); + + path.restore(&filesystem); + assert_written(&mut writer, Record::new(43, 64, 1)).await; + vector_common::event_test_util::contains_name_once("DiskBufferBackpressureRecovered").unwrap(); +} + +#[tokio::test] +async fn runtime_rotation_non_capacity_open_faults_remain_fatal() { + for path in [OpenPath::Atomic, OpenPath::Fallback] { + vector_common::event_test_util::clear_recorded_events(); + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, ledger, .. + } = build_rotating_buffer(filesystem.clone()).await; + fill_for_rotation(&mut writer, 50).await; + path.prepare(&filesystem, &ledger); + path.fail(&filesystem, io::ErrorKind::PermissionDenied); + + let error = writer + .try_write_record(Record::new(52, 64, 1)) + .await + .expect_err("permission failure must remain fatal"); + let WriterError::Io { source } = error else { + panic!("expected I/O error, got {error:?}"); + }; + assert_eq!(source.kind(), io::ErrorKind::PermissionDenied); + assert_eq!(path.attempts(&filesystem), 1); + assert!( + vector_common::event_test_util::contains_name_once("DiskBufferBackpressure").is_err() + ); + } +} + +#[tokio::test] +async fn runtime_rotation_sync_capacity_retries_without_losing_current_record() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, ledger, .. + } = build_rotating_buffer(filesystem.clone()).await; + fill_for_rotation(&mut writer, 30).await; + + let writer_file_id = ledger.get_current_writer_file_id(); + // Let the full current file sync successfully, then fail syncing the newly created file. + #[cfg(unix)] + filesystem.fail_data_file_sync_after_raw_os_error(1, libc::ENOSPC); + #[cfg(not(unix))] + filesystem.fail_data_file_sync_after(1, io::ErrorKind::StorageFull); + let current = Record::new(32, 64, 1); + let outcome = writer.try_write_record(current.clone()).await.unwrap(); + assert_eq!(outcome, TryWriteOutcome::Full(current.clone())); + assert_eq!(ledger.get_current_writer_file_id(), writer_file_id); + filesystem.restore_data_file_sync(); + assert_eq!( + writer.retry_capacity().await.unwrap(), + CapacityProgress::Ready + ); + assert_written(&mut writer, current).await; +} + +#[tokio::test] +async fn rotation_flush_capacity_error_retains_data_write_operation() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { mut writer, .. } = build_rotating_buffer(filesystem.clone()).await; + fill_for_rotation(&mut writer, 80).await; + assert_written(&mut writer, Record::new(82, 64, 1)).await; + assert_written(&mut writer, Record::new(83, 64, 1)).await; + + filesystem.fail_data_writes_after(0, io::ErrorKind::StorageFull); + let record = Record::new(84, 64, 1); + assert_eq!( + writer.try_write_record(record.clone()).await.unwrap(), + TryWriteOutcome::Full(record.clone()) + ); + + filesystem.restore_data_writes(); + assert_eq!( + writer.retry_capacity().await.unwrap(), + CapacityProgress::Ready + ); + assert_written(&mut writer, record).await; +} + +#[tokio::test(start_paused = true)] +async fn write_record_capacity_retry_preserves_exponential_backoff() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { mut writer, .. } = build_buffer(filesystem.clone(), 16).await; + filesystem.fail_data_writes_after(0, io::ErrorKind::StorageFull); + + let mut write = Box::pin(writer.write_record(Record::new(85, 64, 1))); + tokio::select! { + biased; + result = &mut write => panic!("capacity-blocked write unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + assert_eq!(filesystem.data_write_attempts(), 1); + + tokio::time::advance(Duration::from_millis(100)).await; + tokio::select! { + biased; + result = &mut write => panic!("capacity-blocked write unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + assert_eq!(filesystem.data_write_attempts(), 2); + + tokio::time::advance(Duration::from_millis(100)).await; + tokio::select! { + biased; + result = &mut write => panic!("capacity-blocked write unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + assert_eq!(filesystem.data_write_attempts(), 2); + + filesystem.restore_data_writes(); + tokio::time::advance(Duration::from_millis(100)).await; + write.await.unwrap(); +} + +#[tokio::test] +async fn try_write_waits_while_previous_accepted_data_retries() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, + mut reader, + .. + } = build_buffer(filesystem.clone(), 128).await; + let accepted = Record::new(5, 64, 1); + let subsequent = Record::new(6, 64, 1); + filesystem.fail_data_writes_after(3, io::ErrorKind::StorageFull); + assert_written(&mut writer, accepted.clone()).await; + assert_eq!( + writer.try_write_record(subsequent.clone()).await.unwrap(), + TryWriteOutcome::Full(subsequent.clone()) + ); + filesystem.restore_data_writes(); + assert_written(&mut writer, subsequent.clone()).await; + writer.flush().await.unwrap(); + assert_eq!(reader.next().await.unwrap(), Some(accepted)); + assert_eq!(reader.next().await.unwrap(), Some(subsequent)); +} + +#[tokio::test] +async fn implicit_flush_capacity_error_keeps_current_write_pending() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { + mut writer, + mut reader, + .. + } = build_buffer(filesystem.clone(), 128).await; + let accepted = Record::new(12, 32, 1); + let current = Record::new(13, 32, 1); + assert_written(&mut writer, accepted.clone()).await; + + filesystem.fail_data_writes_after(4, io::ErrorKind::StorageFull); + assert_eq!( + writer.try_write_record(current.clone()).await.unwrap(), + TryWriteOutcome::Full(current.clone()), + "the current record is still unowned when only the previous buffer flush failed" + ); + + filesystem.restore_data_writes(); + assert_written(&mut writer, current.clone()).await; + writer.flush().await.unwrap(); + assert_eq!(reader.next().await.unwrap(), Some(accepted)); + assert_eq!(reader.next().await.unwrap(), Some(current)); +} + +#[tokio::test] +async fn non_capacity_runtime_io_error_remains_fatal() { + let filesystem = TestFilesystem::default(); + let CapacityBuffer { mut writer, .. } = build_buffer(filesystem.clone(), 1024).await; + assert_written(&mut writer, Record::new(7, 64, 1)).await; + filesystem.fail_data_writes_after(0, io::ErrorKind::PermissionDenied); + + let error = writer + .flush() + .await + .expect_err("permission failure should be fatal"); + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied); +} + +#[tokio::test] +async fn startup_storage_full_is_immediately_fatal() { + let filesystem = TestFilesystem::default(); + filesystem.fail_data_file_open(io::ErrorKind::StorageFull); + let directory = std::env::temp_dir().join(format!( + "vector-disk-v2-capacity-startup-{}", + rand::random::() + )); + let config = DiskBufferConfigBuilder::from_path(directory) + .filesystem(filesystem) + .build() + .unwrap(); + + let result = Buffer::::from_config_inner(config, BufferUsageHandle::noop()).await; + assert!(result.is_err()); +} diff --git a/lib/vector-buffers/src/variants/disk_v2/writer.rs b/lib/vector-buffers/src/variants/disk_v2/writer.rs index e4536ad70644f..78bd1e27bb40e 100644 --- a/lib/vector-buffers/src/variants/disk_v2/writer.rs +++ b/lib/vector-buffers/src/variants/disk_v2/writer.rs @@ -2,11 +2,13 @@ use std::{ cmp::Ordering, convert::Infallible as StdInfallible, fmt, + future::Future, io::{self, ErrorKind}, marker::PhantomData, num::NonZeroUsize, path::Path, sync::Arc, + time::Instant, }; use bytes::BufMut; @@ -22,25 +24,33 @@ use rkyv::{ }, }; use snafu::{ResultExt, Snafu}; -use tokio::io::{AsyncWrite, AsyncWriteExt}; +use tokio::time::{Duration, sleep}; use super::{ common::{DiskBufferConfig, create_crc32c_hasher}, - io::Filesystem, + io::{Filesystem, is_filesystem_full}, ledger::Ledger, record::{Record, RecordStatus, validate_record_archive}, }; use crate::{ Bufferable, + buffer_usage_data::BufferUsageHandle, encoding::{AsMetadata, Encodable}, + internal_events::{DiskBufferBackpressure, DiskBufferBackpressureRecovered}, variants::disk_v2::{ io::AsyncFile, reader::{ReadToken, ReaderError, RecordReader, decode_record_payload}, record::{RECORD_HEADER_LEN, try_as_record_archive}, }, }; -use vector_common::finalization::{EventFinalizerGroups, EventStatus}; +use vector_common::{ + finalization::{EventFinalizerGroups, EventStatus}, + internal_event::emit, +}; + +const FILESYSTEM_FULL_RETRY_INITIAL: Duration = Duration::from_millis(100); +const FILESYSTEM_FULL_RETRY_MAX: Duration = Duration::from_secs(5); /// Error that occurred during calls to [`BufferWriter`]. #[derive(Debug, Snafu)] @@ -245,6 +255,9 @@ pub enum TryWriteOutcome { Written, /// The buffer is currently full; the record is returned for retry or discard. Full(T), + /// The record is owned by the writer because some of its bytes may already have been written. + /// The caller must not retry or route another copy of it. + Pending, /// The record permanently exceeded the maximum record size and was dropped. /// /// Its finalizers have already been resolved as [`EventStatus::Dropped`] (equivalent to @@ -258,17 +271,12 @@ pub enum TryWriteOutcome { /// /// Used in `try_write_record_inner` so that every `?` exit automatically notifies acking sources /// to nack / withhold checkpoints for any record that did not reach durable storage. -struct FinalizerGuard { - finalizers: EventFinalizerGroups, - error_on_drop: bool, -} +#[derive(Debug)] +struct FinalizerGuard(Option); impl FinalizerGuard { fn new(finalizers: EventFinalizerGroups) -> Self { - Self { - finalizers, - error_on_drop: true, - } + Self(Some(finalizers)) } /// Releases the guard without marking finalizers as errored. @@ -276,30 +284,96 @@ impl FinalizerGuard { /// Call when the record was intentionally dropped (unwritable) or successfully flushed to /// disk — both cases where the upstream source should ack rather than retry. fn disarm(mut self) { - self.error_on_drop = false; + self.0.take(); } /// Returns the finalizers for reattachment to the recovered record on buffer-full retry. fn into_inner(mut self) -> EventFinalizerGroups { - self.error_on_drop = false; - std::mem::take(&mut self.finalizers) + self.0.take().expect("finalizers must exist") } } impl Drop for FinalizerGuard { fn drop(&mut self) { - if self.error_on_drop { - self.finalizers.update_status(EventStatus::Errored); + if let Some(finalizers) = self.0.as_mut() { + finalizers.update_status(EventStatus::Errored); } } } -#[derive(Debug)] +#[derive(Clone, Copy, Debug)] pub(super) struct WriteToken { event_count: usize, serialized_len: usize, } +#[derive(Debug)] +struct PendingRecordWrite { + token: WriteToken, + finalizers: FinalizerGuard, +} + +enum PendingWriteOutcome { + None, + Complete(usize), + CapacityBlocked, +} + +enum WriteAttempt { + Written(usize), + Full(T), + Pending, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum CapacityProgress { + Ready, + Blocked, + WaitingForReader, +} + +#[derive(Debug)] +struct CapacityBackpressure { + operation: CapacityOperation, + retries: u64, + started: Instant, +} + +struct CapacityRetry { + delay: Duration, + reader_wakeup_used: bool, +} + +impl Default for CapacityRetry { + fn default() -> Self { + Self { + delay: FILESYSTEM_FULL_RETRY_INITIAL, + reader_wakeup_used: false, + } + } +} + +enum EnsureReady { + Ready, + RetryCapacity(CapacityOperation, io::Error), + WaitingForReader, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum CapacityOperation { + DataWrite, + OpenDataFile, +} + +impl CapacityOperation { + const fn as_str(self) -> &'static str { + match self { + Self::DataWrite => "write data", + Self::OpenDataFile => "open data file", + } + } +} + impl WriteToken { pub fn event_count(&self) -> usize { self.event_count @@ -329,26 +403,54 @@ pub(super) struct FlushResult { struct TrackingBufWriter { inner: W, buf: Vec, + capacity: usize, + flush_offset: usize, + direct_write_offset: usize, + completed_flush: Option, unflushed_events: usize, } -impl TrackingBufWriter { +impl TrackingBufWriter { /// Creates a new `TrackingBufWriter` with the specified buffer capacity. fn with_capacity(cap: usize, inner: W) -> Self { Self { inner, buf: Vec::with_capacity(cap), + capacity: cap, + flush_offset: 0, + direct_write_offset: 0, + completed_flush: None, unflushed_events: 0, } } + fn take_completed_flush(&mut self) -> Option { + self.completed_flush.take() + } + + fn current_record_may_be_committed(&self) -> bool { + self.direct_write_offset > 0 || self.inner.has_pending_write() + } + + async fn write_all_resumable(inner: &mut W, buf: &[u8], offset: &mut usize) -> io::Result<()> { + while *offset < buf.len() { + match inner.write_resumable(&buf[*offset..]).await { + Ok(0) => return Err(io::ErrorKind::WriteZero.into()), + Ok(written) => *offset += written, + Err(error) if error.kind() == io::ErrorKind::Interrupted => {} + Err(error) => return Err(error), + } + } + Ok(()) + } + /// Writes the given buffer. /// /// If enough internal buffer capacity is available, then this write will be buffered internally /// until [`flush`] is called. If there's not enough remaining internal buffer capacity, then /// the internal buffer will be flushed to the inner writer first. If the given buffer is - /// larger than the internal buffer capacity, then it will be written directly to the inner - /// writer. + /// larger than the internal buffer capacity, it is written directly from the caller's retained + /// serialization buffer. /// /// Internally, a counter is kept of how many buffered events are waiting to be flushed. This /// count is incremented every time `write` can fully buffer the record without having to flush @@ -364,27 +466,29 @@ impl TrackingBufWriter { /// If a write to the inner writer occurs, and that write encounters an error, an error variant /// will be returned describing the error. async fn write(&mut self, event_count: usize, buf: &[u8]) -> io::Result> { - let mut flush_result = None; - // If this write would cause us to exceed our internal buffer capacity, flush whatever we // have buffered already. - if self.buf.len() + buf.len() > self.buf.capacity() { - flush_result = self.flush().await?; + if self.buf.len() + buf.len() > self.capacity + && let Some(flush_result) = self.flush().await? + { + let completed = self.completed_flush.get_or_insert(FlushResult::default()); + completed.events_flushed += flush_result.events_flushed; + completed.bytes_flushed += flush_result.bytes_flushed; } - // If the given buffer is too large to be buffered at all, then bypass the internal buffer. - if buf.len() >= self.buf.capacity() { - self.inner.write_all(buf).await?; + if buf.len() >= self.capacity { + Self::write_all_resumable(&mut self.inner, buf, &mut self.direct_write_offset).await?; - let flush_result = flush_result.get_or_insert(FlushResult::default()); - flush_result.events_flushed += event_count as u64; - flush_result.bytes_flushed += buf.len() as u64; + self.direct_write_offset = 0; + let completed = self.completed_flush.get_or_insert(FlushResult::default()); + completed.events_flushed += event_count as u64; + completed.bytes_flushed += buf.len() as u64; } else { self.buf.extend_from_slice(buf); self.unflushed_events += event_count; } - Ok(flush_result) + Ok(self.completed_flush.take()) } /// Flushes the internal buffer to the underlying writer. @@ -408,16 +512,16 @@ impl TrackingBufWriter { let events_flushed = self.unflushed_events as u64; let bytes_flushed = self.buf.len() as u64; - let result = self.inner.write_all(&self.buf[..]).await; + Self::write_all_resumable(&mut self.inner, &self.buf, &mut self.flush_offset).await?; + + self.flush_offset = 0; self.unflushed_events = 0; self.buf.clear(); - result.map(|()| { - Some(FlushResult { - events_flushed, - bytes_flushed, - }) - }) + Ok(Some(FlushResult { + events_flushed, + bytes_flushed, + })) } /// Gets a reference to the underlying writer. @@ -438,9 +542,12 @@ impl fmt::Debug for TrackingBufWriter { .field("writer", &self.inner) .field( "buffer", - &format_args!("{}/{}", self.buf.len(), self.buf.capacity()), + &format_args!("{}/{}", self.buf.len(), self.capacity), ) .field("unflushed_events", &self.unflushed_events) + .field("flush_offset", &self.flush_offset) + .field("direct_write_offset", &self.direct_write_offset) + .field("completed_flush", &self.completed_flush) .finish() } } @@ -516,6 +623,10 @@ where self.writer.get_ref() } + fn current_record_may_be_committed(&self) -> bool { + self.writer.current_record_may_be_committed() + } + /// Whether or not `amount` bytes could be written while obeying the data file size limit. /// /// If no bytes have written at all to a data file, then `amount` is allowed to exceed the @@ -680,7 +791,7 @@ where record: T, ) -> Result<(usize, Option), WriterError> { let token = self.archive_record(id, record)?; - self.flush_record(token).await + self.flush_record(&token).await } /// Flushes the previously-archived record. @@ -699,7 +810,7 @@ where #[instrument(skip(self), level = "trace")] pub async fn flush_record( &mut self, - token: WriteToken, + token: &WriteToken, ) -> Result<(usize, Option), WriterError> { // Make sure the write token we've been given matches whatever the last call to `archive_record` generated. let event_count = token.event_count(); @@ -742,6 +853,10 @@ where Ok((serialized_len, flush_result)) } + fn take_completed_flush(&mut self) -> Option { + self.writer.take_completed_flush() + } + /// Recovers an archived record that has not yet been flushed. /// /// In some cases, we must archive a record to see how large the resulting archived record is, and potentially @@ -870,6 +985,7 @@ enum WriterCheckpointScanAction { /// Writes records to the buffer. #[derive(Debug)] +#[allow(clippy::struct_excessive_bools)] pub struct BufferWriter where FS: Filesystem, @@ -885,6 +1001,9 @@ where data_file_full: bool, skip_to_next: bool, ready_to_write: bool, + pending_record_write: Option, + capacity_backpressure: Option, + capacity_sync_pending: bool, _t: PhantomData, } @@ -907,6 +1026,9 @@ where unflushed_bytes: 0, skip_to_next: false, ready_to_write: false, + pending_record_write: None, + capacity_backpressure: None, + capacity_sync_pending: false, next_record_id, unflushed_events: 0, _t: PhantomData, @@ -944,6 +1066,123 @@ where !self.data_file_full && self.data_file_size < self.config.max_data_file_size } + async fn wait_for_capacity(&self, retry: &mut CapacityRetry) { + if retry.reader_wakeup_used { + sleep(retry.delay).await; + retry.reader_wakeup_used = false; + } else { + tokio::select! { + () = self.ledger.wait_for_reader() => retry.reader_wakeup_used = true, + () = sleep(retry.delay) => {} + } + } + retry.delay = retry.delay.saturating_mul(2).min(FILESYSTEM_FULL_RETRY_MAX); + } + + fn record_capacity_error( + &mut self, + operation: CapacityOperation, + error: &io::Error, + retry_delay: Duration, + ) { + if let Some(backpressure) = self.capacity_backpressure.as_mut() { + debug_assert_eq!(backpressure.operation, operation); + backpressure.retries += 1; + return; + } + + emit(DiskBufferBackpressure { + operation: operation.as_str(), + error, + retry_delay, + buffer_path: &self.config.data_dir, + }); + self.capacity_backpressure = Some(CapacityBackpressure { + operation, + retries: 1, + started: Instant::now(), + }); + } + + fn record_capacity_recovered(&mut self, operation: CapacityOperation) { + let Some(backpressure) = self + .capacity_backpressure + .take_if(|backpressure| backpressure.operation == operation) + else { + return; + }; + emit(DiskBufferBackpressureRecovered { + operation: backpressure.operation.as_str(), + retries: backpressure.retries, + duration: backpressure.started.elapsed(), + buffer_path: &self.config.data_dir, + }); + } + + async fn finish_pending_record_write(&mut self) -> Result> { + let Some(token) = self + .pending_record_write + .as_ref() + .map(|pending| pending.token) + else { + return Ok(PendingWriteOutcome::None); + }; + + let (result, completed_flush) = { + let writer = self + .writer + .as_mut() + .expect("pending record write must have an open writer"); + let result = writer.flush_record(&token).await; + (result, writer.take_completed_flush()) + }; + if let Some(flush_result) = completed_flush { + self.publish_flushed_progress(flush_result.events_flushed, flush_result.bytes_flushed); + } + + let (bytes_written, flush_result) = match result { + Ok(result) => result, + Err(WriterError::Io { source }) + if self.ready_to_write && is_filesystem_full(&source) => + { + self.record_capacity_error( + CapacityOperation::DataWrite, + &source, + FILESYSTEM_FULL_RETRY_INITIAL, + ); + return Ok(PendingWriteOutcome::CapacityBlocked); + } + Err(error) => return Err(error), + }; + self.record_capacity_recovered(CapacityOperation::DataWrite); + + self.track_write(token.event_count(), bytes_written as u64); + if let Some(flush_result) = flush_result { + self.publish_flushed_progress(flush_result.events_flushed, flush_result.bytes_flushed); + } + + self.pending_record_write + .take() + .expect("pending record write must exist") + .finalizers + .disarm(); + Ok(PendingWriteOutcome::Complete(bytes_written)) + } + + fn recover_pending_record(&mut self) -> Result> { + let pending = self + .pending_record_write + .take() + .expect("pending record write must exist"); + let writer = self + .writer + .as_mut() + .expect("pending record write must have an open writer"); + let mut record = writer.recover_archived_record(&pending.token)?; + record.merge_finalizer_groups(pending.finalizers.into_inner()); + Ok(record) + } + fn can_write_record(&self, amount: usize) -> bool { let total_buffer_size = self.ledger.get_total_buffer_size() + self.unflushed_bytes; let potential_write_len = @@ -1050,7 +1289,7 @@ where let previous_writer_file_id = self.ledger.get_current_writer_file_id(); self.reset(); self.ledger.state().increment_writer_file_id(); - self.ensure_ready_for_write().await.context(IoSnafu)?; + self.ensure_ready_for_write_fatal().await.context(IoSnafu)?; self.reconcile_current_data_file_with_checkpoint().await?; self.ledger.flush().context(IoSnafu)?; @@ -1279,7 +1518,7 @@ where current_writer_data_file = ?self.ledger.get_current_writer_data_file_path(), "Validating last written record in current data file." ); - self.ensure_ready_for_write().await.context(IoSnafu)?; + self.ensure_ready_for_write_fatal().await.context(IoSnafu)?; // If our current file is empty, there's no sense doing this check. if self.data_file_size == 0 { @@ -1443,7 +1682,7 @@ where self.reset(); self.mark_for_skip(); - self.ensure_ready_for_write().await.context(IoSnafu)?; + self.ensure_ready_for_write_fatal().await.context(IoSnafu)?; self.ledger.flush().context(IoSnafu)?; debug!( @@ -1461,14 +1700,13 @@ where total_buffer_size >= max_buffer_size } - /// Records sub-items that arrived at the buffer but were dropped before - /// reaching disk (e.g. `Bufferable::filter_unencodable` rejecting events - /// the protobuf decoder cannot handle). Delegates to the ledger's usage - /// handle so the rejection shows up under the disk-v2 stage's - /// `received` / `dropped` metrics in production, where the - /// `BufferSender` does not carry its own usage instrumentation. - pub(crate) fn track_dropped(&self, event_count: u64, byte_size: u64) { - self.ledger.track_dropped(event_count, byte_size); + pub(crate) fn usage_handle(&self) -> BufferUsageHandle { + self.ledger.usage_handle() + } + + #[cfg(test)] + pub(crate) fn next_writer_data_file_path(&self) -> std::path::PathBuf { + self.ledger.get_next_writer_data_file_path() } /// Ensures this writer is ready to attempt writer the next record. @@ -1476,7 +1714,7 @@ where // The inline antithesis assertion block pushes this over the line limit. Its // source lines count even when the feature is off, so the allow is unconditional. #[allow(clippy::too_many_lines)] - async fn ensure_ready_for_write(&mut self) -> io::Result<()> { + async fn ensure_ready_for_write(&mut self, wait_for_reader: bool) -> io::Result { // Check the overall size of the buffer and figure out if we can write. loop { // If we haven't yet exceeded the maximum buffer size, then we can proceed. Likewise, if @@ -1509,6 +1747,9 @@ where ); } + if !wait_for_reader { + return Ok(EnsureReady::WaitingForReader); + } self.ledger.wait_for_reader().await; } @@ -1520,7 +1761,7 @@ where let mut should_open_next = self.should_skip(); if self.writer.is_some() { if self.can_write() { - return Ok(()); + return Ok(EnsureReady::Ready); } // Our current data file is full, so we need to open a new one. Signal to the loop @@ -1531,7 +1772,12 @@ where // // We still flush ourselves to disk, etc, to make sure all of the data is there. should_open_next = true; - self.flush_inner(true).await?; + if matches!(self.try_flush_inner(true).await?, CapacityProgress::Blocked) { + return Ok(EnsureReady::RetryCapacity( + CapacityOperation::DataWrite, + io::ErrorKind::StorageFull.into(), + )); + } self.reset(); } @@ -1573,8 +1819,8 @@ where Ok(data_file) => Some((data_file, 0)), // We got back an error trying to open the file: might be that it already exists, // might be something else. - Err(e) => match e.kind() { - ErrorKind::AlreadyExists => { + Err(e) => { + if e.kind() == ErrorKind::AlreadyExists { // We open the file again, without the atomic "create new" behavior. If we // can do that successfully, we check its length. There's three main // situations we encounter: @@ -1584,11 +1830,18 @@ where // it, or waiting for acknowledgements to be able to delete it // - it may not be full, which could be because it's the data file the // writer left off on last time - let data_file = self + let data_file = match self .ledger .filesystem() .open_file_writable(&data_file_path) - .await?; + .await + { + Ok(data_file) => data_file, + Err(error) => { + drop(cleanup_guard); + return self.handle_open_error(error, should_open_next); + } + }; let metadata = data_file.metadata().await?; let file_len = metadata.len(); if file_len == 0 || !should_open_next { @@ -1604,10 +1857,14 @@ where // before we can proceed. None } + } else { + // Legitimate I/O error with the operation, bubble this up. Preserve a + // pending rotation so a retry targets the same next file instead of + // reopening the current full file first. + drop(cleanup_guard); + return self.handle_open_error(e, should_open_next); } - // Legitimate I/O error with the operation, bubble this up. - _ => return Err(e), - }, + } }; if let Some((data_file, data_file_size)) = file { @@ -1619,7 +1876,10 @@ where ); // Make sure the file is flushed to disk, especially if we just created it. - data_file.sync_all().await?; + if let Err(error) = data_file.sync_all().await { + drop(cleanup_guard); + return self.handle_open_error(error, should_open_next); + } self.writer = Some(RecordWriter::new( data_file, @@ -1656,7 +1916,7 @@ where ); } - return Ok(()); + return Ok(EnsureReady::Ready); } // The file is still present and waiting for a reader to finish reading it in order @@ -1666,10 +1926,42 @@ where // Wait until the reader signals progress and try again. debug!("Target data file is still present and not yet processed. Waiting for reader."); + if !wait_for_reader { + if should_open_next { + self.mark_for_skip(); + } + return Ok(EnsureReady::WaitingForReader); + } self.ledger.wait_for_reader().await; } } + async fn ensure_ready_for_write_fatal(&mut self) -> io::Result<()> { + match self.ensure_ready_for_write(true).await? { + EnsureReady::Ready => Ok(()), + EnsureReady::RetryCapacity(_, error) => Err(error), + EnsureReady::WaitingForReader => unreachable!("blocking readiness must wait"), + } + } + + fn handle_open_error( + &mut self, + error: io::Error, + preserve_rotation: bool, + ) -> io::Result { + if self.ready_to_write && is_filesystem_full(&error) { + if preserve_rotation { + self.mark_for_skip(); + } + Ok(EnsureReady::RetryCapacity( + CapacityOperation::OpenDataFile, + error, + )) + } else { + Err(error) + } + } + /// Attempts to write a record. /// /// Returns a [`TryWriteOutcome`] indicating whether the record was written, the buffer was @@ -1688,20 +1980,53 @@ where .await .map(|inner| match inner { // A zero-byte write is the sentinel for a silently dropped oversized record. - Ok(0) => TryWriteOutcome::Dropped, - Ok(_) => TryWriteOutcome::Written, - Err(record) => TryWriteOutcome::Full(record), + WriteAttempt::Written(0) => TryWriteOutcome::Dropped, + WriteAttempt::Written(_) => TryWriteOutcome::Written, + WriteAttempt::Full(record) => TryWriteOutcome::Full(record), + WriteAttempt::Pending => TryWriteOutcome::Pending, }) } #[instrument(skip_all, level = "debug")] + #[allow(clippy::too_many_lines)] async fn try_write_record_inner( &mut self, mut record: T, - ) -> Result, WriterError> { + ) -> Result, WriterError> { + // Own the finalizers before the first await so cancellation always nacks an unowned record. + // Once physical writing starts, this guard moves into `pending_record_write` instead. + let record_finalizers = FinalizerGuard::new(record.take_finalizer_groups()); + + // Cancellation can leave a direct write in progress. Finish it before archiving a new + // record so its saved byte offset and finalizers cannot be applied to different data. + match self.finish_pending_record_write().await? { + PendingWriteOutcome::None | PendingWriteOutcome::Complete(_) => {} + PendingWriteOutcome::CapacityBlocked => { + record.merge_finalizer_groups(record_finalizers.into_inner()); + return Ok(WriteAttempt::Full(record)); + } + } + + // A cancelled explicit flush can leave buffered bytes and an active physical-capacity + // episode without a pending record token. Resolve that underlying data write before a new + // record is allowed to fit in memory and appear accepted. + if self + .capacity_backpressure + .as_ref() + .is_some_and(|backpressure| backpressure.operation == CapacityOperation::DataWrite) + && matches!( + self.try_flush_inner(false).await?, + CapacityProgress::Blocked + ) + { + record.merge_finalizer_groups(record_finalizers.into_inner()); + return Ok(WriteAttempt::Full(record)); + } + // If the buffer is already full, we definitely can't complete this write. if self.is_buffer_full() { - return Ok(Err(record)); + record.merge_finalizer_groups(record_finalizers.into_inner()); + return Ok(WriteAttempt::Full(record)); } let record_events: NonZeroUsize = record @@ -1709,21 +2034,29 @@ where .try_into() .map_err(|_| WriterError::EmptyRecord)?; - // Extract the finalizers before `archive_record` consumes the record. The encoder - // unconditionally consumes the record (even on failure), so we must take what we need here - // to handle the case where encoding fails because the record is too large to ever write. - // The guard automatically resolves the finalizers as Errored if this function exits via - // `?`; call `disarm()` or `into_inner()` on the non-error paths. - let record_finalizers = FinalizerGuard::new(record.take_finalizer_groups()); - // Grab the next record ID and attempt to write the record. let record_id = self.get_next_record_id(); - let token = loop { // Make sure we have an open data file to write to, which might also be us opening the // next data file because our first attempt at writing had to finalize a data file that // was already full. - self.ensure_ready_for_write().await.context(IoSnafu)?; + match self.ensure_ready_for_write(false).await { + Ok(EnsureReady::Ready) => { + self.record_capacity_recovered(CapacityOperation::OpenDataFile); + } + Ok(EnsureReady::RetryCapacity(operation, source)) => { + self.record_capacity_error(operation, &source, FILESYSTEM_FULL_RETRY_INITIAL); + let mut record = record; + record.merge_finalizer_groups(record_finalizers.into_inner()); + return Ok(WriteAttempt::Full(record)); + } + Ok(EnsureReady::WaitingForReader) => { + let mut record = record; + record.merge_finalizer_groups(record_finalizers.into_inner()); + return Ok(WriteAttempt::Full(record)); + } + Err(source) => return Err(WriterError::Io { source }), + } let writer = self .writer @@ -1788,7 +2121,7 @@ where record_events.get() as u64, encoded_len as u64, ); - return Ok(Ok(0)); + return Ok(WriteAttempt::Written(0)); } e => return Err(e), }, @@ -1801,19 +2134,27 @@ where // // Otherwise, we proceed with flushing like we normally would. let can_write_record = self.can_write_record(token.serialized_len()); - let writer = self - .writer - .as_mut() - .expect("writer should exist after `ensure_ready_for_write`"); - - let (bytes_written, flush_result) = if can_write_record { - // We always return errors here because flushing the record won't return a recoverable - // error like `DataFileFull`, as that gets checked during archiving. The guard fires - // Errored automatically on `?` exit. - let result = writer.flush_record(token).await?; - // Record is durable on disk; disarm so finalizers resolve as Delivered. - record_finalizers.disarm(); - result + let bytes_written = if can_write_record { + self.pending_record_write = Some(PendingRecordWrite { + token, + finalizers: record_finalizers, + }); + match self.finish_pending_record_write().await? { + PendingWriteOutcome::Complete(bytes_written) => bytes_written, + PendingWriteOutcome::CapacityBlocked => { + let current_may_be_committed = self + .writer + .as_ref() + .is_some_and(RecordWriter::current_record_may_be_committed); + if current_may_be_committed { + return Ok(WriteAttempt::Pending); + } + return self.recover_pending_record().map(WriteAttempt::Full); + } + PendingWriteOutcome::None => { + unreachable!("record write was just marked pending") + } + } } else { // The record would not fit given the current size of the buffer, so we need to recover it from the // writer and hand it back. This looks a little weird because we want to surface deserialize/decoding @@ -1824,29 +2165,17 @@ where // record path, but this record is being returned for retry (block mode) or overflow. // `into_inner` extracts from the guard without resolving status; the finalizers will // be resolved only when the returned record is eventually written or dropped. + let writer = self + .writer + .as_mut() + .expect("writer should exist after `ensure_ready_for_write`"); let mut record = writer.recover_archived_record(&token)?; record.merge_finalizer_groups(record_finalizers.into_inner()); - return Ok(Err(record)); + return Ok(WriteAttempt::Full(record)); }; - // Track our write since things appear to have succeeded. This only updates our internal - // state as we have not yet authoritatively flushed the write to the data file. This tracks - // not only how many bytes we have buffered, but also how many events, which in turn drives - // record ID generation. We do this after the write appears to succeed to avoid issues with - // setting the ledger state to a record ID that we may never have actually written, which - // could lead to record ID gaps. - self.track_write(record_events.get(), bytes_written as u64); - - // If we did flush some buffered writes during this write, however, we now compensate for - // that after updating our internal state. Publishing the flushed state also notifies the - // reader, after all shared state reflects the readable bytes. - if let Some(flush_result) = flush_result { - self.publish_flushed_progress(flush_result.events_flushed, flush_result.bytes_flushed); - } - - // A record at or above the write-buffer size forces the buffered writer to - // flush mid-record, exercising the large-record path that splits a single - // record across multiple underlying writes. + // A record at or above the write-buffer size exercises the owned large-record path. The + // subsequent flush may split the record across multiple underlying writes. #[cfg(feature = "antithesis-disk-asserts")] { #![allow(clippy::disallowed_types)] // once_cell::Lazy @@ -1868,7 +2197,7 @@ where "Wrote record." ); - Ok(Ok(bytes_written)) + Ok(WriteAttempt::Written(bytes_written)) } /// Writes a record. @@ -1882,13 +2211,28 @@ where /// the error. #[instrument(skip_all, level = "debug")] pub async fn write_record(&mut self, mut record: T) -> Result> { + let mut retry = CapacityRetry::default(); loop { match self.try_write_record_inner(record).await? { - Ok(bytes_written) => return Ok(bytes_written), - Err(old_record) => { + WriteAttempt::Written(bytes_written) => return Ok(bytes_written), + WriteAttempt::Full(old_record) => { record = old_record; - self.ledger.wait_for_reader().await; + if self.capacity_backpressure.is_some() { + self.wait_for_capacity(&mut retry).await; + } else { + self.ledger.wait_for_reader().await; + } } + WriteAttempt::Pending => loop { + self.wait_for_capacity(&mut retry).await; + match self.finish_pending_record_write().await? { + PendingWriteOutcome::Complete(bytes_written) => return Ok(bytes_written), + PendingWriteOutcome::CapacityBlocked => {} + PendingWriteOutcome::None => { + unreachable!("pending write disappeared before completion") + } + } + }, } } } @@ -1911,7 +2255,7 @@ where } #[instrument(skip(self), level = "debug")] - async fn flush_inner(&mut self, force_full_flush: bool) -> io::Result<()> { + async fn try_flush_inner(&mut self, force_full_flush: bool) -> io::Result { // We always flush the `BufWriter` when this is called, but we don't always flush to disk or // flush the ledger. This is enough for readers on Linux since the file ends up in the page // cache, as we don't do any O_DIRECT fanciness, and the new contents can be immediately @@ -1919,26 +2263,131 @@ where // // TODO: Windows has a page cache as well, and macOS _should_, but we should verify this // behavior works on those platforms as well. - let flush_result = if let Some(writer) = self.writer.as_mut() { - writer.flush().await? - } else { - None + let result = match self.writer.as_mut() { + Some(writer) => writer.flush().await, + None => Ok(None), + }; + let flush_result = match result { + Ok(result) => result, + Err(error) if self.ready_to_write && is_filesystem_full(&error) => { + self.record_capacity_error( + CapacityOperation::DataWrite, + &error, + FILESYSTEM_FULL_RETRY_INITIAL, + ); + return Ok(CapacityProgress::Blocked); + } + Err(error) => return Err(error), }; - if let Some(flush_result) = flush_result { // Publish the readable bytes before waking the reader. self.publish_flushed_progress(flush_result.events_flushed, flush_result.bytes_flushed); } - if self.ledger.should_flush() || force_full_flush { - if let Some(writer) = self.writer.as_mut() { - writer.sync_all().await?; + if self.capacity_sync_pending || self.ledger.should_flush() || force_full_flush { + if let Some(writer) = self.writer.as_mut() + && let Err(error) = writer.sync_all().await + { + if self.ready_to_write && is_filesystem_full(&error) { + self.capacity_sync_pending = true; + self.record_capacity_error( + CapacityOperation::DataWrite, + &error, + FILESYSTEM_FULL_RETRY_INITIAL, + ); + return Ok(CapacityProgress::Blocked); + } + return Err(error); } - self.ledger.flush() + if let Err(error) = self.ledger.flush() { + if self.ready_to_write && is_filesystem_full(&error) { + self.capacity_sync_pending = true; + self.record_capacity_error( + CapacityOperation::DataWrite, + &error, + FILESYSTEM_FULL_RETRY_INITIAL, + ); + return Ok(CapacityProgress::Blocked); + } + return Err(error); + } + self.capacity_sync_pending = false; + self.record_capacity_recovered(CapacityOperation::DataWrite); + Ok(CapacityProgress::Ready) } else { - Ok(()) + self.record_capacity_recovered(CapacityOperation::DataWrite); + Ok(CapacityProgress::Ready) + } + } + + pub(crate) async fn try_flush(&mut self) -> io::Result { + match self + .finish_pending_record_write() + .await + .map_err(|error| match error { + WriterError::Io { source } => source, + error => io::Error::other(error.to_string()), + })? { + PendingWriteOutcome::CapacityBlocked => return Ok(CapacityProgress::Blocked), + PendingWriteOutcome::None | PendingWriteOutcome::Complete(_) => {} } + self.try_flush_inner(false).await + } + + pub(crate) async fn retry_capacity(&mut self) -> io::Result { + // Cancellation can occur while a production `spawn_blocking` write is still in flight, + // before its result records a capacity episode. Probe writer-owned work first so the + // shared retry driver can finish that syscall and publish the record. + if self.pending_record_write.is_some() + || self + .writer + .as_ref() + .is_some_and(RecordWriter::current_record_may_be_committed) + { + return self.try_flush().await; + } + + match self + .capacity_backpressure + .as_ref() + .map(|state| state.operation) + { + Some(CapacityOperation::DataWrite) => self.try_flush().await, + Some(CapacityOperation::OpenDataFile) => { + match self.ensure_ready_for_write(false).await? { + EnsureReady::Ready => { + self.record_capacity_recovered(CapacityOperation::OpenDataFile); + Ok(CapacityProgress::Ready) + } + EnsureReady::RetryCapacity(operation, error) => { + self.record_capacity_error( + operation, + &error, + FILESYSTEM_FULL_RETRY_INITIAL, + ); + Ok(CapacityProgress::Blocked) + } + EnsureReady::WaitingForReader => Ok(CapacityProgress::WaitingForReader), + } + } + None => Ok(CapacityProgress::Ready), + } + } + + pub(crate) fn is_capacity_blocked(&self) -> bool { + self.capacity_backpressure.is_some() + } + + pub(crate) fn fail_pending_write(&mut self) { + self.pending_record_write.take(); + } + + pub(crate) fn reader_progress_waiter( + &self, + ) -> impl Future + Send + use + 'static { + let notify = self.ledger.reader_notifier(); + async move { notify.notified().await } } /// Flushes the writer. @@ -1955,8 +2404,14 @@ where /// variant will be returned describing the error. #[instrument(skip(self), level = "trace")] pub async fn flush(&mut self) -> io::Result<()> { - self.flush_inner(false).await?; - Ok(()) + let mut retry = CapacityRetry::default(); + loop { + match self.try_flush().await? { + CapacityProgress::Ready => return Ok(()), + CapacityProgress::Blocked => self.wait_for_capacity(&mut retry).await, + CapacityProgress::WaitingForReader => self.ledger.wait_for_reader().await, + } + } } } @@ -1992,3 +2447,140 @@ where self.close(); } } + +#[cfg(test)] +mod tests { + use std::{ + io, + pin::Pin, + sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + }, + task::{Context, Poll}, + time::Duration, + }; + + use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; + + use super::TrackingBufWriter; + use crate::variants::disk_v2::io::{AsyncFile, Metadata}; + + #[derive(Debug)] + struct PendingAfterPrefixWriter { + bytes: Vec, + pause: Arc, + wrote_prefix: bool, + } + + impl AsyncWrite for PendingAfterPrefixWriter { + fn poll_write( + mut self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + if self.wrote_prefix && self.pause.load(Ordering::Acquire) { + return Poll::Pending; + } + + let amount = if self.wrote_prefix { + buf.len() + } else { + buf.len().min(4) + }; + self.bytes.extend_from_slice(&buf[..amount]); + self.wrote_prefix = true; + Poll::Ready(Ok(amount)) + } + + fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + } + + impl AsyncRead for PendingAfterPrefixWriter { + fn poll_read( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + _buf: &mut ReadBuf<'_>, + ) -> Poll> { + Poll::Ready(Ok(())) + } + } + + impl AsyncFile for PendingAfterPrefixWriter { + async fn metadata(&self) -> io::Result { + Ok(Metadata { + len: self.bytes.len() as u64, + }) + } + + async fn truncate(&self, _size: u64) -> io::Result<()> { + Ok(()) + } + + async fn sync_all(&self) -> io::Result<()> { + Ok(()) + } + } + + #[derive(Clone, Copy)] + enum CancelledWritePath { + Flush, + Direct, + } + + async fn assert_cancelled_write_resumes(path: CancelledWritePath) { + let pause = Arc::new(AtomicBool::new(true)); + let inner = PendingAfterPrefixWriter { + bytes: Vec::new(), + pause: Arc::clone(&pause), + wrote_prefix: false, + }; + let capacity = match path { + CancelledWritePath::Flush => 16, + CancelledWritePath::Direct => 4, + }; + let mut writer = TrackingBufWriter::with_capacity(capacity, inner); + let expected = b"abcdefghij"; + if matches!(path, CancelledWritePath::Flush) { + assert_eq!(writer.write(1, expected).await.unwrap(), None); + } + + let timed_out = match path { + CancelledWritePath::Flush => { + tokio::time::timeout(Duration::from_millis(10), writer.flush()).await + } + CancelledWritePath::Direct => { + tokio::time::timeout(Duration::from_millis(10), writer.write(1, expected)).await + } + }; + assert!(timed_out.is_err()); + assert_eq!(writer.get_ref().bytes, b"abcd"); + + pause.store(false, Ordering::Release); + let result = match path { + CancelledWritePath::Flush => writer.flush().await, + CancelledWritePath::Direct => writer.write(1, expected).await, + } + .unwrap() + .unwrap(); + assert_eq!(result.events_flushed, 1); + assert_eq!(result.bytes_flushed, expected.len() as u64); + assert_eq!(writer.get_ref().bytes, expected); + } + + #[tokio::test] + async fn cancelled_flush_resumes_after_committed_prefix() { + assert_cancelled_write_resumes(CancelledWritePath::Flush).await; + } + + #[tokio::test] + async fn cancelled_direct_write_resumes_after_committed_prefix() { + assert_cancelled_write_resumes(CancelledWritePath::Direct).await; + } +} From 698f2bb531b764ef26ce7517e35248c6b67fb2f0 Mon Sep 17 00:00:00 2001 From: Jansen Date: Wed, 2 Sep 2026 11:05:39 -0400 Subject: [PATCH 2/5] docs(buffers): clarify disk capacity behavior --- changelog.d/disk_v2_runtime_capacity_backpressure.fix.md | 3 +++ lib/vector-buffers/src/lib.rs | 5 ++++- website/cue/reference/components/generated/sinks.cue | 9 +++++++-- website/cue/reference/generated/configuration.cue | 9 +++++++-- 4 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 changelog.d/disk_v2_runtime_capacity_backpressure.fix.md diff --git a/changelog.d/disk_v2_runtime_capacity_backpressure.fix.md b/changelog.d/disk_v2_runtime_capacity_backpressure.fix.md new file mode 100644 index 0000000000000..c90e1ba26bf8b --- /dev/null +++ b/changelog.d/disk_v2_runtime_capacity_backpressure.fix.md @@ -0,0 +1,3 @@ +`disk_v2` buffers now apply their configured `when_full` policy when runtime filesystem space or quota is exhausted. Blocking buffers retry with backpressure, while drop-newest and overflow buffers promptly handle subsequent unwritten events according to policy. Records whose writes have already started remain owned by the disk buffer and complete exactly once after capacity recovers. Startup and non-capacity I/O failures remain fatal. + +authors: Jansen-w diff --git a/lib/vector-buffers/src/lib.rs b/lib/vector-buffers/src/lib.rs index 81e3d0c0f2d55..5516c40d06dc8 100644 --- a/lib/vector-buffers/src/lib.rs +++ b/lib/vector-buffers/src/lib.rs @@ -46,7 +46,10 @@ use vector_common::{ finalization::{AddBatchNotifier, Finalizable, GroupedFinalizable}, }; -/// Event handling behavior when a buffer is full. +/// Controls what happens when a buffer reaches its configured size limit. +/// +/// If the disk fills, Vector follows this setting for new events. An event already being written +/// finishes when space becomes available. Initialization and other I/O errors remain fatal. #[configurable_component] #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] #[serde(rename_all = "snake_case")] diff --git a/website/cue/reference/components/generated/sinks.cue b/website/cue/reference/components/generated/sinks.cue index 6268666fc49c5..7f6ab8af91e50 100644 --- a/website/cue/reference/components/generated/sinks.cue +++ b/website/cue/reference/components/generated/sinks.cue @@ -51,8 +51,13 @@ generated: components: sinks: configuration: { } } when_full: { - description: "Event handling behavior when a buffer is full." - required: false + description: """ + Controls what happens when a buffer reaches its configured size limit. + + If the disk fills, Vector follows this setting for new events. An event already being written + finishes when space becomes available. Initialization and other I/O errors remain fatal. + """ + required: false type: string: { default: "block" enum: { diff --git a/website/cue/reference/generated/configuration.cue b/website/cue/reference/generated/configuration.cue index 0daf2245874f0..670a61c81786b 100644 --- a/website/cue/reference/generated/configuration.cue +++ b/website/cue/reference/generated/configuration.cue @@ -1032,8 +1032,13 @@ generated: configuration: { } default: "block" } - description: "Event handling behavior when a buffer is full." - required: false + description: """ + Controls what happens when a buffer reaches its configured size limit. + + If the disk fills, Vector follows this setting for new events. An event already being written + finishes when space becomes available. Initialization and other I/O errors remain fatal. + """ + required: false } max_events: { type: uint: default: 500 From eda697717c2d45c65a11c1a3d04c7cebad386425 Mon Sep 17 00:00:00 2001 From: Jansen Date: Thu, 3 Sep 2026 16:53:37 -0400 Subject: [PATCH 3/5] fix(buffers): preserve terminal retry failures --- .../src/topology/channel/sender.rs | 57 ++++++++++++++----- .../src/topology/channel/tests.rs | 41 +++++++++++-- .../disk_v2/tests/model/filesystem.rs | 23 +++++--- 3 files changed, 92 insertions(+), 29 deletions(-) diff --git a/lib/vector-buffers/src/topology/channel/sender.rs b/lib/vector-buffers/src/topology/channel/sender.rs index 5e67044bf7751..a26259f7e9e1e 100644 --- a/lib/vector-buffers/src/topology/channel/sender.rs +++ b/lib/vector-buffers/src/topology/channel/sender.rs @@ -38,7 +38,35 @@ const CAPACITY_RETRY_MAX: Duration = Duration::from_secs(5); enum CapacityState { Ready, Retrying, - Failed(String), + Failed(TerminalError), +} + +#[derive(Clone, Debug)] +struct TerminalError { + kind: io::ErrorKind, + raw_os_error: Option, + message: String, +} + +impl From<&io::Error> for TerminalError { + fn from(error: &io::Error) -> Self { + Self { + kind: error.kind(), + raw_os_error: error.raw_os_error(), + message: error.to_string(), + } + } +} + +impl TerminalError { + fn to_io_error(&self) -> io::Error { + // io::Error can preserve either a raw OS error or a custom message, but not both. + // Prefer the raw error when present so callers retain its platform classification. + match self.raw_os_error { + Some(raw_os_error) => io::Error::from_raw_os_error(raw_os_error), + None => io::Error::new(self.kind, self.message.clone()), + } + } } #[cfg(test)] @@ -197,13 +225,6 @@ where } } - fn check_terminal_error(&self) -> crate::Result<()> { - if let CapacityState::Failed(error) = &*self.capacity_state.lock().expect("poisoned") { - return Err(io::Error::other(error.clone()).into()); - } - Ok(()) - } - fn is_capacity_blocked(&self) -> bool { matches!( *self.capacity_state.lock().expect("poisoned"), @@ -284,7 +305,7 @@ where error!(%error, "Disk buffer capacity retry failed."); let mut capacity_state = self.capacity_state.lock().expect("poisoned"); if matches!(*capacity_state, CapacityState::Retrying) { - *capacity_state = CapacityState::Failed(error.to_string()); + *capacity_state = CapacityState::Failed(TerminalError::from(error)); self.capacity_notify.notify_waiters(); } } @@ -354,7 +375,7 @@ where match &*self.capacity_state.lock().expect("poisoned") { CapacityState::Ready => return Ok(()), CapacityState::Failed(error) => { - return Err(io::Error::other(error.clone()).into()); + return Err(error.to_io_error().into()); } CapacityState::Retrying => {} } @@ -364,7 +385,13 @@ where } async fn try_send_record(self: &Arc, item: T) -> crate::Result> { - self.check_terminal_error()?; + let mut item = item; + let finalizers = UnownedFinalizers::take(&mut item); + if let CapacityState::Failed(error) = &*self.capacity_state.lock().expect("poisoned") { + return Err(error.to_io_error().into()); + } + finalizers.restore(&mut item); + let pre_count = item.event_count() as u64; let pre_size = item.size_of() as u64; let Some(mut item) = item.filter_unencodable() else { @@ -398,7 +425,7 @@ where return Ok(TryWriteOutcome::Full(item)); } CapacityState::Failed(error) => { - return Err(io::Error::other(error.clone()).into()); + return Err(error.to_io_error().into()); } } @@ -416,7 +443,7 @@ where return Ok(TryWriteOutcome::Full(item)); } CapacityState::Failed(error) => { - return Err(io::Error::other(error.clone()).into()); + return Err(error.to_io_error().into()); } } } @@ -482,7 +509,7 @@ where CapacityState::Ready => false, CapacityState::Retrying => true, CapacityState::Failed(error) => { - return Err(io::Error::other(error.clone()).into()); + return Err(error.to_io_error().into()); } } }; @@ -500,7 +527,7 @@ where CapacityState::Ready => false, CapacityState::Retrying => true, CapacityState::Failed(error) => { - return Err(io::Error::other(error.clone()).into()); + return Err(error.to_io_error().into()); } } }; diff --git a/lib/vector-buffers/src/topology/channel/tests.rs b/lib/vector-buffers/src/topology/channel/tests.rs index 0d27867ee86be..498a345305b2a 100644 --- a/lib/vector-buffers/src/topology/channel/tests.rs +++ b/lib/vector-buffers/src/topology/channel/tests.rs @@ -33,6 +33,15 @@ impl Drop for WriteGateCleanup { } } +fn assert_terminal_permission_denied(error: vector_common::Error) { + let error = error + .downcast::() + .expect("terminal error should retain its I/O classification"); + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied); + #[cfg(unix)] + assert_eq!(error.raw_os_error(), Some(libc::EACCES)); +} + async fn assert_send_ok_with_capacities( sender: &mut BufferSender, value: impl Into, @@ -671,17 +680,39 @@ async fn terminal_capacity_retry_wakes_waiters_and_stays_failed() { let waiting = tokio::spawn(async move { blocker.send(Record::new(113, 64, 1), None).await }); sleep(Duration::from_millis(20)).await; assert!(!waiting.is_finished(), "block send should await capacity"); + #[cfg(unix)] + filesystem.fail_data_writes_after_raw_os_error(0, libc::EACCES); + #[cfg(not(unix))] filesystem.fail_data_writes_after(0, io::ErrorKind::PermissionDenied); - assert!( + assert_terminal_permission_denied( timeout(Duration::from_secs(1), waiting) .await .expect("terminal retry must wake the waiter") .unwrap() - .is_err() + .expect_err("terminal retry must fail the waiter"), ); - assert!( - owner.send(Record::new(114, 64, 1), None).await.is_err(), - "terminal retry state must reject subsequent sends" + + let mut rejected = Record::new(114, 64, 1); + let (batch, finalizer) = BatchNotifier::new_with_receiver(); + rejected.add_batch_notifier(batch); + assert_terminal_permission_denied( + owner + .send(rejected, None) + .await + .expect_err("terminal retry state must reject subsequent sends"), + ); + assert_eq!( + timeout(Duration::from_secs(1), finalizer) + .await + .expect("terminal send finalizer should resolve"), + BatchStatus::Errored, + "terminal sends must nack finalizers for source redelivery" + ); + assert_terminal_permission_denied( + owner + .flush() + .await + .expect_err("terminal retry state must reject flushes"), ); drop(owner); drop(reader); diff --git a/lib/vector-buffers/src/variants/disk_v2/tests/model/filesystem.rs b/lib/vector-buffers/src/variants/disk_v2/tests/model/filesystem.rs index 7857341451897..d17e6f83fe495 100644 --- a/lib/vector-buffers/src/variants/disk_v2/tests/model/filesystem.rs +++ b/lib/vector-buffers/src/variants/disk_v2/tests/model/filesystem.rs @@ -35,7 +35,7 @@ struct FileInner { #[derive(Debug, Default)] struct FaultState { - data_write_error: Option, + data_write_error: Option, data_write_attempts: usize, bytes_until_error: Option, max_write_size: Option, @@ -65,7 +65,6 @@ impl OpenFault { #[derive(Clone, Copy, Debug)] enum FaultError { - #[cfg(not(unix))] Kind(io::ErrorKind), #[cfg(unix)] RawOs(i32), @@ -74,7 +73,6 @@ enum FaultError { impl FaultError { fn into_error(self) -> io::Error { match self { - #[cfg(not(unix))] Self::Kind(kind) => kind.into(), #[cfg(unix)] Self::RawOs(raw) => io::Error::from_raw_os_error(raw), @@ -240,11 +238,10 @@ impl AsyncWrite for TestFile { let mut faults = self.faults.lock().expect("poisoned"); faults.data_write_attempts += 1; if faults.bytes_until_error == Some(0) { - return Err(io::Error::from( - faults - .data_write_error - .unwrap_or(io::ErrorKind::StorageFull), - )) + return Err(faults + .data_write_error + .unwrap_or(FaultError::Kind(io::ErrorKind::StorageFull)) + .into_error()) .into(); } @@ -429,7 +426,15 @@ impl TestFilesystem { pub(crate) fn fail_data_writes_after(&self, bytes: usize, kind: io::ErrorKind) { self.with_faults(|faults| { - faults.data_write_error = Some(kind); + faults.data_write_error = Some(FaultError::Kind(kind)); + faults.bytes_until_error = Some(bytes); + }); + } + + #[cfg(unix)] + pub(crate) fn fail_data_writes_after_raw_os_error(&self, bytes: usize, raw_os_error: i32) { + self.with_faults(|faults| { + faults.data_write_error = Some(FaultError::RawOs(raw_os_error)); faults.bytes_until_error = Some(bytes); }); } From 0856d9cf1734673ff2b69767e9a8d28409aa3aae Mon Sep 17 00:00:00 2001 From: Jansen Date: Thu, 3 Sep 2026 17:03:21 -0400 Subject: [PATCH 4/5] fix(buffers): report capacity retries once --- .../disk_v2/tests/runtime_capacity.rs | 68 +++++++++++++++++-- .../src/variants/disk_v2/writer.rs | 36 ++++++++-- 2 files changed, 92 insertions(+), 12 deletions(-) diff --git a/lib/vector-buffers/src/variants/disk_v2/tests/runtime_capacity.rs b/lib/vector-buffers/src/variants/disk_v2/tests/runtime_capacity.rs index 586dae10a8826..86417ce2ed158 100644 --- a/lib/vector-buffers/src/variants/disk_v2/tests/runtime_capacity.rs +++ b/lib/vector-buffers/src/variants/disk_v2/tests/runtime_capacity.rs @@ -586,27 +586,83 @@ async fn runtime_rotation_sync_capacity_retries_without_losing_current_record() assert_written(&mut writer, current).await; } -#[tokio::test] -async fn rotation_flush_capacity_error_retains_data_write_operation() { +#[tokio::test(start_paused = true)] +async fn rotation_flush_capacity_error_is_recorded_once_with_scheduled_retries() { + vector_common::event_test_util::clear_recorded_events(); let filesystem = TestFilesystem::default(); let CapacityBuffer { mut writer, .. } = build_rotating_buffer(filesystem.clone()).await; fill_for_rotation(&mut writer, 80).await; assert_written(&mut writer, Record::new(82, 64, 1)).await; assert_written(&mut writer, Record::new(83, 64, 1)).await; + let write_attempts_before_fault = filesystem.data_write_attempts(); + #[cfg(unix)] + filesystem.fail_data_writes_after_raw_os_error(0, libc::ENOSPC); + #[cfg(not(unix))] filesystem.fail_data_writes_after(0, io::ErrorKind::StorageFull); let record = Record::new(84, 64, 1); assert_eq!( writer.try_write_record(record.clone()).await.unwrap(), TryWriteOutcome::Full(record.clone()) ); + assert_eq!( + filesystem.data_write_attempts() - write_attempts_before_fault, + 1 + ); + #[cfg(unix)] + assert_eq!( + writer.capacity_backpressure_details(), + Some(( + "persist data", + 1, + io::ErrorKind::StorageFull, + Some(libc::ENOSPC) + )) + ); + #[cfg(not(unix))] + assert_eq!( + writer.capacity_backpressure_details(), + Some(("persist data", 1, io::ErrorKind::StorageFull, None)) + ); + vector_common::event_test_util::contains_name_once("DiskBufferBackpressure").unwrap(); - filesystem.restore_data_writes(); + let mut write = Box::pin(writer.write_record(record.clone())); + tokio::select! { + biased; + result = &mut write => panic!("capacity-blocked rotation unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } assert_eq!( - writer.retry_capacity().await.unwrap(), - CapacityProgress::Ready + filesystem.data_write_attempts() - write_attempts_before_fault, + 2 ); - assert_written(&mut writer, record).await; + + tokio::time::advance(Duration::from_millis(100)).await; + tokio::select! { + biased; + result = &mut write => panic!("capacity-blocked rotation unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + assert_eq!( + filesystem.data_write_attempts() - write_attempts_before_fault, + 3 + ); + + tokio::time::advance(Duration::from_millis(100)).await; + tokio::select! { + biased; + result = &mut write => panic!("capacity-blocked rotation unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + assert_eq!( + filesystem.data_write_attempts() - write_attempts_before_fault, + 3 + ); + + filesystem.restore_data_writes(); + tokio::time::advance(Duration::from_millis(100)).await; + write.await.unwrap(); + vector_common::event_test_util::contains_name_once("DiskBufferBackpressureRecovered").unwrap(); } #[tokio::test(start_paused = true)] diff --git a/lib/vector-buffers/src/variants/disk_v2/writer.rs b/lib/vector-buffers/src/variants/disk_v2/writer.rs index 78bd1e27bb40e..cf2ed09cfb966 100644 --- a/lib/vector-buffers/src/variants/disk_v2/writer.rs +++ b/lib/vector-buffers/src/variants/disk_v2/writer.rs @@ -337,6 +337,10 @@ struct CapacityBackpressure { operation: CapacityOperation, retries: u64, started: Instant, + #[cfg(test)] + error_kind: io::ErrorKind, + #[cfg(test)] + raw_os_error: Option, } struct CapacityRetry { @@ -355,6 +359,7 @@ impl Default for CapacityRetry { enum EnsureReady { Ready, + CapacityBlocked, RetryCapacity(CapacityOperation, io::Error), WaitingForReader, } @@ -368,7 +373,7 @@ enum CapacityOperation { impl CapacityOperation { const fn as_str(self) -> &'static str { match self { - Self::DataWrite => "write data", + Self::DataWrite => "persist data", Self::OpenDataFile => "open data file", } } @@ -1101,9 +1106,27 @@ where operation, retries: 1, started: Instant::now(), + #[cfg(test)] + error_kind: error.kind(), + #[cfg(test)] + raw_os_error: error.raw_os_error(), }); } + #[cfg(test)] + pub(crate) fn capacity_backpressure_details( + &self, + ) -> Option<(&'static str, u64, io::ErrorKind, Option)> { + self.capacity_backpressure.as_ref().map(|backpressure| { + ( + backpressure.operation.as_str(), + backpressure.retries, + backpressure.error_kind, + backpressure.raw_os_error, + ) + }) + } + fn record_capacity_recovered(&mut self, operation: CapacityOperation) { let Some(backpressure) = self .capacity_backpressure @@ -1773,10 +1796,7 @@ where // We still flush ourselves to disk, etc, to make sure all of the data is there. should_open_next = true; if matches!(self.try_flush_inner(true).await?, CapacityProgress::Blocked) { - return Ok(EnsureReady::RetryCapacity( - CapacityOperation::DataWrite, - io::ErrorKind::StorageFull.into(), - )); + return Ok(EnsureReady::CapacityBlocked); } self.reset(); @@ -1939,6 +1959,9 @@ where async fn ensure_ready_for_write_fatal(&mut self) -> io::Result<()> { match self.ensure_ready_for_write(true).await? { EnsureReady::Ready => Ok(()), + EnsureReady::CapacityBlocked => { + unreachable!("blocking readiness cannot retry an already-recorded capacity error") + } EnsureReady::RetryCapacity(_, error) => Err(error), EnsureReady::WaitingForReader => unreachable!("blocking readiness must wait"), } @@ -2050,7 +2073,7 @@ where record.merge_finalizer_groups(record_finalizers.into_inner()); return Ok(WriteAttempt::Full(record)); } - Ok(EnsureReady::WaitingForReader) => { + Ok(EnsureReady::CapacityBlocked | EnsureReady::WaitingForReader) => { let mut record = record; record.merge_finalizer_groups(record_finalizers.into_inner()); return Ok(WriteAttempt::Full(record)); @@ -2360,6 +2383,7 @@ where self.record_capacity_recovered(CapacityOperation::OpenDataFile); Ok(CapacityProgress::Ready) } + EnsureReady::CapacityBlocked => Ok(CapacityProgress::Blocked), EnsureReady::RetryCapacity(operation, error) => { self.record_capacity_error( operation, From 813b07ed17315a673260c5b4cbadc4a44c1c7b66 Mon Sep 17 00:00:00 2001 From: Jansen Date: Thu, 3 Sep 2026 17:11:28 -0400 Subject: [PATCH 5/5] refactor(buffers): isolate disk sender retries --- .../src/topology/channel/disk_v2_sender.rs | 586 +++++++++++++++ .../topology/channel/disk_v2_sender_tests.rs | 696 +++++++++++++++++ .../src/topology/channel/mod.rs | 7 + .../src/topology/channel/sender.rs | 575 +-------------- .../src/topology/channel/tests.rs | 697 +----------------- 5 files changed, 1294 insertions(+), 1267 deletions(-) create mode 100644 lib/vector-buffers/src/topology/channel/disk_v2_sender.rs create mode 100644 lib/vector-buffers/src/topology/channel/disk_v2_sender_tests.rs diff --git a/lib/vector-buffers/src/topology/channel/disk_v2_sender.rs b/lib/vector-buffers/src/topology/channel/disk_v2_sender.rs new file mode 100644 index 0000000000000..608bb6d47932c --- /dev/null +++ b/lib/vector-buffers/src/topology/channel/disk_v2_sender.rs @@ -0,0 +1,586 @@ +use std::{ + fmt, io, + sync::{Arc, Mutex as StdMutex}, + time::Duration, +}; + +use futures::future::BoxFuture; +use tokio::{ + sync::{Mutex, Notify}, + time::sleep, +}; +use vector_common::finalization::EventStatus; + +use crate::{ + Bufferable, + buffer_usage_data::BufferUsageHandle, + variants::disk_v2::{ + self, CapacityProgress, Filesystem, ProductionFilesystem, TryWriteOutcome, + }, +}; + +#[cfg(test)] +use crate::variants::disk_v2::tests::model::filesystem::TestFilesystem; + +const CAPACITY_RETRY_INITIAL: Duration = Duration::from_millis(100); +const CAPACITY_RETRY_MAX: Duration = Duration::from_secs(5); + +#[derive(Debug)] +enum CapacityState { + Ready, + Retrying, + Failed(TerminalError), +} + +#[derive(Clone, Debug)] +struct TerminalError { + kind: io::ErrorKind, + raw_os_error: Option, + message: String, +} + +impl From<&io::Error> for TerminalError { + fn from(error: &io::Error) -> Self { + Self { + kind: error.kind(), + raw_os_error: error.raw_os_error(), + message: error.to_string(), + } + } +} + +impl TerminalError { + fn to_io_error(&self) -> io::Error { + // io::Error can preserve either a raw OS error or a custom message, but not both. + // Prefer the raw error when present so callers retain its platform classification. + match self.raw_os_error { + Some(raw_os_error) => io::Error::from_raw_os_error(raw_os_error), + None => io::Error::new(self.kind, self.message.clone()), + } + } +} + +#[cfg(test)] +#[derive(Debug, Default)] +pub(crate) struct CapacityBlockedHook { + pub(crate) detected: Notify, + pub(crate) resume: Notify, +} + +struct UnownedFinalizers(Option); + +impl UnownedFinalizers { + fn take(item: &mut T) -> Self { + Self(Some(item.take_finalizer_groups())) + } + + fn restore(mut self, item: &mut T) { + item.merge_finalizer_groups(self.0.take().expect("finalizers must exist")); + } +} + +impl Drop for UnownedFinalizers { + fn drop(&mut self) { + if let Some(finalizers) = self.0.as_mut() { + finalizers.update_status(EventStatus::Errored); + } + } +} + +trait SenderFilesystem: Filesystem + fmt::Debug + Clone + Sized + 'static +where + T: Bufferable, + Self::File: Unpin, +{ + fn try_write_record( + writer: &mut disk_v2::BufferWriter, + item: T, + ) -> BoxFuture<'_, Result, disk_v2::WriterError>>; + + fn try_flush( + writer: &mut disk_v2::BufferWriter, + ) -> BoxFuture<'_, io::Result>; + + fn retry_capacity( + writer: &mut disk_v2::BufferWriter, + ) -> BoxFuture<'_, io::Result>; +} + +macro_rules! impl_sender_filesystem { + ($filesystem:ty) => { + impl SenderFilesystem for $filesystem { + fn try_write_record( + writer: &mut disk_v2::BufferWriter, + item: T, + ) -> BoxFuture<'_, Result, disk_v2::WriterError>> { + Box::pin(writer.try_write_record(item)) + } + + fn try_flush( + writer: &mut disk_v2::BufferWriter, + ) -> BoxFuture<'_, io::Result> { + Box::pin(writer.try_flush()) + } + + fn retry_capacity( + writer: &mut disk_v2::BufferWriter, + ) -> BoxFuture<'_, io::Result> { + Box::pin(writer.retry_capacity()) + } + } + }; +} + +impl_sender_filesystem!(ProductionFilesystem); +#[cfg(test)] +impl_sender_filesystem!(TestFilesystem); + +#[derive(Debug)] +pub struct DiskV2Sender +where + T: Bufferable, + FS: Filesystem, + FS::File: fmt::Debug + Unpin, +{ + writer: Mutex>, + pub(super) usage: BufferUsageHandle, + capacity_state: StdMutex, + capacity_notify: Notify, + #[cfg(test)] + capacity_blocked_hook: StdMutex>>, +} + +struct WriterProgressOnCancel<'a, T, FS> +where + T: Bufferable, + FS: SenderFilesystem, + FS::File: fmt::Debug + Unpin, +{ + state: &'a Arc>, + armed: bool, +} + +impl<'a, T, FS> WriterProgressOnCancel<'a, T, FS> +where + T: Bufferable, + FS: SenderFilesystem, + FS::File: fmt::Debug + Unpin, +{ + fn new(state: &'a Arc>) -> Self { + Self { state, armed: true } + } + + fn disarm(&mut self) { + self.armed = false; + } +} + +impl Drop for WriterProgressOnCancel<'_, T, FS> +where + T: Bufferable, + FS: SenderFilesystem, + FS::File: fmt::Debug + Unpin, +{ + fn drop(&mut self) { + if self.armed { + self.state.start_cancellation_retry(); + } + } +} + +#[allow(private_bounds)] +impl DiskV2Sender +where + T: Bufferable, + FS: SenderFilesystem, + FS::File: fmt::Debug + Unpin, +{ + pub(super) fn new(writer: disk_v2::BufferWriter) -> Self { + let usage = writer.usage_handle(); + Self { + writer: Mutex::new(writer), + usage, + capacity_state: StdMutex::new(CapacityState::Ready), + capacity_notify: Notify::new(), + #[cfg(test)] + capacity_blocked_hook: StdMutex::new(None), + } + } + + #[cfg(test)] + async fn pause_after_capacity_blocked_for_test(&self) { + let hook = self.capacity_blocked_hook.lock().expect("poisoned").clone(); + if let Some(hook) = hook { + hook.detected.notify_waiters(); + hook.resume.notified().await; + } + } + + fn is_capacity_blocked(&self) -> bool { + matches!( + *self.capacity_state.lock().expect("poisoned"), + CapacityState::Retrying + ) + } + + fn start_capacity_retry(self: &Arc) { + let should_start = { + let mut capacity_state = self.capacity_state.lock().expect("poisoned"); + match *capacity_state { + CapacityState::Ready => { + *capacity_state = CapacityState::Retrying; + true + } + CapacityState::Retrying | CapacityState::Failed(_) => false, + } + }; + if should_start { + self.capacity_notify.notify_waiters(); + self.spawn_capacity_retry_driver(None); + } + } + + fn start_cancellation_retry(self: &Arc) { + let (retained_driver, classifier) = { + let mut capacity_state = self.capacity_state.lock().expect("poisoned"); + match *capacity_state { + CapacityState::Ready => { + *capacity_state = CapacityState::Retrying; + (Some(Arc::clone(self)), None) + } + CapacityState::Retrying => (None, Some(Arc::clone(self))), + CapacityState::Failed(_) => return, + } + }; + if let Some(state) = retained_driver { + self.capacity_notify.notify_waiters(); + self.spawn_capacity_retry_driver(Some(state)); + } else if let Some(state) = classifier { + Self::classify_cancelled_retry(state); + } + } + + fn classify_cancelled_retry(state: Arc) { + tokio::spawn(async move { + let result = { + let mut writer = state.writer.lock().await; + state.retry_capacity_once(&mut writer).await + }; + + let _ = result; + }); + } + + async fn retry_capacity_once( + &self, + writer: &mut disk_v2::BufferWriter, + ) -> Option> { + if !matches!( + *self.capacity_state.lock().expect("poisoned"), + CapacityState::Retrying + ) { + return None; + } + + let result = FS::retry_capacity(writer).await; + match &result { + Ok(CapacityProgress::Ready) if !writer.is_capacity_blocked() => { + let mut capacity_state = self.capacity_state.lock().expect("poisoned"); + if matches!(*capacity_state, CapacityState::Retrying) { + *capacity_state = CapacityState::Ready; + self.capacity_notify.notify_waiters(); + } + } + Err(error) => { + writer.fail_pending_write(); + error!(%error, "Disk buffer capacity retry failed."); + let mut capacity_state = self.capacity_state.lock().expect("poisoned"); + if matches!(*capacity_state, CapacityState::Retrying) { + *capacity_state = CapacityState::Failed(TerminalError::from(error)); + self.capacity_notify.notify_waiters(); + } + } + Ok( + CapacityProgress::Blocked + | CapacityProgress::WaitingForReader + | CapacityProgress::Ready, + ) => {} + } + Some(result) + } + + fn spawn_capacity_retry_driver(self: &Arc, mut retained_state: Option>) { + let weak_state = Arc::downgrade(self); + tokio::spawn(async move { + let mut delay = CAPACITY_RETRY_INITIAL; + loop { + let Some(state) = retained_state.clone().or_else(|| weak_state.upgrade()) else { + return; + }; + let reader_progress = { + let writer = state.writer.lock().await; + writer.reader_progress_waiter() + }; + drop(state); + tokio::select! { + () = sleep(delay) => {} + () = reader_progress => {} + } + let Some(state) = retained_state.clone().or_else(|| weak_state.upgrade()) else { + return; + }; + let result = { + let mut writer = state.writer.lock().await; + state.retry_capacity_once(&mut writer).await + }; + let Some(result) = result else { + return; + }; + // A cancellation can leave a production syscall running after its caller drops. + // Keep its sender alive through the first retry result, then return to weak ownership. + retained_state = None; + match result { + Ok(CapacityProgress::Ready) => { + if state.is_capacity_blocked() { + continue; + } + return; + } + Ok(CapacityProgress::Blocked) => { + delay = delay.saturating_mul(2).min(CAPACITY_RETRY_MAX); + } + Ok(CapacityProgress::WaitingForReader) => {} + Err(_) => { + return; + } + } + } + }); + } + + async fn wait_for_capacity(&self) -> crate::Result<()> { + let notified = self.capacity_notify.notified(); + tokio::pin!(notified); + loop { + notified.as_mut().enable(); + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => return Ok(()), + CapacityState::Failed(error) => { + return Err(error.to_io_error().into()); + } + CapacityState::Retrying => {} + } + notified.as_mut().await; + notified.set(self.capacity_notify.notified()); + } + } + + pub(super) async fn try_send_record( + self: &Arc, + item: T, + ) -> crate::Result> { + let mut item = item; + let finalizers = UnownedFinalizers::take(&mut item); + if let CapacityState::Failed(error) = &*self.capacity_state.lock().expect("poisoned") { + return Err(error.to_io_error().into()); + } + finalizers.restore(&mut item); + + let pre_count = item.event_count() as u64; + let pre_size = item.size_of() as u64; + let Some(mut item) = item.filter_unencodable() else { + self.usage + .increment_received_event_count_and_byte_size(pre_count, pre_size); + self.usage + .increment_dropped_event_count_and_byte_size(pre_count, pre_size, false); + return Ok(TryWriteOutcome::Dropped); + }; + if item.event_count() as u64 != pre_count { + let dropped_events = pre_count - item.event_count() as u64; + let dropped_bytes = pre_size.saturating_sub(item.size_of() as u64); + self.usage + .increment_received_event_count_and_byte_size(dropped_events, dropped_bytes); + self.usage.increment_dropped_event_count_and_byte_size( + dropped_events, + dropped_bytes, + false, + ); + } + + let finalizers = UnownedFinalizers::take(&mut item); + let notified = self.capacity_notify.notified(); + tokio::pin!(notified); + let mut writer = loop { + notified.as_mut().enable(); + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => {} + CapacityState::Retrying => { + finalizers.restore(&mut item); + return Ok(TryWriteOutcome::Full(item)); + } + CapacityState::Failed(error) => { + return Err(error.to_io_error().into()); + } + } + + tokio::select! { + biased; + () = notified.as_mut() => { + notified.set(self.capacity_notify.notified()); + } + writer = self.writer.lock() => { + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => break writer, + CapacityState::Retrying => { + drop(writer); + finalizers.restore(&mut item); + return Ok(TryWriteOutcome::Full(item)); + } + CapacityState::Failed(error) => { + return Err(error.to_io_error().into()); + } + } + } + } + }; + if writer.is_capacity_blocked() { + #[cfg(test)] + self.pause_after_capacity_blocked_for_test().await; + self.start_capacity_retry(); + drop(writer); + finalizers.restore(&mut item); + return Ok(TryWriteOutcome::Full(item)); + } + + finalizers.restore(&mut item); + let mut progress_on_cancel = WriterProgressOnCancel::new(self); + let result = FS::try_write_record(&mut writer, item).await; + progress_on_cancel.disarm(); + let outcome = result.inspect_err(|_error| { + error!("Disk buffer writer has encountered an unrecoverable error."); + })?; + let blocked = writer.is_capacity_blocked(); + if blocked { + #[cfg(test)] + self.pause_after_capacity_blocked_for_test().await; + self.start_capacity_retry(); + } + drop(writer); + Ok(outcome) + } + + pub(super) async fn send_record( + self: &Arc, + item: T, + ) -> crate::Result> { + let mut item = item; + loop { + match self.try_send_record(item).await? { + TryWriteOutcome::Written => return Ok(TryWriteOutcome::Written), + TryWriteOutcome::Dropped => return Ok(TryWriteOutcome::Dropped), + TryWriteOutcome::Pending => { + self.wait_for_capacity().await?; + return Ok(TryWriteOutcome::Written); + } + TryWriteOutcome::Full(mut returned) => { + let finalizers = UnownedFinalizers::take(&mut returned); + item = returned; + if self.is_capacity_blocked() { + self.wait_for_capacity().await?; + } else { + let writer = self.writer.lock().await; + let reader_progress = writer.reader_progress_waiter(); + drop(writer); + reader_progress.await; + } + finalizers.restore(&mut item); + } + } + } + } + + pub(super) async fn flush(self: &Arc, block: bool) -> crate::Result<()> { + loop { + let retrying = { + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => false, + CapacityState::Retrying => true, + CapacityState::Failed(error) => { + return Err(error.to_io_error().into()); + } + } + }; + if retrying { + if !block { + return Ok(()); + } + self.wait_for_capacity().await?; + continue; + } + + let mut writer = self.writer.lock().await; + let retrying = { + match &*self.capacity_state.lock().expect("poisoned") { + CapacityState::Ready => false, + CapacityState::Retrying => true, + CapacityState::Failed(error) => { + return Err(error.to_io_error().into()); + } + } + }; + if retrying { + drop(writer); + if !block { + return Ok(()); + } + self.wait_for_capacity().await?; + continue; + } + + // This guard only covers filesystem I/O. Cancelling while waiting for capacity must not + // start a second capacity retry driver. + let mut progress_on_cancel = WriterProgressOnCancel::new(self); + let result = FS::try_flush(&mut writer).await; + progress_on_cancel.disarm(); + let progress = result.map_err(|error| -> Box { + error!("Disk buffer writer has encountered an unrecoverable error."); + error.into() + })?; + + if matches!(progress, CapacityProgress::Blocked) { + // Publish Retrying before releasing the writer, so another flush cannot race in + // and issue another filesystem call before observing the capacity episode. + self.start_capacity_retry(); + } + drop(writer); + + if matches!(progress, CapacityProgress::Blocked) && block { + self.wait_for_capacity().await?; + continue; + } + return Ok(()); + } + } + + #[cfg(test)] + pub(crate) fn start_normal_capacity_retry_for_test(self: &Arc) { + self.start_capacity_retry(); + } + + #[cfg(test)] + pub(crate) fn start_cancellation_retry_for_test(self: &Arc) { + self.start_cancellation_retry(); + } + + #[cfg(test)] + pub(crate) fn set_capacity_blocked_hook(&self, hook: Arc) { + *self.capacity_blocked_hook.lock().expect("poisoned") = Some(hook); + } +} + +#[cfg(test)] +impl DiskV2Sender { + pub(crate) async fn next_writer_data_file_path(&self) -> std::path::PathBuf { + self.writer.lock().await.next_writer_data_file_path() + } +} diff --git a/lib/vector-buffers/src/topology/channel/disk_v2_sender_tests.rs b/lib/vector-buffers/src/topology/channel/disk_v2_sender_tests.rs new file mode 100644 index 0000000000000..1f4786f0563ad --- /dev/null +++ b/lib/vector-buffers/src/topology/channel/disk_v2_sender_tests.rs @@ -0,0 +1,696 @@ +use std::{io, sync::Arc, time::Duration}; + +use temp_dir::TempDir; +use tokio::time::{advance, sleep, timeout}; +use vector_common::finalization::{AddBatchNotifier, BatchNotifier, BatchStatus}; + +use crate::{ + WhenFull, + buffer_usage_data::BufferUsageHandle, + topology::channel::{BufferSender, CapacityBlockedHook, SenderAdapter}, + variants::disk_v2::{ + Buffer, DiskBufferConfigBuilder, ProductionFilesystem, StalledWrites, TestWriteGate, + tests::model::{filesystem::TestFilesystem, record::Record}, + }, +}; + +struct WriteGateCleanup(Arc); + +impl Drop for WriteGateCleanup { + fn drop(&mut self) { + self.0.release(); + } +} + +fn assert_terminal_permission_denied(error: vector_common::Error) { + let error = error + .downcast::() + .expect("terminal error should retain its I/O classification"); + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied); + #[cfg(unix)] + assert_eq!(error.raw_os_error(), Some(libc::EACCES)); +} + +// Physical disk exhaustion acts like a full buffer for records not yet owned by disk_v2. These +// tests verify each policy while partially written records recover exactly once. +async fn build_policy_disk( + filesystem: TestFilesystem, + write_buffer_size: usize, + rotating: bool, + when_full: WhenFull, +) -> ( + BufferSender, + crate::variants::disk_v2::BufferReader, + BufferUsageHandle, +) { + let directory = + std::env::temp_dir().join(format!("vector-policy-disk-{}", rand::random::())); + let mut builder = DiskBufferConfigBuilder::from_path(directory) + .write_buffer_size(write_buffer_size) + .filesystem(filesystem); + if rotating { + builder = builder.max_data_file_size(256).max_record_size(256); + } + let usage = BufferUsageHandle::noop(); + let (writer, reader, _) = Buffer::from_config_inner(builder.build().unwrap(), usage.clone()) + .await + .unwrap(); + ( + BufferSender::new(SenderAdapter::from(writer), when_full), + reader, + usage, + ) +} + +#[tokio::test] +async fn disk_capacity_block_retries_owned_record() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, mut reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::Block).await; + let record = Record::new(100, 256, 1); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + let mut send = Box::pin(sender.send(record.clone(), None)); + assert!( + timeout(Duration::from_millis(20), &mut send).await.is_err(), + "block policy must backpressure while the owned record retries" + ); + filesystem.restore_data_writes(); + timeout(Duration::from_secs(2), send) + .await + .expect("block send should recover") + .unwrap(); + sender.flush().await.unwrap(); + assert_eq!(reader.next().await.unwrap(), Some(record)); +} + +#[tokio::test] +async fn disk_capacity_drop_newest_is_prompt_and_counted_once() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, mut reader, usage) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let outer_usage = BufferUsageHandle::noop(); + sender.with_usage_instrumentation(outer_usage.clone()); + let owned = Record::new(101, 256, 1); + let dropped = Record::new(102, 64, 1); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + timeout(Duration::from_secs(1), sender.send(owned.clone(), None)) + .await + .expect("the owned drop-newest record should be accepted") + .unwrap(); + timeout(Duration::from_millis(20), sender.send(dropped, None)) + .await + .expect("drop-newest send must not wait for the retry driver") + .unwrap(); + assert_eq!(usage.snapshot().dropped_event_count_intentional, 1); + assert_eq!(outer_usage.snapshot().dropped_event_count_intentional, 0); + + filesystem.restore_data_writes(); + timeout(Duration::from_secs(2), async { + while usage.snapshot().received_event_count < 2 { + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("owned record should complete in the background"); + assert_eq!(reader.next().await.unwrap(), Some(owned)); + let snapshot = usage.snapshot(); + assert_eq!(snapshot.received_event_count, 2); + assert_eq!(snapshot.dropped_event_count_intentional, 1); +} + +#[tokio::test(start_paused = true)] +async fn disk_capacity_drop_newest_flush_skips_writer_while_retrying() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, _reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + sender.send(Record::new(120, 256, 1), None).await.unwrap(); + tokio::task::yield_now().await; + let write_attempts = filesystem.data_write_attempts(); + + for id in 121..124 { + sender.send(Record::new(id, 64, 1), None).await.unwrap(); + sender.flush().await.unwrap(); + } + assert_eq!(filesystem.data_write_attempts(), write_attempts); + + // The retry driver is the only path allowed to make another filesystem attempt, and it + // retains its exponential-backoff schedule while sends and flushes are dropped. + advance(Duration::from_millis(99)).await; + tokio::task::yield_now().await; + assert_eq!(filesystem.data_write_attempts(), write_attempts); + advance(Duration::from_millis(1)).await; + tokio::task::yield_now().await; + assert_eq!(filesystem.data_write_attempts(), write_attempts + 1); +} + +#[tokio::test] +async fn disk_capacity_send_publishes_retrying_before_releasing_writer() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, _reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match sender.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let hook = Arc::new(CapacityBlockedHook::default()); + state.set_capacity_blocked_hook(Arc::clone(&hook)); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + let detected = hook.detected.notified(); + tokio::pin!(detected); + detected.as_mut().enable(); + let send = tokio::spawn(async move { sender.send(Record::new(127, 256, 1), None).await }); + detected.await; + let write_attempts = filesystem.data_write_attempts(); + + let mut flusher = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::DropNewest); + let flush = tokio::spawn(async move { flusher.flush().await }); + tokio::task::yield_now().await; + hook.resume.notify_waiters(); + + send.await.unwrap().unwrap(); + flush.await.unwrap().unwrap(); + assert_eq!(filesystem.data_write_attempts(), write_attempts); +} + +#[tokio::test] +async fn disk_capacity_overflow_routes_from_cloned_sender() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, mut base_reader, base_usage) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let (overflow_sender, mut overflow_reader, overflow_usage) = + build_policy_disk(TestFilesystem::default(), 1024, false, WhenFull::Block).await; + sender.switch_to_overflow(overflow_sender); + let mut clone = sender.clone(); + let owned = Record::new(103, 256, 1); + let overflowed = Record::new(104, 64, 1); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + timeout(Duration::from_secs(1), sender.send(owned.clone(), None)) + .await + .expect("the owned overflow record should be accepted by the base") + .unwrap(); + timeout( + Duration::from_millis(20), + clone.send(overflowed.clone(), None), + ) + .await + .expect("cloned overflow sender must not wait for the base retry") + .unwrap(); + clone.flush().await.unwrap(); + timeout(Duration::from_secs(1), async { + while overflow_usage.snapshot().received_event_count < 1 { + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("overflowed record should be flushed"); + assert_eq!(overflow_reader.next().await.unwrap(), Some(overflowed)); + + filesystem.restore_data_writes(); + timeout(Duration::from_secs(2), async { + while base_usage.snapshot().received_event_count < 1 { + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("base owned record should recover"); + assert_eq!(base_reader.next().await.unwrap(), Some(owned)); +} + +#[tokio::test(start_paused = true)] +async fn disk_capacity_overflow_flushes_base_without_retrying_writer() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, _base_reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let (overflow_sender, mut overflow_reader, overflow_usage) = + build_policy_disk(TestFilesystem::default(), 1024, false, WhenFull::Block).await; + sender.switch_to_overflow(overflow_sender); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + sender.send(Record::new(124, 256, 1), None).await.unwrap(); + tokio::task::yield_now().await; + let write_attempts = filesystem.data_write_attempts(); + let overflowed = Record::new(125, 64, 1); + sender.send(overflowed.clone(), None).await.unwrap(); + sender.flush().await.unwrap(); + + assert_eq!(filesystem.data_write_attempts(), write_attempts); + assert_eq!(overflow_usage.snapshot().received_event_count, 1); + assert_eq!(overflow_reader.next().await.unwrap(), Some(overflowed)); +} + +#[tokio::test(start_paused = true)] +async fn disk_capacity_blocking_flush_waits_then_recovers() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut owner, mut reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match owner.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let mut blocker = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::Block); + let record = Record::new(126, 256, 1); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + owner.send(record.clone(), None).await.unwrap(); + tokio::task::yield_now().await; + let write_attempts = filesystem.data_write_attempts(); + + let mut flush = Box::pin(blocker.flush()); + tokio::select! { + result = &mut flush => panic!("capacity-blocked flush unexpectedly completed: {result:?}"), + () = tokio::task::yield_now() => {} + } + assert_eq!(filesystem.data_write_attempts(), write_attempts); + + filesystem.restore_data_writes(); + advance(Duration::from_millis(100)).await; + flush.await.unwrap(); + assert_eq!(reader.next().await.unwrap(), Some(record)); +} + +#[tokio::test] +async fn disk_retry_waiting_for_reader_releases_sender_mutex() { + let filesystem = TestFilesystem::default(); + let (mut sender, _reader, _) = + build_policy_disk(filesystem.clone(), 1024, true, WhenFull::DropNewest).await; + for id in 105..107 { + sender.send(Record::new(id, 64, 1), None).await.unwrap(); + sender.flush().await.unwrap(); + } + let state = match sender.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let next_path = state.next_writer_data_file_path().await; + filesystem.fail_data_file_open(io::ErrorKind::StorageFull); + sender.send(Record::new(107, 64, 1), None).await.unwrap(); + filesystem.restore_data_file_open(); + filesystem.create_data_file_with_data(&next_path, b"occupied"); + sleep(Duration::from_millis(150)).await; + + let mut clone = sender.clone(); + timeout( + Duration::from_millis(20), + clone.send(Record::new(108, 64, 1), None), + ) + .await + .expect("retry waiting for reader must not hold the sender mutex") + .unwrap(); +} + +#[tokio::test] +async fn ordinary_capacity_retry_does_not_retain_dropped_sender() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let weak = match sender.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::downgrade(state), + _ => unreachable!("test disk sender expected"), + }; + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + sender.send(Record::new(109, 256, 1), None).await.unwrap(); + + drop(sender); + drop(reader); + timeout(Duration::from_millis(100), async { + while weak.upgrade().is_some() { + tokio::task::yield_now().await; + } + }) + .await + .expect("ordinary retry task must not retain the disk sender or writer"); +} + +#[tokio::test] +async fn cancelling_unowned_block_send_does_not_detach_or_duplicate_record() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut owner, mut reader, usage) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match owner.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let weak = Arc::downgrade(&state); + let mut blocker = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::Block); + let pending_record = Record::new(110, 256, 1); + let mut cancelled_record = Record::new(111, 64, 1); + let (batch, finalizer) = BatchNotifier::new_with_receiver(); + cancelled_record.add_batch_notifier(batch); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + + owner.send(pending_record.clone(), None).await.unwrap(); + let send = tokio::spawn(async move { blocker.send(cancelled_record, None).await }); + sleep(Duration::from_millis(20)).await; + assert!( + !send.is_finished(), + "block send should be waiting for capacity" + ); + send.abort(); + assert!(send.await.unwrap_err().is_cancelled()); + assert_eq!( + timeout(Duration::from_secs(1), finalizer).await.unwrap(), + BatchStatus::Errored, + "cancelling an unowned send must nack it for source redelivery" + ); + + filesystem.restore_data_writes(); + timeout(Duration::from_secs(2), async { + while usage.snapshot().received_event_count < 1 { + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("writer-owned record should complete in the background"); + assert_eq!(reader.next().await.unwrap(), Some(pending_record)); + assert!( + timeout(Duration::from_millis(50), reader.next()) + .await + .is_err(), + "the cancelled unowned record must not be written later" + ); + + drop(owner); + drop(reader); + timeout(Duration::from_millis(100), async { + while weak.upgrade().is_some() { + tokio::task::yield_now().await; + } + }) + .await + .expect("cancelled block send must not retain the disk sender or writer"); +} + +#[tokio::test] +async fn terminal_capacity_retry_wakes_waiters_and_stays_failed() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut owner, reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match owner.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let mut blocker = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::Block); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + owner.send(Record::new(112, 256, 1), None).await.unwrap(); + + let waiting = tokio::spawn(async move { blocker.send(Record::new(113, 64, 1), None).await }); + sleep(Duration::from_millis(20)).await; + assert!(!waiting.is_finished(), "block send should await capacity"); + #[cfg(unix)] + filesystem.fail_data_writes_after_raw_os_error(0, libc::EACCES); + #[cfg(not(unix))] + filesystem.fail_data_writes_after(0, io::ErrorKind::PermissionDenied); + assert_terminal_permission_denied( + timeout(Duration::from_secs(1), waiting) + .await + .expect("terminal retry must wake the waiter") + .unwrap() + .expect_err("terminal retry must fail the waiter"), + ); + + let mut rejected = Record::new(114, 64, 1); + let (batch, finalizer) = BatchNotifier::new_with_receiver(); + rejected.add_batch_notifier(batch); + assert_terminal_permission_denied( + owner + .send(rejected, None) + .await + .expect_err("terminal retry state must reject subsequent sends"), + ); + assert_eq!( + timeout(Duration::from_secs(1), finalizer) + .await + .expect("terminal send finalizer should resolve"), + BatchStatus::Errored, + "terminal sends must nack finalizers for source redelivery" + ); + assert_terminal_permission_denied( + owner + .flush() + .await + .expect_err("terminal retry state must reject flushes"), + ); + drop(owner); + drop(reader); +} + +#[tokio::test] +async fn terminal_cancellation_retry_is_not_overwritten_by_queued_ready_retry() { + let filesystem = TestFilesystem::default(); + filesystem.set_max_write_size(Some(5)); + let (mut sender, _reader, _) = + build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; + let state = match sender.get_base_ref() { + SenderAdapter::DiskV2Test(state) => Arc::clone(state), + _ => unreachable!("test disk sender expected"), + }; + let mut record = Record::new(114, 256, 1); + let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); + record.add_batch_notifier(batch); + filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); + timeout(Duration::from_secs(1), sender.send(record, None)) + .await + .expect("drop-newest should accept the writer-owned record") + .unwrap(); + + // Let the ordinary weak retry observe StorageFull and schedule its later retry. + sleep(Duration::from_millis(150)).await; + filesystem.fail_data_writes_after(0, io::ErrorKind::PermissionDenied); + state.start_cancellation_retry_for_test(); + assert_eq!( + timeout(Duration::from_secs(1), &mut finalizer) + .await + .expect("terminal classification should resolve the owned record once"), + BatchStatus::Errored + ); + + // The queued ordinary driver would get Ready from the restored filesystem if it did not + // recheck the terminal state while holding the writer mutex. + filesystem.restore_data_writes(); + sleep(Duration::from_millis(250)).await; + assert!( + sender.send(Record::new(115, 64, 1), None).await.is_err(), + "a stale Ready retry must not overwrite Failed" + ); +} + +#[tokio::test] +async fn cancelled_production_send_from_ready_retains_owner_until_syscall_is_classified() { + let directory = TempDir::with_prefix("vector-buffer-adapter-cancel").unwrap(); + let data_dir = directory.path().to_path_buf(); + let StalledWrites { filesystem, gate } = ProductionFilesystem::with_stalled_writes(); + let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); + let config = DiskBufferConfigBuilder::from_path(&data_dir) + .write_buffer_size(16) + .filesystem(filesystem.clone()) + .build() + .unwrap(); + let (writer, reader, _) = Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + let mut sender = BufferSender::new(SenderAdapter::from(writer), WhenFull::Block); + let weak = match sender.get_base_ref() { + SenderAdapter::DiskV2(state) => Arc::downgrade(state), + _ => unreachable!("production disk sender expected"), + }; + let mut record = Record::new(115, 256, 1); + let expected = record.clone(); + let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); + record.add_batch_notifier(batch); + + let mut send = Box::pin(sender.send(record, None)); + tokio::select! { + result = &mut send => panic!("gated send unexpectedly completed: {result:?}"), + () = gate.wait_until_started() => {} + } + drop(send); + // Cancellation registers its retained driver before a normal starter can observe Retrying. + match sender.get_base_ref() { + SenderAdapter::DiskV2(state) => state.start_normal_capacity_retry_for_test(), + _ => unreachable!("production disk sender expected"), + } + drop(sender); + drop(reader); + assert!( + weak.upgrade().is_some(), + "the cancellation retry must retain the sender while its syscall is gated" + ); + assert!( + timeout(Duration::from_millis(20), &mut finalizer) + .await + .is_err(), + "writer-owned finalizers must remain unresolved while the syscall is pending" + ); + + gate.release(); + assert_eq!( + timeout(Duration::from_secs(2), &mut finalizer) + .await + .expect("the recovered write should resolve its finalizer"), + BatchStatus::Delivered + ); + timeout(Duration::from_millis(100), async { + while weak.upgrade().is_some() { + tokio::task::yield_now().await; + } + }) + .await + .expect("the retry must release the sender after classifying the syscall"); + + let config = DiskBufferConfigBuilder::from_path(&data_dir) + .write_buffer_size(16) + .filesystem(filesystem) + .build() + .unwrap(); + let (recovered_writer, mut recovered_reader, _) = + Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + drop(recovered_writer); + assert_eq!( + timeout(Duration::from_secs(2), recovered_reader.next()) + .await + .expect("the recovered record should be readable") + .unwrap(), + Some(expected) + ); + assert_eq!( + timeout(Duration::from_secs(2), recovered_reader.next()) + .await + .expect("the recovered buffer should reach EOF") + .unwrap(), + None + ); +} + +#[tokio::test] +async fn cancelled_production_flush_while_retrying_retains_owner_until_classified() { + let directory = TempDir::with_prefix("vector-buffer-adapter-retrying-cancel").unwrap(); + let data_dir = directory.path().to_path_buf(); + let StalledWrites { filesystem, gate } = ProductionFilesystem::with_stalled_writes(); + let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); + let config = DiskBufferConfigBuilder::from_path(&data_dir) + .write_buffer_size(1024) + .filesystem(filesystem.clone()) + .build() + .unwrap(); + let (writer, reader, _) = Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + let mut sender = BufferSender::new(SenderAdapter::from(writer), WhenFull::Block); + let weak = match sender.get_base_ref() { + SenderAdapter::DiskV2(state) => Arc::downgrade(state), + _ => unreachable!("production disk sender expected"), + }; + let mut record = Record::new(119, 256, 1); + let expected = record.clone(); + let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); + record.add_batch_notifier(batch); + sender.send(record, None).await.unwrap(); + match sender.get_base_ref() { + SenderAdapter::DiskV2(state) => state.start_normal_capacity_retry_for_test(), + _ => unreachable!("production disk sender expected"), + } + + let mut flush = Box::pin(sender.flush()); + tokio::select! { + result = &mut flush => panic!("gated flush unexpectedly completed: {result:?}"), + () = gate.wait_until_started() => {} + } + drop(flush); + drop(sender); + drop(reader); + assert!( + weak.upgrade().is_some(), + "the cancellation classifier must retain the sender while its syscall is gated" + ); + + gate.release(); + assert_eq!( + timeout(Duration::from_secs(2), &mut finalizer) + .await + .expect("the accepted record should resolve its finalizer"), + BatchStatus::Delivered + ); + timeout(Duration::from_secs(2), async { + while weak.upgrade().is_some() { + tokio::task::yield_now().await; + } + }) + .await + .expect("the classifier must release the sender after classifying the syscall"); + + let config = DiskBufferConfigBuilder::from_path(&data_dir) + .write_buffer_size(1024) + .filesystem(filesystem) + .build() + .unwrap(); + let (recovered_writer, mut recovered_reader, _) = + Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + drop(recovered_writer); + assert_eq!( + timeout(Duration::from_secs(2), recovered_reader.next()) + .await + .expect("the recovered record should be readable") + .unwrap(), + Some(expected) + ); + assert_eq!( + timeout(Duration::from_secs(2), recovered_reader.next()) + .await + .expect("the recovered buffer should reach EOF") + .unwrap(), + None + ); +} + +#[tokio::test] +async fn cancelled_production_flush_recovers_accepted_record() { + let directory = TempDir::with_prefix("vector-buffer-adapter-flush-cancel").unwrap(); + let StalledWrites { filesystem, gate } = ProductionFilesystem::with_stalled_writes(); + let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); + let config = DiskBufferConfigBuilder::from_path(directory.path()) + .write_buffer_size(1024) + .filesystem(filesystem) + .build() + .unwrap(); + let (writer, mut reader, _) = Buffer::from_config_inner(config, BufferUsageHandle::noop()) + .await + .unwrap(); + let mut sender = BufferSender::new(SenderAdapter::from(writer), WhenFull::Block); + let record = Record::new(118, 256, 1); + + sender.send(record.clone(), None).await.unwrap(); + let mut flush = Box::pin(sender.flush()); + tokio::select! { + result = &mut flush => panic!("gated flush unexpectedly completed: {result:?}"), + () = gate.wait_until_started() => {} + } + drop(flush); + gate.release(); + + assert_eq!( + timeout(Duration::from_secs(2), reader.next()) + .await + .expect("cancelled flush should be recovered in the background") + .unwrap(), + Some(record) + ); +} diff --git a/lib/vector-buffers/src/topology/channel/mod.rs b/lib/vector-buffers/src/topology/channel/mod.rs index ab8e0fa62e5ce..5a8c3b2aec98a 100644 --- a/lib/vector-buffers/src/topology/channel/mod.rs +++ b/lib/vector-buffers/src/topology/channel/mod.rs @@ -1,7 +1,12 @@ +mod disk_v2_sender; mod limited_queue; mod receiver; mod sender; +#[cfg(test)] +pub(crate) use disk_v2_sender::CapacityBlockedHook; + +pub use disk_v2_sender::DiskV2Sender; pub use limited_queue::{ BufferChannelKind, ChannelMetricMetadata, DEFAULT_EWMA_HALF_LIFE_SECONDS, LimitedReceiver, LimitedSender, SendError, limited, @@ -9,5 +14,7 @@ pub use limited_queue::{ pub use receiver::*; pub use sender::*; +#[cfg(test)] +mod disk_v2_sender_tests; #[cfg(test)] mod tests; diff --git a/lib/vector-buffers/src/topology/channel/sender.rs b/lib/vector-buffers/src/topology/channel/sender.rs index a26259f7e9e1e..fbf4775ac65a2 100644 --- a/lib/vector-buffers/src/topology/channel/sender.rs +++ b/lib/vector-buffers/src/topology/channel/sender.rs @@ -1,593 +1,24 @@ // Derivative's Debug impl generates 'let _ = field.fmt(f)' which triggers this lint. #![allow(clippy::let_underscore_must_use)] -use std::{ - fmt, io, - sync::{Arc, Mutex as StdMutex}, - time::{Duration, Instant}, -}; +use std::{sync::Arc, time::Instant}; use async_recursion::async_recursion; use derivative::Derivative; -use futures::future::BoxFuture; -use tokio::{ - sync::{Mutex, Notify}, - time::sleep, -}; use tracing::Span; -use vector_common::finalization::EventStatus; use vector_common::internal_event::{InternalEventHandle, Registered, register}; -use super::limited_queue::LimitedSender; +use super::{disk_v2_sender::DiskV2Sender, limited_queue::LimitedSender}; use crate::{ BufferInstrumentation, Bufferable, WhenFull, buffer_usage_data::BufferUsageHandle, internal_events::BufferSendDuration, - variants::disk_v2::{ - self, CapacityProgress, Filesystem, ProductionFilesystem, TryWriteOutcome, - }, + variants::disk_v2::{self, ProductionFilesystem, TryWriteOutcome}, }; #[cfg(test)] use crate::variants::disk_v2::tests::model::filesystem::TestFilesystem; -const CAPACITY_RETRY_INITIAL: Duration = Duration::from_millis(100); -const CAPACITY_RETRY_MAX: Duration = Duration::from_secs(5); - -#[derive(Debug)] -enum CapacityState { - Ready, - Retrying, - Failed(TerminalError), -} - -#[derive(Clone, Debug)] -struct TerminalError { - kind: io::ErrorKind, - raw_os_error: Option, - message: String, -} - -impl From<&io::Error> for TerminalError { - fn from(error: &io::Error) -> Self { - Self { - kind: error.kind(), - raw_os_error: error.raw_os_error(), - message: error.to_string(), - } - } -} - -impl TerminalError { - fn to_io_error(&self) -> io::Error { - // io::Error can preserve either a raw OS error or a custom message, but not both. - // Prefer the raw error when present so callers retain its platform classification. - match self.raw_os_error { - Some(raw_os_error) => io::Error::from_raw_os_error(raw_os_error), - None => io::Error::new(self.kind, self.message.clone()), - } - } -} - -#[cfg(test)] -#[derive(Debug, Default)] -pub(crate) struct CapacityBlockedHook { - pub(crate) detected: Notify, - pub(crate) resume: Notify, -} - -struct UnownedFinalizers(Option); - -impl UnownedFinalizers { - fn take(item: &mut T) -> Self { - Self(Some(item.take_finalizer_groups())) - } - - fn restore(mut self, item: &mut T) { - item.merge_finalizer_groups(self.0.take().expect("finalizers must exist")); - } -} - -impl Drop for UnownedFinalizers { - fn drop(&mut self) { - if let Some(finalizers) = self.0.as_mut() { - finalizers.update_status(EventStatus::Errored); - } - } -} - -trait SenderFilesystem: Filesystem + fmt::Debug + Clone + Sized + 'static -where - T: Bufferable, - Self::File: Unpin, -{ - fn try_write_record( - writer: &mut disk_v2::BufferWriter, - item: T, - ) -> BoxFuture<'_, Result, disk_v2::WriterError>>; - - fn try_flush( - writer: &mut disk_v2::BufferWriter, - ) -> BoxFuture<'_, io::Result>; - - fn retry_capacity( - writer: &mut disk_v2::BufferWriter, - ) -> BoxFuture<'_, io::Result>; -} - -macro_rules! impl_sender_filesystem { - ($filesystem:ty) => { - impl SenderFilesystem for $filesystem { - fn try_write_record( - writer: &mut disk_v2::BufferWriter, - item: T, - ) -> BoxFuture<'_, Result, disk_v2::WriterError>> { - Box::pin(writer.try_write_record(item)) - } - - fn try_flush( - writer: &mut disk_v2::BufferWriter, - ) -> BoxFuture<'_, io::Result> { - Box::pin(writer.try_flush()) - } - - fn retry_capacity( - writer: &mut disk_v2::BufferWriter, - ) -> BoxFuture<'_, io::Result> { - Box::pin(writer.retry_capacity()) - } - } - }; -} - -impl_sender_filesystem!(ProductionFilesystem); -#[cfg(test)] -impl_sender_filesystem!(TestFilesystem); - -#[derive(Debug)] -pub struct DiskV2Sender -where - T: Bufferable, - FS: Filesystem, - FS::File: fmt::Debug + Unpin, -{ - writer: Mutex>, - usage: BufferUsageHandle, - capacity_state: StdMutex, - capacity_notify: Notify, - #[cfg(test)] - capacity_blocked_hook: StdMutex>>, -} - -struct WriterProgressOnCancel<'a, T, FS> -where - T: Bufferable, - FS: SenderFilesystem, - FS::File: fmt::Debug + Unpin, -{ - state: &'a Arc>, - armed: bool, -} - -impl<'a, T, FS> WriterProgressOnCancel<'a, T, FS> -where - T: Bufferable, - FS: SenderFilesystem, - FS::File: fmt::Debug + Unpin, -{ - fn new(state: &'a Arc>) -> Self { - Self { state, armed: true } - } - - fn disarm(&mut self) { - self.armed = false; - } -} - -impl Drop for WriterProgressOnCancel<'_, T, FS> -where - T: Bufferable, - FS: SenderFilesystem, - FS::File: fmt::Debug + Unpin, -{ - fn drop(&mut self) { - if self.armed { - self.state.start_cancellation_retry(); - } - } -} - -#[allow(private_bounds)] -impl DiskV2Sender -where - T: Bufferable, - FS: SenderFilesystem, - FS::File: fmt::Debug + Unpin, -{ - fn new(writer: disk_v2::BufferWriter) -> Self { - let usage = writer.usage_handle(); - Self { - writer: Mutex::new(writer), - usage, - capacity_state: StdMutex::new(CapacityState::Ready), - capacity_notify: Notify::new(), - #[cfg(test)] - capacity_blocked_hook: StdMutex::new(None), - } - } - - #[cfg(test)] - async fn pause_after_capacity_blocked_for_test(&self) { - let hook = self.capacity_blocked_hook.lock().expect("poisoned").clone(); - if let Some(hook) = hook { - hook.detected.notify_waiters(); - hook.resume.notified().await; - } - } - - fn is_capacity_blocked(&self) -> bool { - matches!( - *self.capacity_state.lock().expect("poisoned"), - CapacityState::Retrying - ) - } - - fn start_capacity_retry(self: &Arc) { - let should_start = { - let mut capacity_state = self.capacity_state.lock().expect("poisoned"); - match *capacity_state { - CapacityState::Ready => { - *capacity_state = CapacityState::Retrying; - true - } - CapacityState::Retrying | CapacityState::Failed(_) => false, - } - }; - if should_start { - self.capacity_notify.notify_waiters(); - self.spawn_capacity_retry_driver(None); - } - } - - fn start_cancellation_retry(self: &Arc) { - let (retained_driver, classifier) = { - let mut capacity_state = self.capacity_state.lock().expect("poisoned"); - match *capacity_state { - CapacityState::Ready => { - *capacity_state = CapacityState::Retrying; - (Some(Arc::clone(self)), None) - } - CapacityState::Retrying => (None, Some(Arc::clone(self))), - CapacityState::Failed(_) => return, - } - }; - if let Some(state) = retained_driver { - self.capacity_notify.notify_waiters(); - self.spawn_capacity_retry_driver(Some(state)); - } else if let Some(state) = classifier { - Self::classify_cancelled_retry(state); - } - } - - fn classify_cancelled_retry(state: Arc) { - tokio::spawn(async move { - let result = { - let mut writer = state.writer.lock().await; - state.retry_capacity_once(&mut writer).await - }; - - let _ = result; - }); - } - - async fn retry_capacity_once( - &self, - writer: &mut disk_v2::BufferWriter, - ) -> Option> { - if !matches!( - *self.capacity_state.lock().expect("poisoned"), - CapacityState::Retrying - ) { - return None; - } - - let result = FS::retry_capacity(writer).await; - match &result { - Ok(CapacityProgress::Ready) if !writer.is_capacity_blocked() => { - let mut capacity_state = self.capacity_state.lock().expect("poisoned"); - if matches!(*capacity_state, CapacityState::Retrying) { - *capacity_state = CapacityState::Ready; - self.capacity_notify.notify_waiters(); - } - } - Err(error) => { - writer.fail_pending_write(); - error!(%error, "Disk buffer capacity retry failed."); - let mut capacity_state = self.capacity_state.lock().expect("poisoned"); - if matches!(*capacity_state, CapacityState::Retrying) { - *capacity_state = CapacityState::Failed(TerminalError::from(error)); - self.capacity_notify.notify_waiters(); - } - } - Ok( - CapacityProgress::Blocked - | CapacityProgress::WaitingForReader - | CapacityProgress::Ready, - ) => {} - } - Some(result) - } - - fn spawn_capacity_retry_driver(self: &Arc, mut retained_state: Option>) { - let weak_state = Arc::downgrade(self); - tokio::spawn(async move { - let mut delay = CAPACITY_RETRY_INITIAL; - loop { - let Some(state) = retained_state.clone().or_else(|| weak_state.upgrade()) else { - return; - }; - let reader_progress = { - let writer = state.writer.lock().await; - writer.reader_progress_waiter() - }; - drop(state); - tokio::select! { - () = sleep(delay) => {} - () = reader_progress => {} - } - let Some(state) = retained_state.clone().or_else(|| weak_state.upgrade()) else { - return; - }; - let result = { - let mut writer = state.writer.lock().await; - state.retry_capacity_once(&mut writer).await - }; - let Some(result) = result else { - return; - }; - // A cancellation can leave a production syscall running after its caller drops. - // Keep its sender alive through the first retry result, then return to weak ownership. - retained_state = None; - match result { - Ok(CapacityProgress::Ready) => { - if state.is_capacity_blocked() { - continue; - } - return; - } - Ok(CapacityProgress::Blocked) => { - delay = delay.saturating_mul(2).min(CAPACITY_RETRY_MAX); - } - Ok(CapacityProgress::WaitingForReader) => {} - Err(_) => { - return; - } - } - } - }); - } - - async fn wait_for_capacity(&self) -> crate::Result<()> { - let notified = self.capacity_notify.notified(); - tokio::pin!(notified); - loop { - notified.as_mut().enable(); - match &*self.capacity_state.lock().expect("poisoned") { - CapacityState::Ready => return Ok(()), - CapacityState::Failed(error) => { - return Err(error.to_io_error().into()); - } - CapacityState::Retrying => {} - } - notified.as_mut().await; - notified.set(self.capacity_notify.notified()); - } - } - - async fn try_send_record(self: &Arc, item: T) -> crate::Result> { - let mut item = item; - let finalizers = UnownedFinalizers::take(&mut item); - if let CapacityState::Failed(error) = &*self.capacity_state.lock().expect("poisoned") { - return Err(error.to_io_error().into()); - } - finalizers.restore(&mut item); - - let pre_count = item.event_count() as u64; - let pre_size = item.size_of() as u64; - let Some(mut item) = item.filter_unencodable() else { - self.usage - .increment_received_event_count_and_byte_size(pre_count, pre_size); - self.usage - .increment_dropped_event_count_and_byte_size(pre_count, pre_size, false); - return Ok(TryWriteOutcome::Dropped); - }; - if item.event_count() as u64 != pre_count { - let dropped_events = pre_count - item.event_count() as u64; - let dropped_bytes = pre_size.saturating_sub(item.size_of() as u64); - self.usage - .increment_received_event_count_and_byte_size(dropped_events, dropped_bytes); - self.usage.increment_dropped_event_count_and_byte_size( - dropped_events, - dropped_bytes, - false, - ); - } - - let finalizers = UnownedFinalizers::take(&mut item); - let notified = self.capacity_notify.notified(); - tokio::pin!(notified); - let mut writer = loop { - notified.as_mut().enable(); - match &*self.capacity_state.lock().expect("poisoned") { - CapacityState::Ready => {} - CapacityState::Retrying => { - finalizers.restore(&mut item); - return Ok(TryWriteOutcome::Full(item)); - } - CapacityState::Failed(error) => { - return Err(error.to_io_error().into()); - } - } - - tokio::select! { - biased; - () = notified.as_mut() => { - notified.set(self.capacity_notify.notified()); - } - writer = self.writer.lock() => { - match &*self.capacity_state.lock().expect("poisoned") { - CapacityState::Ready => break writer, - CapacityState::Retrying => { - drop(writer); - finalizers.restore(&mut item); - return Ok(TryWriteOutcome::Full(item)); - } - CapacityState::Failed(error) => { - return Err(error.to_io_error().into()); - } - } - } - } - }; - if writer.is_capacity_blocked() { - #[cfg(test)] - self.pause_after_capacity_blocked_for_test().await; - self.start_capacity_retry(); - drop(writer); - finalizers.restore(&mut item); - return Ok(TryWriteOutcome::Full(item)); - } - - finalizers.restore(&mut item); - let mut progress_on_cancel = WriterProgressOnCancel::new(self); - let result = FS::try_write_record(&mut writer, item).await; - progress_on_cancel.disarm(); - let outcome = result.inspect_err(|_error| { - error!("Disk buffer writer has encountered an unrecoverable error."); - })?; - let blocked = writer.is_capacity_blocked(); - if blocked { - #[cfg(test)] - self.pause_after_capacity_blocked_for_test().await; - self.start_capacity_retry(); - } - drop(writer); - Ok(outcome) - } - - async fn send_record(self: &Arc, item: T) -> crate::Result> { - let mut item = item; - loop { - match self.try_send_record(item).await? { - TryWriteOutcome::Written => return Ok(TryWriteOutcome::Written), - TryWriteOutcome::Dropped => return Ok(TryWriteOutcome::Dropped), - TryWriteOutcome::Pending => { - self.wait_for_capacity().await?; - return Ok(TryWriteOutcome::Written); - } - TryWriteOutcome::Full(mut returned) => { - let finalizers = UnownedFinalizers::take(&mut returned); - item = returned; - if self.is_capacity_blocked() { - self.wait_for_capacity().await?; - } else { - let writer = self.writer.lock().await; - let reader_progress = writer.reader_progress_waiter(); - drop(writer); - reader_progress.await; - } - finalizers.restore(&mut item); - } - } - } - } - - async fn flush(self: &Arc, block: bool) -> crate::Result<()> { - loop { - let retrying = { - match &*self.capacity_state.lock().expect("poisoned") { - CapacityState::Ready => false, - CapacityState::Retrying => true, - CapacityState::Failed(error) => { - return Err(error.to_io_error().into()); - } - } - }; - if retrying { - if !block { - return Ok(()); - } - self.wait_for_capacity().await?; - continue; - } - - let mut writer = self.writer.lock().await; - let retrying = { - match &*self.capacity_state.lock().expect("poisoned") { - CapacityState::Ready => false, - CapacityState::Retrying => true, - CapacityState::Failed(error) => { - return Err(error.to_io_error().into()); - } - } - }; - if retrying { - drop(writer); - if !block { - return Ok(()); - } - self.wait_for_capacity().await?; - continue; - } - - // This guard only covers filesystem I/O. Cancelling while waiting for capacity must not - // start a second capacity retry driver. - let mut progress_on_cancel = WriterProgressOnCancel::new(self); - let result = FS::try_flush(&mut writer).await; - progress_on_cancel.disarm(); - let progress = result.map_err(|error| -> Box { - error!("Disk buffer writer has encountered an unrecoverable error."); - error.into() - })?; - - if matches!(progress, CapacityProgress::Blocked) { - // Publish Retrying before releasing the writer, so another flush cannot race in - // and issue another filesystem call before observing the capacity episode. - self.start_capacity_retry(); - } - drop(writer); - - if matches!(progress, CapacityProgress::Blocked) && block { - self.wait_for_capacity().await?; - continue; - } - return Ok(()); - } - } - - #[cfg(test)] - pub(crate) fn start_normal_capacity_retry_for_test(self: &Arc) { - self.start_capacity_retry(); - } - - #[cfg(test)] - pub(crate) fn start_cancellation_retry_for_test(self: &Arc) { - self.start_cancellation_retry(); - } - - #[cfg(test)] - pub(crate) fn set_capacity_blocked_hook(&self, hook: Arc) { - *self.capacity_blocked_hook.lock().expect("poisoned") = Some(hook); - } -} - -#[cfg(test)] -impl DiskV2Sender { - pub(crate) async fn next_writer_data_file_path(&self) -> std::path::PathBuf { - self.writer.lock().await.next_writer_data_file_path() - } -} - /// Adapter for papering over various sender backends. #[derive(Clone, Debug)] pub enum SenderAdapter { diff --git a/lib/vector-buffers/src/topology/channel/tests.rs b/lib/vector-buffers/src/topology/channel/tests.rs index 498a345305b2a..7bee7e475c2b1 100644 --- a/lib/vector-buffers/src/topology/channel/tests.rs +++ b/lib/vector-buffers/src/topology/channel/tests.rs @@ -1,47 +1,18 @@ use std::{ - io, sync::Arc, time::{Duration, Instant}, }; -use temp_dir::TempDir; -use tokio::{ - pin, - sync::Barrier, - time::{advance, sleep, timeout}, -}; -use vector_common::finalization::{AddBatchNotifier, BatchNotifier, BatchStatus}; +use tokio::{pin, sync::Barrier, time::sleep}; use crate::{ Bufferable, WhenFull, - buffer_usage_data::BufferUsageHandle, topology::{ - channel::{BufferReceiver, BufferSender, CapacityBlockedHook, SenderAdapter}, + channel::{BufferReceiver, BufferSender}, test_util::{assert_current_send_capacity, build_buffer}, }, - variants::disk_v2::{ - Buffer, DiskBufferConfigBuilder, ProductionFilesystem, StalledWrites, TestWriteGate, - tests::model::{filesystem::TestFilesystem, record::Record}, - }, }; -struct WriteGateCleanup(Arc); - -impl Drop for WriteGateCleanup { - fn drop(&mut self) { - self.0.release(); - } -} - -fn assert_terminal_permission_denied(error: vector_common::Error) { - let error = error - .downcast::() - .expect("terminal error should retain its I/O classification"); - assert_eq!(error.kind(), io::ErrorKind::PermissionDenied); - #[cfg(unix)] - assert_eq!(error.raw_os_error(), Some(libc::EACCES)); -} - async fn assert_send_ok_with_capacities( sender: &mut BufferSender, value: impl Into, @@ -299,667 +270,3 @@ async fn test_buffer_metrics_overflow_block() { assert_eq!(4, snapshot.sent_event_count); assert_eq!(0, snapshot.dropped_event_count_intentional); } - -// Physical disk exhaustion acts like a full buffer for records not yet owned by disk_v2. These -// tests verify each policy while partially written records recover exactly once. -async fn build_policy_disk( - filesystem: TestFilesystem, - write_buffer_size: usize, - rotating: bool, - when_full: WhenFull, -) -> ( - BufferSender, - crate::variants::disk_v2::BufferReader, - BufferUsageHandle, -) { - let directory = - std::env::temp_dir().join(format!("vector-policy-disk-{}", rand::random::())); - let mut builder = DiskBufferConfigBuilder::from_path(directory) - .write_buffer_size(write_buffer_size) - .filesystem(filesystem); - if rotating { - builder = builder.max_data_file_size(256).max_record_size(256); - } - let usage = BufferUsageHandle::noop(); - let (writer, reader, _) = Buffer::from_config_inner(builder.build().unwrap(), usage.clone()) - .await - .unwrap(); - ( - BufferSender::new(SenderAdapter::from(writer), when_full), - reader, - usage, - ) -} - -#[tokio::test] -async fn disk_capacity_block_retries_owned_record() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut sender, mut reader, _) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::Block).await; - let record = Record::new(100, 256, 1); - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - - let mut send = Box::pin(sender.send(record.clone(), None)); - assert!( - timeout(Duration::from_millis(20), &mut send).await.is_err(), - "block policy must backpressure while the owned record retries" - ); - filesystem.restore_data_writes(); - timeout(Duration::from_secs(2), send) - .await - .expect("block send should recover") - .unwrap(); - sender.flush().await.unwrap(); - assert_eq!(reader.next().await.unwrap(), Some(record)); -} - -#[tokio::test] -async fn disk_capacity_drop_newest_is_prompt_and_counted_once() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut sender, mut reader, usage) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - let outer_usage = BufferUsageHandle::noop(); - sender.with_usage_instrumentation(outer_usage.clone()); - let owned = Record::new(101, 256, 1); - let dropped = Record::new(102, 64, 1); - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - - timeout(Duration::from_secs(1), sender.send(owned.clone(), None)) - .await - .expect("the owned drop-newest record should be accepted") - .unwrap(); - timeout(Duration::from_millis(20), sender.send(dropped, None)) - .await - .expect("drop-newest send must not wait for the retry driver") - .unwrap(); - assert_eq!(usage.snapshot().dropped_event_count_intentional, 1); - assert_eq!(outer_usage.snapshot().dropped_event_count_intentional, 0); - - filesystem.restore_data_writes(); - timeout(Duration::from_secs(2), async { - while usage.snapshot().received_event_count < 2 { - sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("owned record should complete in the background"); - assert_eq!(reader.next().await.unwrap(), Some(owned)); - let snapshot = usage.snapshot(); - assert_eq!(snapshot.received_event_count, 2); - assert_eq!(snapshot.dropped_event_count_intentional, 1); -} - -#[tokio::test(start_paused = true)] -async fn disk_capacity_drop_newest_flush_skips_writer_while_retrying() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut sender, _reader, _) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - - sender.send(Record::new(120, 256, 1), None).await.unwrap(); - tokio::task::yield_now().await; - let write_attempts = filesystem.data_write_attempts(); - - for id in 121..124 { - sender.send(Record::new(id, 64, 1), None).await.unwrap(); - sender.flush().await.unwrap(); - } - assert_eq!(filesystem.data_write_attempts(), write_attempts); - - // The retry driver is the only path allowed to make another filesystem attempt, and it - // retains its exponential-backoff schedule while sends and flushes are dropped. - advance(Duration::from_millis(99)).await; - tokio::task::yield_now().await; - assert_eq!(filesystem.data_write_attempts(), write_attempts); - advance(Duration::from_millis(1)).await; - tokio::task::yield_now().await; - assert_eq!(filesystem.data_write_attempts(), write_attempts + 1); -} - -#[tokio::test] -async fn disk_capacity_send_publishes_retrying_before_releasing_writer() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut sender, _reader, _) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - let state = match sender.get_base_ref() { - SenderAdapter::DiskV2Test(state) => Arc::clone(state), - _ => unreachable!("test disk sender expected"), - }; - let hook = Arc::new(CapacityBlockedHook::default()); - state.set_capacity_blocked_hook(Arc::clone(&hook)); - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - - let detected = hook.detected.notified(); - tokio::pin!(detected); - detected.as_mut().enable(); - let send = tokio::spawn(async move { sender.send(Record::new(127, 256, 1), None).await }); - detected.await; - let write_attempts = filesystem.data_write_attempts(); - - let mut flusher = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::DropNewest); - let flush = tokio::spawn(async move { flusher.flush().await }); - tokio::task::yield_now().await; - hook.resume.notify_waiters(); - - send.await.unwrap().unwrap(); - flush.await.unwrap().unwrap(); - assert_eq!(filesystem.data_write_attempts(), write_attempts); -} - -#[tokio::test] -async fn disk_capacity_overflow_routes_from_cloned_sender() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut sender, mut base_reader, base_usage) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - let (overflow_sender, mut overflow_reader, overflow_usage) = - build_policy_disk(TestFilesystem::default(), 1024, false, WhenFull::Block).await; - sender.switch_to_overflow(overflow_sender); - let mut clone = sender.clone(); - let owned = Record::new(103, 256, 1); - let overflowed = Record::new(104, 64, 1); - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - - timeout(Duration::from_secs(1), sender.send(owned.clone(), None)) - .await - .expect("the owned overflow record should be accepted by the base") - .unwrap(); - timeout( - Duration::from_millis(20), - clone.send(overflowed.clone(), None), - ) - .await - .expect("cloned overflow sender must not wait for the base retry") - .unwrap(); - clone.flush().await.unwrap(); - timeout(Duration::from_secs(1), async { - while overflow_usage.snapshot().received_event_count < 1 { - sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("overflowed record should be flushed"); - assert_eq!(overflow_reader.next().await.unwrap(), Some(overflowed)); - - filesystem.restore_data_writes(); - timeout(Duration::from_secs(2), async { - while base_usage.snapshot().received_event_count < 1 { - sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("base owned record should recover"); - assert_eq!(base_reader.next().await.unwrap(), Some(owned)); -} - -#[tokio::test(start_paused = true)] -async fn disk_capacity_overflow_flushes_base_without_retrying_writer() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut sender, _base_reader, _) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - let (overflow_sender, mut overflow_reader, overflow_usage) = - build_policy_disk(TestFilesystem::default(), 1024, false, WhenFull::Block).await; - sender.switch_to_overflow(overflow_sender); - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - - sender.send(Record::new(124, 256, 1), None).await.unwrap(); - tokio::task::yield_now().await; - let write_attempts = filesystem.data_write_attempts(); - let overflowed = Record::new(125, 64, 1); - sender.send(overflowed.clone(), None).await.unwrap(); - sender.flush().await.unwrap(); - - assert_eq!(filesystem.data_write_attempts(), write_attempts); - assert_eq!(overflow_usage.snapshot().received_event_count, 1); - assert_eq!(overflow_reader.next().await.unwrap(), Some(overflowed)); -} - -#[tokio::test(start_paused = true)] -async fn disk_capacity_blocking_flush_waits_then_recovers() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut owner, mut reader, _) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - let state = match owner.get_base_ref() { - SenderAdapter::DiskV2Test(state) => Arc::clone(state), - _ => unreachable!("test disk sender expected"), - }; - let mut blocker = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::Block); - let record = Record::new(126, 256, 1); - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - owner.send(record.clone(), None).await.unwrap(); - tokio::task::yield_now().await; - let write_attempts = filesystem.data_write_attempts(); - - let mut flush = Box::pin(blocker.flush()); - tokio::select! { - result = &mut flush => panic!("capacity-blocked flush unexpectedly completed: {result:?}"), - () = tokio::task::yield_now() => {} - } - assert_eq!(filesystem.data_write_attempts(), write_attempts); - - filesystem.restore_data_writes(); - advance(Duration::from_millis(100)).await; - flush.await.unwrap(); - assert_eq!(reader.next().await.unwrap(), Some(record)); -} - -#[tokio::test] -async fn disk_retry_waiting_for_reader_releases_sender_mutex() { - let filesystem = TestFilesystem::default(); - let (mut sender, _reader, _) = - build_policy_disk(filesystem.clone(), 1024, true, WhenFull::DropNewest).await; - for id in 105..107 { - sender.send(Record::new(id, 64, 1), None).await.unwrap(); - sender.flush().await.unwrap(); - } - let state = match sender.get_base_ref() { - SenderAdapter::DiskV2Test(state) => Arc::clone(state), - _ => unreachable!("test disk sender expected"), - }; - let next_path = state.next_writer_data_file_path().await; - filesystem.fail_data_file_open(io::ErrorKind::StorageFull); - sender.send(Record::new(107, 64, 1), None).await.unwrap(); - filesystem.restore_data_file_open(); - filesystem.create_data_file_with_data(&next_path, b"occupied"); - sleep(Duration::from_millis(150)).await; - - let mut clone = sender.clone(); - timeout( - Duration::from_millis(20), - clone.send(Record::new(108, 64, 1), None), - ) - .await - .expect("retry waiting for reader must not hold the sender mutex") - .unwrap(); -} - -#[tokio::test] -async fn ordinary_capacity_retry_does_not_retain_dropped_sender() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut sender, reader, _) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - let weak = match sender.get_base_ref() { - SenderAdapter::DiskV2Test(state) => Arc::downgrade(state), - _ => unreachable!("test disk sender expected"), - }; - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - sender.send(Record::new(109, 256, 1), None).await.unwrap(); - - drop(sender); - drop(reader); - timeout(Duration::from_millis(100), async { - while weak.upgrade().is_some() { - tokio::task::yield_now().await; - } - }) - .await - .expect("ordinary retry task must not retain the disk sender or writer"); -} - -#[tokio::test] -async fn cancelling_unowned_block_send_does_not_detach_or_duplicate_record() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut owner, mut reader, usage) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - let state = match owner.get_base_ref() { - SenderAdapter::DiskV2Test(state) => Arc::clone(state), - _ => unreachable!("test disk sender expected"), - }; - let weak = Arc::downgrade(&state); - let mut blocker = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::Block); - let pending_record = Record::new(110, 256, 1); - let mut cancelled_record = Record::new(111, 64, 1); - let (batch, finalizer) = BatchNotifier::new_with_receiver(); - cancelled_record.add_batch_notifier(batch); - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - - owner.send(pending_record.clone(), None).await.unwrap(); - let send = tokio::spawn(async move { blocker.send(cancelled_record, None).await }); - sleep(Duration::from_millis(20)).await; - assert!( - !send.is_finished(), - "block send should be waiting for capacity" - ); - send.abort(); - assert!(send.await.unwrap_err().is_cancelled()); - assert_eq!( - timeout(Duration::from_secs(1), finalizer).await.unwrap(), - BatchStatus::Errored, - "cancelling an unowned send must nack it for source redelivery" - ); - - filesystem.restore_data_writes(); - timeout(Duration::from_secs(2), async { - while usage.snapshot().received_event_count < 1 { - sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("writer-owned record should complete in the background"); - assert_eq!(reader.next().await.unwrap(), Some(pending_record)); - assert!( - timeout(Duration::from_millis(50), reader.next()) - .await - .is_err(), - "the cancelled unowned record must not be written later" - ); - - drop(owner); - drop(reader); - timeout(Duration::from_millis(100), async { - while weak.upgrade().is_some() { - tokio::task::yield_now().await; - } - }) - .await - .expect("cancelled block send must not retain the disk sender or writer"); -} - -#[tokio::test] -async fn terminal_capacity_retry_wakes_waiters_and_stays_failed() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut owner, reader, _) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - let state = match owner.get_base_ref() { - SenderAdapter::DiskV2Test(state) => Arc::clone(state), - _ => unreachable!("test disk sender expected"), - }; - let mut blocker = BufferSender::new(SenderAdapter::DiskV2Test(state), WhenFull::Block); - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - owner.send(Record::new(112, 256, 1), None).await.unwrap(); - - let waiting = tokio::spawn(async move { blocker.send(Record::new(113, 64, 1), None).await }); - sleep(Duration::from_millis(20)).await; - assert!(!waiting.is_finished(), "block send should await capacity"); - #[cfg(unix)] - filesystem.fail_data_writes_after_raw_os_error(0, libc::EACCES); - #[cfg(not(unix))] - filesystem.fail_data_writes_after(0, io::ErrorKind::PermissionDenied); - assert_terminal_permission_denied( - timeout(Duration::from_secs(1), waiting) - .await - .expect("terminal retry must wake the waiter") - .unwrap() - .expect_err("terminal retry must fail the waiter"), - ); - - let mut rejected = Record::new(114, 64, 1); - let (batch, finalizer) = BatchNotifier::new_with_receiver(); - rejected.add_batch_notifier(batch); - assert_terminal_permission_denied( - owner - .send(rejected, None) - .await - .expect_err("terminal retry state must reject subsequent sends"), - ); - assert_eq!( - timeout(Duration::from_secs(1), finalizer) - .await - .expect("terminal send finalizer should resolve"), - BatchStatus::Errored, - "terminal sends must nack finalizers for source redelivery" - ); - assert_terminal_permission_denied( - owner - .flush() - .await - .expect_err("terminal retry state must reject flushes"), - ); - drop(owner); - drop(reader); -} - -#[tokio::test] -async fn terminal_cancellation_retry_is_not_overwritten_by_queued_ready_retry() { - let filesystem = TestFilesystem::default(); - filesystem.set_max_write_size(Some(5)); - let (mut sender, _reader, _) = - build_policy_disk(filesystem.clone(), 16, false, WhenFull::DropNewest).await; - let state = match sender.get_base_ref() { - SenderAdapter::DiskV2Test(state) => Arc::clone(state), - _ => unreachable!("test disk sender expected"), - }; - let mut record = Record::new(114, 256, 1); - let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); - record.add_batch_notifier(batch); - filesystem.fail_data_writes_after(10, io::ErrorKind::StorageFull); - timeout(Duration::from_secs(1), sender.send(record, None)) - .await - .expect("drop-newest should accept the writer-owned record") - .unwrap(); - - // Let the ordinary weak retry observe StorageFull and schedule its later retry. - sleep(Duration::from_millis(150)).await; - filesystem.fail_data_writes_after(0, io::ErrorKind::PermissionDenied); - state.start_cancellation_retry_for_test(); - assert_eq!( - timeout(Duration::from_secs(1), &mut finalizer) - .await - .expect("terminal classification should resolve the owned record once"), - BatchStatus::Errored - ); - - // The queued ordinary driver would get Ready from the restored filesystem if it did not - // recheck the terminal state while holding the writer mutex. - filesystem.restore_data_writes(); - sleep(Duration::from_millis(250)).await; - assert!( - sender.send(Record::new(115, 64, 1), None).await.is_err(), - "a stale Ready retry must not overwrite Failed" - ); -} - -#[tokio::test] -async fn cancelled_production_send_from_ready_retains_owner_until_syscall_is_classified() { - let directory = TempDir::with_prefix("vector-buffer-adapter-cancel").unwrap(); - let data_dir = directory.path().to_path_buf(); - let StalledWrites { filesystem, gate } = ProductionFilesystem::with_stalled_writes(); - let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); - let config = DiskBufferConfigBuilder::from_path(&data_dir) - .write_buffer_size(16) - .filesystem(filesystem.clone()) - .build() - .unwrap(); - let (writer, reader, _) = Buffer::from_config_inner(config, BufferUsageHandle::noop()) - .await - .unwrap(); - let mut sender = BufferSender::new(SenderAdapter::from(writer), WhenFull::Block); - let weak = match sender.get_base_ref() { - SenderAdapter::DiskV2(state) => Arc::downgrade(state), - _ => unreachable!("production disk sender expected"), - }; - let mut record = Record::new(115, 256, 1); - let expected = record.clone(); - let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); - record.add_batch_notifier(batch); - - let mut send = Box::pin(sender.send(record, None)); - tokio::select! { - result = &mut send => panic!("gated send unexpectedly completed: {result:?}"), - () = gate.wait_until_started() => {} - } - drop(send); - // Cancellation registers its retained driver before a normal starter can observe Retrying. - match sender.get_base_ref() { - SenderAdapter::DiskV2(state) => state.start_normal_capacity_retry_for_test(), - _ => unreachable!("production disk sender expected"), - } - drop(sender); - drop(reader); - assert!( - weak.upgrade().is_some(), - "the cancellation retry must retain the sender while its syscall is gated" - ); - assert!( - timeout(Duration::from_millis(20), &mut finalizer) - .await - .is_err(), - "writer-owned finalizers must remain unresolved while the syscall is pending" - ); - - gate.release(); - assert_eq!( - timeout(Duration::from_secs(2), &mut finalizer) - .await - .expect("the recovered write should resolve its finalizer"), - BatchStatus::Delivered - ); - timeout(Duration::from_millis(100), async { - while weak.upgrade().is_some() { - tokio::task::yield_now().await; - } - }) - .await - .expect("the retry must release the sender after classifying the syscall"); - - let config = DiskBufferConfigBuilder::from_path(&data_dir) - .write_buffer_size(16) - .filesystem(filesystem) - .build() - .unwrap(); - let (recovered_writer, mut recovered_reader, _) = - Buffer::from_config_inner(config, BufferUsageHandle::noop()) - .await - .unwrap(); - drop(recovered_writer); - assert_eq!( - timeout(Duration::from_secs(2), recovered_reader.next()) - .await - .expect("the recovered record should be readable") - .unwrap(), - Some(expected) - ); - assert_eq!( - timeout(Duration::from_secs(2), recovered_reader.next()) - .await - .expect("the recovered buffer should reach EOF") - .unwrap(), - None - ); -} - -#[tokio::test] -async fn cancelled_production_flush_while_retrying_retains_owner_until_classified() { - let directory = TempDir::with_prefix("vector-buffer-adapter-retrying-cancel").unwrap(); - let data_dir = directory.path().to_path_buf(); - let StalledWrites { filesystem, gate } = ProductionFilesystem::with_stalled_writes(); - let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); - let config = DiskBufferConfigBuilder::from_path(&data_dir) - .write_buffer_size(1024) - .filesystem(filesystem.clone()) - .build() - .unwrap(); - let (writer, reader, _) = Buffer::from_config_inner(config, BufferUsageHandle::noop()) - .await - .unwrap(); - let mut sender = BufferSender::new(SenderAdapter::from(writer), WhenFull::Block); - let weak = match sender.get_base_ref() { - SenderAdapter::DiskV2(state) => Arc::downgrade(state), - _ => unreachable!("production disk sender expected"), - }; - let mut record = Record::new(119, 256, 1); - let expected = record.clone(); - let (batch, mut finalizer) = BatchNotifier::new_with_receiver(); - record.add_batch_notifier(batch); - sender.send(record, None).await.unwrap(); - match sender.get_base_ref() { - SenderAdapter::DiskV2(state) => state.start_normal_capacity_retry_for_test(), - _ => unreachable!("production disk sender expected"), - } - - let mut flush = Box::pin(sender.flush()); - tokio::select! { - result = &mut flush => panic!("gated flush unexpectedly completed: {result:?}"), - () = gate.wait_until_started() => {} - } - drop(flush); - drop(sender); - drop(reader); - assert!( - weak.upgrade().is_some(), - "the cancellation classifier must retain the sender while its syscall is gated" - ); - - gate.release(); - assert_eq!( - timeout(Duration::from_secs(2), &mut finalizer) - .await - .expect("the accepted record should resolve its finalizer"), - BatchStatus::Delivered - ); - timeout(Duration::from_secs(2), async { - while weak.upgrade().is_some() { - tokio::task::yield_now().await; - } - }) - .await - .expect("the classifier must release the sender after classifying the syscall"); - - let config = DiskBufferConfigBuilder::from_path(&data_dir) - .write_buffer_size(1024) - .filesystem(filesystem) - .build() - .unwrap(); - let (recovered_writer, mut recovered_reader, _) = - Buffer::from_config_inner(config, BufferUsageHandle::noop()) - .await - .unwrap(); - drop(recovered_writer); - assert_eq!( - timeout(Duration::from_secs(2), recovered_reader.next()) - .await - .expect("the recovered record should be readable") - .unwrap(), - Some(expected) - ); - assert_eq!( - timeout(Duration::from_secs(2), recovered_reader.next()) - .await - .expect("the recovered buffer should reach EOF") - .unwrap(), - None - ); -} - -#[tokio::test] -async fn cancelled_production_flush_recovers_accepted_record() { - let directory = TempDir::with_prefix("vector-buffer-adapter-flush-cancel").unwrap(); - let StalledWrites { filesystem, gate } = ProductionFilesystem::with_stalled_writes(); - let _gate_cleanup = WriteGateCleanup(Arc::clone(&gate)); - let config = DiskBufferConfigBuilder::from_path(directory.path()) - .write_buffer_size(1024) - .filesystem(filesystem) - .build() - .unwrap(); - let (writer, mut reader, _) = Buffer::from_config_inner(config, BufferUsageHandle::noop()) - .await - .unwrap(); - let mut sender = BufferSender::new(SenderAdapter::from(writer), WhenFull::Block); - let record = Record::new(118, 256, 1); - - sender.send(record.clone(), None).await.unwrap(); - let mut flush = Box::pin(sender.flush()); - tokio::select! { - result = &mut flush => panic!("gated flush unexpectedly completed: {result:?}"), - () = gate.wait_until_started() => {} - } - drop(flush); - gate.release(); - - assert_eq!( - timeout(Duration::from_secs(2), reader.next()) - .await - .expect("cancelled flush should be recovered in the background") - .unwrap(), - Some(record) - ); -}