signed_state: Simplify refresh coalescing and stop blocking the executor
This commit is contained in:
@@ -5,7 +5,7 @@ use std::time::{Duration, Instant};
|
|||||||
|
|
||||||
use anyhow::{Error, anyhow, bail};
|
use anyhow::{Error, anyhow, bail};
|
||||||
use bitcoin_hashes::sha1::Hash as Sha1Hash;
|
use bitcoin_hashes::sha1::Hash as Sha1Hash;
|
||||||
use gpui::{App, AppContext, Context, Entity, EventEmitter, Global, Task};
|
use gpui::{App, AppContext, BackgroundExecutor, Context, Entity, EventEmitter, Global, Task};
|
||||||
use nostr::event::IntoEventBuilder;
|
use nostr::event::IntoEventBuilder;
|
||||||
use nostr_connect::prelude::*;
|
use nostr_connect::prelude::*;
|
||||||
use nostr_sdk::client::SyncSummary;
|
use nostr_sdk::client::SyncSummary;
|
||||||
@@ -531,6 +531,7 @@ impl Backend {
|
|||||||
let repo_id = repo_id.clone();
|
let repo_id = repo_id.clone();
|
||||||
let servers = servers.clone();
|
let servers = servers.clone();
|
||||||
let refs = refs.clone();
|
let refs = refs.clone();
|
||||||
|
let executor = cx.background_executor().clone();
|
||||||
async move {
|
async move {
|
||||||
push_staged_to_grasps(
|
push_staged_to_grasps(
|
||||||
&client,
|
&client,
|
||||||
@@ -541,6 +542,7 @@ impl Backend {
|
|||||||
&destination,
|
&destination,
|
||||||
&owner,
|
&owner,
|
||||||
&servers,
|
&servers,
|
||||||
|
&executor,
|
||||||
signed_git::push_main,
|
signed_git::push_main,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
@@ -686,6 +688,7 @@ impl Backend {
|
|||||||
let servers = servers.clone();
|
let servers = servers.clone();
|
||||||
let refs = refs.clone();
|
let refs = refs.clone();
|
||||||
let head = head.clone();
|
let head = head.clone();
|
||||||
|
let executor = cx.background_executor().clone();
|
||||||
async move {
|
async move {
|
||||||
push_staged_to_grasps(
|
push_staged_to_grasps(
|
||||||
&client,
|
&client,
|
||||||
@@ -696,6 +699,7 @@ impl Backend {
|
|||||||
&path,
|
&path,
|
||||||
&owner,
|
&owner,
|
||||||
&servers,
|
&servers,
|
||||||
|
&executor,
|
||||||
signed_git::push_all,
|
signed_git::push_all,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
@@ -854,6 +858,7 @@ impl Backend {
|
|||||||
let relays = relays.clone();
|
let relays = relays.clone();
|
||||||
let refs = refs.clone();
|
let refs = refs.clone();
|
||||||
let head = head.clone();
|
let head = head.clone();
|
||||||
|
let executor = cx.background_executor().clone();
|
||||||
async move {
|
async move {
|
||||||
push_staged_to_grasps(
|
push_staged_to_grasps(
|
||||||
&client,
|
&client,
|
||||||
@@ -864,6 +869,7 @@ impl Backend {
|
|||||||
&path,
|
&path,
|
||||||
&owner,
|
&owner,
|
||||||
&relays,
|
&relays,
|
||||||
|
&executor,
|
||||||
signed_git::push_all,
|
signed_git::push_all,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
@@ -1700,6 +1706,7 @@ async fn push_staged_to_grasps(
|
|||||||
path: &Path,
|
path: &Path,
|
||||||
owner: &str,
|
owner: &str,
|
||||||
servers: &[RelayUrl],
|
servers: &[RelayUrl],
|
||||||
|
executor: &BackgroundExecutor,
|
||||||
push: fn(&Path, &str, &str, &str) -> Result<(), Error>,
|
push: fn(&Path, &str, &str, &str) -> Result<(), Error>,
|
||||||
) -> PushOutcome {
|
) -> PushOutcome {
|
||||||
let mut outcome = PushOutcome::default();
|
let mut outcome = PushOutcome::default();
|
||||||
@@ -1722,7 +1729,7 @@ async fn push_staged_to_grasps(
|
|||||||
'server: for attempt in 1..=GRASP_PUSH_ATTEMPTS {
|
'server: for attempt in 1..=GRASP_PUSH_ATTEMPTS {
|
||||||
if attempt > 1 {
|
if attempt > 1 {
|
||||||
// Give the server's ingest a moment before re-staging.
|
// Give the server's ingest a moment before re-staging.
|
||||||
std::thread::sleep(GRASP_RETRY_DELAY);
|
executor.timer(GRASP_RETRY_DELAY).await;
|
||||||
}
|
}
|
||||||
|
|
||||||
let (event, created_at) =
|
let (event, created_at) =
|
||||||
|
|||||||
@@ -93,6 +93,8 @@ pub struct CheckoutsStore {
|
|||||||
/// A recompute defaults the base the same way.
|
/// A recompute defaults the base the same way.
|
||||||
requested_head: HashMap<RepoAddr, Option<String>>,
|
requested_head: HashMap<RepoAddr, Option<String>>,
|
||||||
refresh: RefreshGate,
|
refresh: RefreshGate,
|
||||||
|
/// True while the timer between a scheduled refresh and its run is pending.
|
||||||
|
debounce_pending: bool,
|
||||||
local_pending: bool,
|
local_pending: bool,
|
||||||
/// When the last full pass (with a remote refresh) completed.
|
/// When the last full pass (with a remote refresh) completed.
|
||||||
///
|
///
|
||||||
@@ -163,6 +165,7 @@ impl CheckoutsStore {
|
|||||||
push_statuses: HashMap::new(),
|
push_statuses: HashMap::new(),
|
||||||
requested_head: HashMap::new(),
|
requested_head: HashMap::new(),
|
||||||
refresh: RefreshGate::default(),
|
refresh: RefreshGate::default(),
|
||||||
|
debounce_pending: false,
|
||||||
local_pending: false,
|
local_pending: false,
|
||||||
last_full_sync: None,
|
last_full_sync: None,
|
||||||
_subscriptions: subscriptions,
|
_subscriptions: subscriptions,
|
||||||
@@ -279,12 +282,15 @@ impl CheckoutsStore {
|
|||||||
|
|
||||||
/// Re-resolve the associations and the requested statuses.
|
/// Re-resolve the associations and the requested statuses.
|
||||||
///
|
///
|
||||||
/// Requests arriving while a pass runs fold into a follow-up.
|
/// Requests arriving while a pass runs fold into a follow-up, requests
|
||||||
|
/// arriving while the debounce timer is pending are dropped.
|
||||||
pub fn refresh(&mut self, cx: &mut Context<Self>) {
|
pub fn refresh(&mut self, cx: &mut Context<Self>) {
|
||||||
if self.refresh.request() != RefreshRequest::Schedule {
|
if self.debounce_pending || self.refresh.request() != RefreshRequest::Schedule {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self.debounce_pending = true;
|
||||||
|
|
||||||
cx.spawn(async move |this, cx| {
|
cx.spawn(async move |this, cx| {
|
||||||
cx.background_executor().timer(REFRESH_DEBOUNCE).await;
|
cx.background_executor().timer(REFRESH_DEBOUNCE).await;
|
||||||
this.update(cx, |this, cx| this.run_refresh(cx))
|
this.update(cx, |this, cx| this.run_refresh(cx))
|
||||||
@@ -300,6 +306,7 @@ impl CheckoutsStore {
|
|||||||
/// remote reconciliation cadence ([`Self::local_tick`]); they also restart
|
/// remote reconciliation cadence ([`Self::local_tick`]); they also restart
|
||||||
/// the fast local pass.
|
/// the fast local pass.
|
||||||
fn run_refresh(&mut self, cx: &mut Context<Self>) {
|
fn run_refresh(&mut self, cx: &mut Context<Self>) {
|
||||||
|
self.debounce_pending = false;
|
||||||
self.refresh.begin();
|
self.refresh.begin();
|
||||||
|
|
||||||
let records = {
|
let records = {
|
||||||
@@ -453,7 +460,7 @@ impl CheckoutsStore {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// A full pass or a fresh request covers this tick, skip it.
|
// A full pass or a fresh request covers this tick, skip it.
|
||||||
if self.refresh.running() || self.refresh.debouncing() {
|
if self.refresh.running() || self.debounce_pending {
|
||||||
self.schedule_local_pass(cx);
|
self.schedule_local_pass(cx);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -515,7 +522,7 @@ impl CheckoutsStore {
|
|||||||
this.update(cx, |this, cx| {
|
this.update(cx, |this, cx| {
|
||||||
// A full pass or a fresh request will apply fresher data
|
// A full pass or a fresh request will apply fresher data
|
||||||
// (the tracking refs move only when a full pass fetches).
|
// (the tracking refs move only when a full pass fetches).
|
||||||
if this.refresh.running() || this.refresh.debouncing() {
|
if this.refresh.running() || this.debounce_pending {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3,14 +3,13 @@
|
|||||||
pub struct RefreshGate {
|
pub struct RefreshGate {
|
||||||
running: bool,
|
running: bool,
|
||||||
dirty: bool,
|
dirty: bool,
|
||||||
debouncing: bool,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||||
pub enum RefreshRequest {
|
pub enum RefreshRequest {
|
||||||
/// No run or timer covers the request, start the debounce timer.
|
/// No run covers the request, start one now.
|
||||||
Schedule,
|
Schedule,
|
||||||
/// A run or pending timer already covers the request.
|
/// A run is in flight and covers the request, fold it into a follow-up.
|
||||||
Fold,
|
Fold,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -19,28 +18,20 @@ impl RefreshGate {
|
|||||||
self.running
|
self.running
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn debouncing(&self) -> bool {
|
|
||||||
self.debouncing
|
|
||||||
}
|
|
||||||
|
|
||||||
/// A new refresh request arrived.
|
/// A new refresh request arrived.
|
||||||
///
|
///
|
||||||
/// Folded into a follow-up run while one is in flight, dropped while the
|
/// Folded into a follow-up run while one is in flight, otherwise the
|
||||||
/// debounce timer is pending, otherwise starts the timer.
|
/// caller starts the run itself.
|
||||||
pub fn request(&mut self) -> RefreshRequest {
|
pub fn request(&mut self) -> RefreshRequest {
|
||||||
if self.running {
|
if self.running {
|
||||||
self.dirty = true;
|
self.dirty = true;
|
||||||
RefreshRequest::Fold
|
RefreshRequest::Fold
|
||||||
} else if self.debouncing {
|
|
||||||
RefreshRequest::Fold
|
|
||||||
} else {
|
} else {
|
||||||
self.debouncing = true;
|
|
||||||
RefreshRequest::Schedule
|
RefreshRequest::Schedule
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn begin(&mut self) {
|
pub fn begin(&mut self) {
|
||||||
self.debouncing = false;
|
|
||||||
self.running = true;
|
self.running = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -55,3 +46,48 @@ impl RefreshGate {
|
|||||||
self.running = false;
|
self.running = false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_request_while_running_folds_into_a_follow_up() {
|
||||||
|
let mut gate = RefreshGate::default();
|
||||||
|
gate.begin();
|
||||||
|
|
||||||
|
assert_eq!(gate.request(), RefreshRequest::Fold);
|
||||||
|
assert!(gate.finish());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_request_without_a_run_schedules() {
|
||||||
|
let mut gate = RefreshGate::default();
|
||||||
|
|
||||||
|
assert_eq!(gate.request(), RefreshRequest::Schedule);
|
||||||
|
assert!(!gate.running());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_request_after_a_run_schedules_again() {
|
||||||
|
let mut gate = RefreshGate::default();
|
||||||
|
gate.begin();
|
||||||
|
assert_eq!(gate.request(), RefreshRequest::Fold);
|
||||||
|
assert!(gate.finish());
|
||||||
|
|
||||||
|
assert_eq!(gate.request(), RefreshRequest::Schedule);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn abort_keeps_the_pending_request() {
|
||||||
|
let mut gate = RefreshGate::default();
|
||||||
|
gate.begin();
|
||||||
|
assert_eq!(gate.request(), RefreshRequest::Fold);
|
||||||
|
|
||||||
|
gate.abort();
|
||||||
|
assert!(!gate.running());
|
||||||
|
|
||||||
|
gate.begin();
|
||||||
|
assert!(gate.finish());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -177,7 +177,6 @@ impl InboxView {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn refresh_initial(&mut self, cx: &mut Context<Self>) {
|
fn refresh_initial(&mut self, cx: &mut Context<Self>) {
|
||||||
debug_assert!(!self.refresh.debouncing());
|
|
||||||
if self.refresh.running() {
|
if self.refresh.running() {
|
||||||
self.refresh.request();
|
self.refresh.request();
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
# Over-optimization and over-engineering action plan
|
# Over-optimization and over-engineering action plan
|
||||||
|
|
||||||
- **Status:** phase 1 implemented; phases 2 and 3 still draft, for manual review
|
- **Status:** phases 1 and 2 implemented; phase 3 still draft, for manual review
|
||||||
- **Date:** 2026-09-13
|
- **Date:** 2026-09-13
|
||||||
- **Basis:** optimization inventory audit of branch `remove-ai-slop` (HEAD `534d572`)
|
- **Basis:** optimization inventory audit of branch `remove-ai-slop` (HEAD `534d572`)
|
||||||
- **Scope:** non-UI crates only — `signed_state`, `signed_git`, `signed_core`, `utils`,
|
- **Scope:** non-UI crates only — `signed_state`, `signed_git`, `signed_core`, `utils`,
|
||||||
@@ -123,6 +123,8 @@ Manual checks for phases 2 and 3 are in the validation section at the end.
|
|||||||
scanning coalesces; inbox refresh after a sync does not double-run.
|
scanning coalesces; inbox refresh after a sync does not double-run.
|
||||||
- **Size:** M. **Risk:** medium. The debounce timing of `CheckoutsStore` must not regress; the
|
- **Size:** M. **Risk:** medium. The debounce timing of `CheckoutsStore` must not regress; the
|
||||||
timer is preserved exactly, only its flag moves.
|
timer is preserved exactly, only its flag moves.
|
||||||
|
- **Status:** done 2026-09-13 — `RefreshGate` is `running`/`dirty` only, `CheckoutsStore` owns
|
||||||
|
`debounce_pending`, and the one-line `inbox.rs` adaptation landed here.
|
||||||
|
|
||||||
### OV-04 — Replace the blocking sleep in the grasp push retry
|
### OV-04 — Replace the blocking sleep in the grasp push retry
|
||||||
|
|
||||||
@@ -140,6 +142,8 @@ Manual checks for phases 2 and 3 are in the validation section at the end.
|
|||||||
- **Acceptance:** `cargo check -p signed_state`; a push that hits a transient denial still
|
- **Acceptance:** `cargo check -p signed_state`; a push that hits a transient denial still
|
||||||
retries with the same spacing.
|
retries with the same spacing.
|
||||||
- **Size:** S. **Risk:** low.
|
- **Size:** S. **Risk:** low.
|
||||||
|
- **Status:** done 2026-09-13 — `push_staged_to_grasps` takes a `BackgroundExecutor` and awaits
|
||||||
|
`executor.timer(GRASP_RETRY_DELAY)`.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -288,7 +292,8 @@ Renumbering, for traceability from the first review round:
|
|||||||
|
|
||||||
## Open questions for the owner
|
## Open questions for the owner
|
||||||
|
|
||||||
1. OV-03: accept the two-flag `RefreshGate` plus a `CheckoutsStore`-owned debounce flag, or
|
1. OV-08: is an approximate (`20000+`) commit total acceptable for the badge contract?
|
||||||
keep the shared state machine as documentation-only?
|
|
||||||
2. OV-08: is an approximate (`20000+`) commit total acceptable for the badge contract?
|
OV-03's questions were settled during implementation: the gate keeps two flags (`running`,
|
||||||
3. OV-03: who owns the one mechanical `inbox.rs` edit — this plan or the UI effort?
|
`dirty`), `CheckoutsStore` owns `debounce_pending`, and the mechanical `inbox.rs` edit landed
|
||||||
|
in this phase.
|
||||||
|
|||||||
Reference in New Issue
Block a user