mmodzelewski commented on code in PR #4318:
URL: https://github.com/apache/iggy/pull/4318#discussion_r4138060916


##########
core/server/src/sysinfo_probe.rs:
##########
@@ -105,32 +117,52 @@ pub fn stats_disk_space() -> (u64, u64) {
     })
 }
 
+/// Count open descriptors, scanning where the kernel keeps no count, and
+/// publish the result for [`probe_system_stats`].
+///
+/// Only the sysinfo printer calls this, once per interval. The scan cost
+/// grows with the count, so `GetStats` reads the published value instead of
+/// scanning on the request path. The value is as old as the printer interval,
+/// and stays 0 while the printer is disabled.
+pub fn publish_open_files_count() {
+    PUBLISHED_OPEN_FILES_COUNT.store(count_open_files().unwrap_or(0), 
Ordering::Relaxed);
+}
+
+/// Probe through this thread's [`SYSINFO`], the `GetStats` path.
 pub fn probe_system_stats() -> SystemStats {
-    let host = HOST_IDENTITY.get_or_init(HostIdentity::probe);
-    let probe = SYSINFO.with_borrow_mut(|slot| {
-        let sys = slot.get_or_insert_with(SysinfoSystem::new);
-        SystemProbe::capture(sys)
-    });
-
-    SystemStats {
-        process_id: probe.process_id,
-        cpu_usage: probe.cpu_usage,
-        total_cpu_usage: probe.total_cpu_usage,
-        memory_usage: probe.memory_usage,
-        total_memory: probe.total_memory,
-        available_memory: probe.available_memory,
-        // sysinfo reports whole seconds; the wire fields are micros (the
-        // SDK decodes them via `IggyDuration` / `IggyTimestamp::from`, both
-        // micro-based).
-        run_time: probe.run_time_secs.saturating_mul(1_000_000),
-        start_time: probe.start_time_secs.saturating_mul(1_000_000),
-        read_bytes: probe.read_bytes,
-        written_bytes: probe.written_bytes,
-        threads_count: probe.threads_count,
-        hostname: host.hostname.clone(),
-        os_name: host.os_name.clone(),
-        os_version: host.os_version.clone(),
-        kernel_version: host.kernel_version.clone(),
+    SYSINFO
+        .with_borrow_mut(|slot| 
SystemStats::capture(slot.get_or_insert_with(SysinfoSystem::new)))
+}
+
+impl SystemStats {
+    /// Probe through `sys`. A caller that keeps its own `sys` gets CPU deltas
+    /// over its own interval, and does not reset the window of `GetStats`.
+    pub fn capture(sys: &mut SysinfoSystem) -> Self {
+        let host = HOST_IDENTITY.get_or_init(HostIdentity::probe);
+        let probe = SystemProbe::capture(sys);
+        Self {
+            process_id: probe.process_id,
+            cpu_usage: probe.cpu_usage,
+            total_cpu_usage: probe.total_cpu_usage,
+            memory_usage: probe.memory_usage,
+            total_memory: probe.total_memory,
+            available_memory: probe.available_memory,
+            // sysinfo reports whole seconds; the wire fields are micros (the
+            // SDK decodes them via `IggyDuration` / `IggyTimestamp::from`, 
both
+            // micro-based).
+            run_time: probe.run_time_secs.saturating_mul(1_000_000),
+            start_time: probe.start_time_secs.saturating_mul(1_000_000),
+            read_bytes: probe.read_bytes,
+            written_bytes: probe.written_bytes,
+            threads_count: probe.threads_count,
+            open_files_count: count_open_files_without_scan()
+                .unwrap_or_else(|| 
PUBLISHED_OPEN_FILES_COUNT.load(Ordering::Relaxed)),
+            open_files_limit: getrlimit(Resource::RLIMIT_NOFILE).map_or(0, 
|(soft, _)| soft),

Review Comment:
   I'd keep it as there is no other function that reports it. Other stats like 
total memory, os name or os version also do not change, yet we report them for 
completeness. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to