make the forge-notify cursor bound explicit: shared fetch-limit const + assert + docs (closes #2117)
This commit is contained in:
parent
c89642872c
commit
383bb3b083
2 changed files with 28 additions and 1 deletions
|
|
@ -100,6 +100,15 @@ Each poll prunes it to the threads still in the unread set. A failed
|
||||||
wake delivery is left unread **and** out of the cursor, so it
|
wake delivery is left unread **and** out of the cursor, so it
|
||||||
resurfaces next tick.
|
resurfaces next tick.
|
||||||
|
|
||||||
|
**Size bound:** the per-poll prune retains only ids present in the
|
||||||
|
single `limit=UNREAD_FETCH_LIMIT` (50) fetch page, so the cursor never
|
||||||
|
exceeds that many entries — it tracks the unread *window*, not the
|
||||||
|
all-time notification count. The fetch limit and the bound are the
|
||||||
|
same constant in `forge_notify.rs` (with a debug assertion), so a
|
||||||
|
future pagination change grows the ceiling visibly rather than
|
||||||
|
silently. This is why the cursor stays a small JSON field rather than
|
||||||
|
a db table — see the storage discussion on the tracker (issue 2117).
|
||||||
|
|
||||||
The cursor is **persisted** as the `forge_cursor` field of the
|
The cursor is **persisted** as the `forge_cursor` field of the
|
||||||
harness's consolidated `hyperhive-harness.json` state file (atomic
|
harness's consolidated `hyperhive-harness.json` state file (atomic
|
||||||
tmp+rename, flushed only when it changed) and reloaded on boot, so a
|
tmp+rename, flushed only when it changed) and reloaded on boot, so a
|
||||||
|
|
|
||||||
|
|
@ -28,6 +28,13 @@ use tracing::{debug, info, warn};
|
||||||
|
|
||||||
const POLL_INTERVAL_SECS: u64 = 30;
|
const POLL_INTERVAL_SECS: u64 = 30;
|
||||||
const HTTP_TIMEOUT_SECS: u64 = 10;
|
const HTTP_TIMEOUT_SECS: u64 = 10;
|
||||||
|
/// Page size of the unread-notifications fetch. This is also the hard
|
||||||
|
/// bound on the persisted delivery-dedupe cursor: each poll prunes the
|
||||||
|
/// cursor to the ids in this window, so the `forge_cursor` field in
|
||||||
|
/// `hyperhive-harness.json` can never exceed this many entries. Keep
|
||||||
|
/// the two coupled — bumping the fetch limit grows the cursor's ceiling
|
||||||
|
/// with it, deliberately and visibly.
|
||||||
|
const UNREAD_FETCH_LIMIT: usize = 50;
|
||||||
/// Maximum characters of a body/comment to include in the wake message.
|
/// Maximum characters of a body/comment to include in the wake message.
|
||||||
const BODY_TRUNCATE: usize = 500;
|
const BODY_TRUNCATE: usize = 500;
|
||||||
/// How long to wait between token-read retries when the token file is
|
/// How long to wait between token-read retries when the token file is
|
||||||
|
|
@ -743,7 +750,7 @@ async fn poll_once(
|
||||||
delivered: &mut HashMap<u64, String>,
|
delivered: &mut HashMap<u64, String>,
|
||||||
own_login: &str,
|
own_login: &str,
|
||||||
) {
|
) {
|
||||||
let url = format!("{forge_url}/api/v1/notifications?all=false&limit=50");
|
let url = format!("{forge_url}/api/v1/notifications?all=false&limit={UNREAD_FETCH_LIMIT}");
|
||||||
let resp = match client
|
let resp = match client
|
||||||
.get(&url)
|
.get(&url)
|
||||||
.header("Authorization", format!("token {token}"))
|
.header("Authorization", format!("token {token}"))
|
||||||
|
|
@ -841,12 +848,23 @@ async fn poll_once(
|
||||||
// dead weight; dropping it bounds the map to the current unread size.
|
// dead weight; dropping it bounds the map to the current unread size.
|
||||||
// If such a thread later goes unread again it carries a fresh
|
// If such a thread later goes unread again it carries a fresh
|
||||||
// `updated_at` and re-delivers correctly.
|
// `updated_at` and re-delivers correctly.
|
||||||
|
//
|
||||||
|
// This retain IS the cursor's size bound: `current_ids` comes from a
|
||||||
|
// single `limit=UNREAD_FETCH_LIMIT` page, so the persisted cursor can
|
||||||
|
// never exceed that many entries — it tracks the unread *window*, not
|
||||||
|
// the all-time notification count. The assert makes the invariant
|
||||||
|
// loud in tests/dev if a future pagination change silently breaks it.
|
||||||
let current_ids: HashSet<u64> = notifications
|
let current_ids: HashSet<u64> = notifications
|
||||||
.iter()
|
.iter()
|
||||||
.filter_map(|n| n["id"].as_u64())
|
.filter_map(|n| n["id"].as_u64())
|
||||||
.collect();
|
.collect();
|
||||||
let before_prune = delivered.len();
|
let before_prune = delivered.len();
|
||||||
delivered.retain(|id, _| current_ids.contains(id));
|
delivered.retain(|id, _| current_ids.contains(id));
|
||||||
|
debug_assert!(
|
||||||
|
delivered.len() <= UNREAD_FETCH_LIMIT,
|
||||||
|
"dedupe cursor exceeded the fetch window ({} > {UNREAD_FETCH_LIMIT})",
|
||||||
|
delivered.len(),
|
||||||
|
);
|
||||||
if delivered.len() != before_prune {
|
if delivered.len() != before_prune {
|
||||||
cursor_dirty = true;
|
cursor_dirty = true;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue