Skip to content

Commit

Permalink
fix(subscriber): correct retain logic (#447)
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
hds authored Aug 1, 2023
1 parent 639e03b commit fa9a45c
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions console-subscriber/src/aggregator/id_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,18 +104,18 @@ impl<T: Unsent> IdData<T> {
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
Expand Down

0 comments on commit fa9a45c

Please sign in to comment.