fix: bound fleet rebuilds and enable OTLP exporter retries
This commit is contained in:
10 files changed
+271
-64
No files matched your search
@@ -51,6 +51,21 @@ async fn fleet_view(state: &AppState) -> anyhow::Result<std::sync::Arc<Vec<Fleet
|
||||
|
||||
const REFRESH_CONCURRENCY: usize = 8;
|
||||
|
||||
fn fleet_error(err: anyhow::Error, fallback_code: &str) -> ApiError {
|
||||
if err.is::<crate::runtime::view_cache::RebuildLimit>() {
|
||||
return ApiError::new(
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
"fleet_busy",
|
||||
err.to_string(),
|
||||
);
|
||||
}
|
||||
ApiError::new(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
fallback_code,
|
||||
err.to_string(),
|
||||
)
|
||||
}
|
||||
|
||||
#[tracing::instrument(name = "fleet_list", skip_all)]
|
||||
pub async fn list_fleet_devices(
|
||||
State(state): State<AppState>,
|
||||
@@ -60,11 +75,7 @@ pub async fn list_fleet_devices(
|
||||
Ok(devices) => Ok((StatusCode::OK, Json(devices))),
|
||||
Err(err) => {
|
||||
warn!(error = %err, "failed to list fleet devices");
|
||||
Err(ApiError::new(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"list_fleet_devices_failed",
|
||||
err.to_string(),
|
||||
))
|
||||
Err(fleet_error(err, "list_fleet_devices_failed"))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -226,11 +237,7 @@ pub async fn wake_fleet_device(
|
||||
};
|
||||
let devices = load_fleet_devices(&state, &query).await.map_err(|err| {
|
||||
warn!(error = %err, "failed loading fleet devices for wake");
|
||||
ApiError::new(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"load_fleet_devices_failed",
|
||||
err.to_string(),
|
||||
)
|
||||
fleet_error(err, "load_fleet_devices_failed")
|
||||
})?;
|
||||
|
||||
let device = devices
|
||||
|
||||
@@ -12,6 +12,22 @@ use super::normalize_refresh_agent_ids;
|
||||
use super::types::ListFleetDevicesQuery;
|
||||
use crate::state::{AgentDeviceRow, AgentDeviceWithChildren, DeviceIdentifier, KnownDevice};
|
||||
|
||||
#[test]
|
||||
fn cache_rebuild_limit_is_retryable_but_db_failures_keep_their_error_code() {
|
||||
let busy = super::fleet_error(
|
||||
crate::runtime::view_cache::RebuildLimit.into(),
|
||||
"list_fleet_devices_failed",
|
||||
);
|
||||
assert_eq!(busy.status, axum::http::StatusCode::SERVICE_UNAVAILABLE);
|
||||
assert_eq!(busy.code, "fleet_busy");
|
||||
let failed = super::fleet_error(
|
||||
anyhow::anyhow!("database failed"),
|
||||
"list_fleet_devices_failed",
|
||||
);
|
||||
assert_eq!(failed.status, axum::http::StatusCode::INTERNAL_SERVER_ERROR);
|
||||
assert_eq!(failed.code, "list_fleet_devices_failed");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn fleet_cache_tracks_store_edits_and_connections_without_caching_filters() {
|
||||
use crate::runtime::{AgentSession, AppState};
|
||||
|
||||
@@ -5,6 +5,19 @@ use std::sync::Arc;
|
||||
|
||||
use tokio::sync::Mutex;
|
||||
|
||||
const MAX_REBUILDS: usize = 3;
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct RebuildLimit;
|
||||
|
||||
impl std::fmt::Display for RebuildLimit {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.write_str("view inputs kept changing during rebuilding; retry the request")
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for RebuildLimit {}
|
||||
|
||||
pub(crate) struct ViewCache<K, V> {
|
||||
entry: Mutex<Option<(K, Arc<V>)>>,
|
||||
}
|
||||
@@ -21,19 +34,20 @@ impl<K: Eq, V> ViewCache<K, V> {
|
||||
/// Reuse a matching view or rebuild it. `key` must reflect every input to
|
||||
/// `build`; rechecking it prevents an in-flight old build being published.
|
||||
/// Errors are not cached, and cancellation releases the rebuild lock.
|
||||
pub(crate) async fn get_or_try_init<E, KF, BF>(
|
||||
/// Three invalidated builds return `RebuildLimit` rather than starving readers.
|
||||
pub(crate) async fn get_or_try_init<KF, BF>(
|
||||
&self,
|
||||
key: impl Fn() -> KF,
|
||||
build: impl Fn() -> BF,
|
||||
) -> Result<Arc<V>, E>
|
||||
) -> anyhow::Result<Arc<V>>
|
||||
where
|
||||
KF: Future<Output = K>,
|
||||
BF: Future<Output = Result<V, E>>,
|
||||
BF: Future<Output = anyhow::Result<V>>,
|
||||
{
|
||||
// Keep publication and rebuilding under one lock. Waiting readers then
|
||||
// reuse the completed view rather than launching duplicate DB work.
|
||||
let mut entry = self.entry.lock().await;
|
||||
loop {
|
||||
for _ in 0..MAX_REBUILDS {
|
||||
let before = key().await;
|
||||
if let Some((stored_key, value)) = &*entry
|
||||
&& *stored_key == before
|
||||
@@ -47,6 +61,7 @@ impl<K: Eq, V> ViewCache<K, V> {
|
||||
return Ok(value);
|
||||
}
|
||||
}
|
||||
Err(RebuildLimit.into())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -65,7 +80,7 @@ mod tests {
|
||||
let build = || async {
|
||||
builds.fetch_add(1, Ordering::SeqCst);
|
||||
tokio::task::yield_now().await;
|
||||
Ok::<_, ()>(42)
|
||||
Ok::<_, anyhow::Error>(42)
|
||||
};
|
||||
let (a, b) = tokio::join!(
|
||||
cache.get_or_try_init(key, build),
|
||||
@@ -93,7 +108,7 @@ mod tests {
|
||||
started.notify_one();
|
||||
release.notified().await;
|
||||
}
|
||||
Ok::<_, ()>(captured)
|
||||
Ok::<_, anyhow::Error>(captured)
|
||||
},
|
||||
);
|
||||
let writer = async {
|
||||
@@ -111,13 +126,16 @@ mod tests {
|
||||
let cache = ViewCache::default();
|
||||
assert!(
|
||||
cache
|
||||
.get_or_try_init(|| async { 0 }, || async { Err::<u8, _>("db failed") })
|
||||
.get_or_try_init(
|
||||
|| async { 0 },
|
||||
|| async { Err::<u8, _>(anyhow::anyhow!("db failed")) }
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
assert_eq!(
|
||||
*cache
|
||||
.get_or_try_init(|| async { 0 }, || async { Ok::<_, &str>(7) })
|
||||
.get_or_try_init(|| async { 0 }, || async { Ok::<_, anyhow::Error>(7) })
|
||||
.await
|
||||
.unwrap(),
|
||||
7
|
||||
@@ -137,7 +155,7 @@ mod tests {
|
||||
|| async { 0 },
|
||||
|| async {
|
||||
started.notify_one();
|
||||
std::future::pending::<Result<u8, ()>>().await
|
||||
std::future::pending::<anyhow::Result<u8>>().await
|
||||
},
|
||||
)
|
||||
.await
|
||||
@@ -148,10 +166,41 @@ mod tests {
|
||||
assert!(task.await.unwrap_err().is_cancelled());
|
||||
assert_eq!(
|
||||
*cache
|
||||
.get_or_try_init(|| async { 0 }, || async { Ok::<_, ()>(7) })
|
||||
.get_or_try_init(|| async { 0 }, || async { Ok::<_, anyhow::Error>(7) })
|
||||
.await
|
||||
.unwrap(),
|
||||
7
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sustained_changes_stop_at_the_limit_and_a_later_read_recovers() {
|
||||
let cache = ViewCache::default();
|
||||
let revision = AtomicU64::new(0);
|
||||
let key = || async { revision.load(Ordering::SeqCst) };
|
||||
cache
|
||||
.get_or_try_init(key, || async { Ok(0_u8) })
|
||||
.await
|
||||
.unwrap();
|
||||
revision.store(1, Ordering::SeqCst);
|
||||
let builds = AtomicU64::new(0);
|
||||
let error = cache
|
||||
.get_or_try_init(key, || async {
|
||||
builds.fetch_add(1, Ordering::SeqCst);
|
||||
revision.fetch_add(1, Ordering::SeqCst);
|
||||
Ok(1)
|
||||
})
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(error.is::<RebuildLimit>());
|
||||
assert_eq!(builds.load(Ordering::SeqCst), MAX_REBUILDS as u64);
|
||||
let recovered = cache
|
||||
.get_or_try_init(key, || async { Ok(2) })
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
*recovered, 2,
|
||||
"must not fall back to the previous cached value"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user