From 36ffc513b26f27d5fb6344c24f12572ec76e41ac Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Tue, 1 Aug 2023 19:34:21 +0200 Subject: [PATCH] fix(subscriber): correct retain logic (#447) The current logic present in `IdData::drop_closed` marks an item (task, resource, and async op stats) to be dropped in the case that the item **is** dirty and there **are** watchers: `(dirty && has_watchers)`. This causes a case where if an item is first received and then completes in between the aggregator push cycle, it will be discarded immediately and never sent. This logic has been in place since the concepts of watchers and dirty items was introduced in #77. However since an item that is created and then dropped within a single update cycle isn't likely to be missed in the UI, it may never have been noticed. Instead the logic should be to **retain** an item if **any** of the following is true: * there are watchers and the item is dirty: `(dirty && has_watchers)` * item has been dropped less time than the retention period: `dropped_for <= retention`. --- console-subscriber/src/aggregator/id_data.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/console-subscriber/src/aggregator/id_data.rs b/console-subscriber/src/aggregator/id_data.rs index b9010b445..2ad2c74b0 100644 --- a/console-subscriber/src/aggregator/id_data.rs +++ b/console-subscriber/src/aggregator/id_data.rs @@ -104,18 +104,18 @@ impl IdData { if let Some(dropped_at) = stats.dropped_at() { let dropped_for = now.checked_duration_since(dropped_at).unwrap_or_default(); let dirty = stats.is_unsent(); - let should_drop = + let should_retain = // if there are any clients watching, retain all dirty tasks regardless of age (dirty && has_watchers) - || dropped_for > retention; + || dropped_for <= retention; tracing::trace!( stats.id = ?id, stats.dropped_at = ?dropped_at, stats.dropped_for = ?dropped_for, stats.dirty = dirty, - should_drop, + should_retain, ); - return !should_drop; + return should_retain; } true