Fix

getsentry · iambriccardo · Jul 22, 2024 · Jul 15, 2024 · Jul 16, 2024 · Jul 16, 2024
commit cc0596dae94ad2cd8b450304425b6ec7da45252a
@@ -1349,7 +1349,7 @@ pub struct Health {
     pub refresh_interval_ms: u64,
     /// Maximum memory watermark in bytes.
     ///
-    /// By default there is no absolute limit set and the watermark
+    /// By default, there is no absolute limit set and the watermark
     /// is only controlled by setting [`Self::max_memory_percent`].
     pub max_memory_bytes: Option<ByteSize>,
     /// Maximum memory watermark as a percentage of maximum system memory.

@@ -296,10 +296,6 @@ fn queue_envelope(
     // Split off the envelopes by item type.
     let envelopes = ProcessingGroup::split_envelope(*managed_envelope.take_envelope());
     for (group, envelope) in envelopes {
-        if !state.memory_stat().has_enough_memory() {
-            return Err(BadStoreRequest::QueueFailed);
-        }
-
         let envelope = ManagedEnvelope::standalone(
             envelope,
             state.outcome_aggregator().clone(),
@@ -335,9 +331,9 @@ pub async fn handle_envelope(
         )
     }
 
-    if !state.memory_stat().has_enough_memory() {
+    if !state.memory_stat_config().has_enough_memory() {
         return Err(BadStoreRequest::QueueFailed);
-    }
+    };
 
     let mut managed_envelope = ManagedEnvelope::standalone(
         envelope,

@@ -26,7 +26,7 @@ use crate::services::relays::{RelayCache, RelayCacheService};
 use crate::services::store::StoreService;
 use crate::services::test_store::{TestStore, TestStoreService};
 use crate::services::upstream::{UpstreamRelay, UpstreamRelayService};
-use crate::utils::{BufferGuard, MemoryStat};
+use crate::utils::{BufferGuard, MemoryStat, MemoryStatConfig};
 
 /// Indicates the type of failure of the server.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, thiserror::Error)]
@@ -84,8 +84,7 @@ pub fn create_runtime(name: &str, threads: usize) -> Runtime {
 #[derive(Debug)]
 struct StateInner {
     config: Arc<Config>,
-    buffer_guard: Arc<BufferGuard>,
-    memory_stat: MemoryStat,
+    memory_stat_config: MemoryStatConfig,
     registry: Registry,
 }
 
@@ -108,9 +107,9 @@ impl ServiceState {
             _ => None,
         };
 
-        let buffer_guard = Arc::new(BufferGuard::new(config.envelope_buffer_size()));
-
-        let memory_stat = MemoryStat::new(config.health_max_memory_watermark_percent());
+        // We create an instance of `MemoryStat` which can be supplied composed with any arbitrary
+        // configuration object down the line.
+        let memory_stat = MemoryStat::new();
 
         // Create an address for the `EnvelopeProcessor`, which can be injected into the
         // other services.
@@ -198,7 +197,7 @@ impl ServiceState {
         );
         ProjectCacheService::new(
             config.clone(),
-            buffer_guard.clone(),
+            memory_stat.with_config(config.clone()),
             project_cache_services,
             metric_outcomes,
             redis_pool.clone(),
@@ -207,10 +206,10 @@ impl ServiceState {
 
         let health_check = HealthCheckService::new(
             config.clone(),
+            memory_stat.with_config(config.clone()),
             aggregator.clone(),
             upstream_relay.clone(),
             project_cache.clone(),
-            memory_stat.clone(),
         )
         .start();
 
@@ -238,9 +237,8 @@ impl ServiceState {
         };
 
         let state = StateInner {
-            config,
-            buffer_guard,
-            memory_stat,
+            config: config.clone(),
+            memory_stat_config: memory_stat.with_config(config),
             registry,
         };
 
@@ -254,16 +252,11 @@ impl ServiceState {
         &self.inner.config
     }
 
-    /// Returns a reference to the guard of the envelope buffer.
-    ///
-    /// This can be used to enter new envelopes into the processing queue and reserve a slot in the
-    /// buffer. See [`BufferGuard`] for more information.
-    pub fn buffer_guard(&self) -> &BufferGuard {
-        &self.inner.buffer_guard
-    }
-
-    pub fn memory_stat(&self) -> &MemoryStat {
-        &self.inner.memory_stat
+    /// Returns a reference to the [`MemoryStatConfig`] which is a [`Config`] aware wrapper on the
+    /// [`MemoryStat`] which gives utility methods to determine whether memory usage is above
+    /// thresholds set in the [`Config`].
+    pub fn memory_stat_config(&self) -> &MemoryStatConfig {
+        &self.inner.memory_stat_config
     }
 
     /// Returns the address of the [`ProjectCache`] service.

@@ -10,7 +10,7 @@ use crate::services::metrics::{AcceptsMetrics, Aggregator};
 use crate::services::project_cache::{ProjectCache, SpoolHealth};
 use crate::services::upstream::{IsAuthenticated, UpstreamRelay};
 use crate::statsd::RelayTimers;
-use crate::utils::MemoryStat;
+use crate::utils::{MemoryStat, MemoryStatConfig};
 
 /// Checks whether Relay is alive and healthy based on its variant.
 #[derive(Clone, Copy, Debug, serde::Deserialize)]
@@ -83,10 +83,10 @@ impl StatusUpdate {
 #[derive(Debug)]
 pub struct HealthCheckService {
     config: Arc<Config>,
+    memory_stat_config: MemoryStatConfig,
     aggregator: Addr<Aggregator>,
     upstream_relay: Addr<UpstreamRelay>,
     project_cache: Addr<ProjectCache>,
-    memory_stat: MemoryStat,
 }
 
 impl HealthCheckService {
@@ -95,23 +95,23 @@ impl HealthCheckService {
     /// The service does not run. To run the service, use [`start`](Self::start).
     pub fn new(
         config: Arc<Config>,
+        memory_stat_config: MemoryStatConfig,
         aggregator: Addr<Aggregator>,
         upstream_relay: Addr<UpstreamRelay>,
         project_cache: Addr<ProjectCache>,
-        memory_stat: MemoryStat,
     ) -> Self {
         Self {
+            config,
+            memory_stat_config,
             aggregator,
             upstream_relay,
             project_cache,
-            config,
-            memory_stat,
         }
     }
 
     fn system_memory_probe(&mut self) -> Status {
-        let memory = self.memory_stat.memory();
-        if memory.used_percent() >= self.config.health_max_memory_watermark_percent() {
+        if !self.memory_stat_config.has_enough_memory_percent() {
+            let memory = self.memory_stat_config.memory_stat.memory();
             relay_log::error!(
                 "Not enough memory, {} / {} ({:.2}% >= {:.2}%)",
                 memory.used,
@@ -122,7 +122,8 @@ impl HealthCheckService {
             return Status::Unhealthy;
         }
 
-        if memory.used > self.config.health_max_memory_watermark_bytes() {
+        if !self.memory_stat_config.has_enough_memory_bytes() {
+            let memory = self.memory_stat_config.memory_stat.memory();
             relay_log::error!(
                 "Not enough memory, {} / {} ({} >= {})",
                 memory.used,