mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
docs: cleanup
This commit is contained in:
@@ -1,373 +0,0 @@
|
||||
# State Structure Redesign Proposal v2
|
||||
|
||||
## The Core Problem
|
||||
|
||||
We need to transform:
|
||||
- **Repo Announcements** (30617) that list relays
|
||||
- **Root Events** (1617/1618/1619/1621) that tag repos
|
||||
|
||||
Into:
|
||||
- **Per-relay subscriptions**: which repos and root events to sync from each relay
|
||||
|
||||
And generate **RelayActions** when this mapping changes.
|
||||
|
||||
---
|
||||
|
||||
## Proposed Data Model
|
||||
|
||||
### 1. RepoIndex (Primary Source of Truth)
|
||||
|
||||
```rust
|
||||
/// Everything we know about repos we're tracking
|
||||
/// Key: repo addressable ref ("30617:pubkey:identifier")
|
||||
pub type RepoIndex = Arc<RwLock<HashMap<String, RepoInfo>>>;
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct RepoInfo {
|
||||
/// Relay URLs listed in the repo's announcement
|
||||
pub relays: HashSet<String>,
|
||||
/// Root event IDs that reference this repo
|
||||
pub root_events: HashSet<EventId>,
|
||||
}
|
||||
```
|
||||
|
||||
**Updated by:** Database init, batch processing of new announcements/root events
|
||||
|
||||
### 2. RelayIndex (Applied State)
|
||||
|
||||
```rust
|
||||
/// What we've told each relay to sync
|
||||
/// Key: relay URL
|
||||
pub type RelayIndex = Arc<RwLock<HashMap<String, SyncTarget>>>;
|
||||
|
||||
#[derive(Debug, Clone, Default, PartialEq)]
|
||||
pub struct SyncTarget {
|
||||
/// Repos we're syncing for this relay
|
||||
pub repos: HashSet<String>,
|
||||
/// Root events we're tracking
|
||||
pub root_events: HashSet<EventId>,
|
||||
}
|
||||
```
|
||||
|
||||
**Updated by:** SyncManager after RelayActions are applied
|
||||
|
||||
---
|
||||
|
||||
## The Transformation
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
subgraph Input
|
||||
RA[Repo Announcements]
|
||||
RE[Root Events]
|
||||
end
|
||||
|
||||
subgraph RepoIndex
|
||||
R1[repo_a: relays=X,Y events=1,2]
|
||||
R2[repo_b: relays=Y,Z events=3]
|
||||
end
|
||||
|
||||
subgraph Derived Target
|
||||
T1[relay_X: repos=a events=1,2]
|
||||
T2[relay_Y: repos=a,b events=1,2,3]
|
||||
T3[relay_Z: repos=b events=3]
|
||||
end
|
||||
|
||||
subgraph RelayIndex Applied
|
||||
A1[relay_X: repos=a events=1,2]
|
||||
A2[relay_Y: repos=a events=1,2]
|
||||
end
|
||||
|
||||
RA --> R1
|
||||
RA --> R2
|
||||
RE --> R1
|
||||
RE --> R2
|
||||
|
||||
R1 --> T1
|
||||
R1 --> T2
|
||||
R2 --> T2
|
||||
R2 --> T3
|
||||
```
|
||||
|
||||
The **diff** between Derived Target and RelayIndex produces RelayActions:
|
||||
- relay_Y needs AddFilters for repo_b and event 3
|
||||
- relay_Z needs SpawnRelay
|
||||
|
||||
---
|
||||
|
||||
## Algorithm: derive_target_from_repo_index
|
||||
|
||||
```rust
|
||||
/// Derive what we SHOULD be syncing from the repo data
|
||||
fn derive_relay_targets(repo_index: &HashMap<String, RepoInfo>) -> HashMap<String, SyncTarget> {
|
||||
let mut targets: HashMap<String, SyncTarget> = HashMap::new();
|
||||
|
||||
for (repo_ref, info) in repo_index {
|
||||
// For each relay that lists this repo
|
||||
for relay_url in &info.relays {
|
||||
let target = targets.entry(relay_url.clone()).or_default();
|
||||
target.repos.insert(repo_ref.clone());
|
||||
target.root_events.extend(info.root_events.iter().cloned());
|
||||
}
|
||||
}
|
||||
|
||||
targets
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Algorithm: process_batch
|
||||
|
||||
```rust
|
||||
async fn process_batch(&self, pending: &mut PendingUpdates) {
|
||||
// ============================================
|
||||
// STEP 1: Update RepoIndex from batch
|
||||
// ============================================
|
||||
|
||||
let mut repo_index = self.repo_index.write().await;
|
||||
|
||||
// 1a. Process root events - add to repo's root_events set
|
||||
for event in pending.root_events.drain(..) {
|
||||
for repo_ref in extract_repo_refs(&event) {
|
||||
repo_index.entry(repo_ref)
|
||||
.or_default()
|
||||
.root_events
|
||||
.insert(event.id);
|
||||
}
|
||||
}
|
||||
|
||||
// 1b. Process announcements - update repo's relay set
|
||||
for event in pending.announcements.drain(..) {
|
||||
if !lists_our_service(&event) {
|
||||
continue;
|
||||
}
|
||||
let repo_ref = build_repo_ref(&event);
|
||||
let relay_urls: HashSet<String> = extract_relay_urls(&event)
|
||||
.into_iter()
|
||||
.filter(|url| !is_own_relay(url))
|
||||
.collect();
|
||||
|
||||
// Replace relay set (handles updates that change relays)
|
||||
repo_index.entry(repo_ref)
|
||||
.or_default()
|
||||
.relays = relay_urls;
|
||||
}
|
||||
|
||||
// ============================================
|
||||
// STEP 2: Derive target state from RepoIndex
|
||||
// ============================================
|
||||
|
||||
let target = derive_relay_targets(&repo_index);
|
||||
drop(repo_index); // Release write lock
|
||||
|
||||
// ============================================
|
||||
// STEP 3: Diff target vs applied (RelayIndex)
|
||||
// ============================================
|
||||
|
||||
let applied = self.relay_index.read().await;
|
||||
let actions = compute_relay_actions(&target, &applied);
|
||||
drop(applied); // Release read lock
|
||||
|
||||
// ============================================
|
||||
// STEP 4: Send actions & update RelayIndex
|
||||
// ============================================
|
||||
|
||||
for action in actions {
|
||||
match &action {
|
||||
RelayAction::SpawnRelay { relay_url, repos_and_root_events } => {
|
||||
// Update RelayIndex with new relay
|
||||
let mut applied = self.relay_index.write().await;
|
||||
applied.insert(relay_url.clone(), SyncTarget {
|
||||
repos: repos_and_root_events.keys().cloned().collect(),
|
||||
root_events: repos_and_root_events.values()
|
||||
.flat_map(|e| e.iter().cloned())
|
||||
.collect(),
|
||||
});
|
||||
}
|
||||
RelayAction::AddFilters { relay_url, repos_and_new_root_event } => {
|
||||
// Update RelayIndex with additions
|
||||
let mut applied = self.relay_index.write().await;
|
||||
if let Some(target) = applied.get_mut(relay_url) {
|
||||
for (repo, events) in repos_and_new_root_event {
|
||||
target.repos.insert(repo.clone());
|
||||
target.root_events.extend(events.iter().cloned());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Send action to SyncManager
|
||||
let _ = self.action_tx.send(action).await;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Algorithm: compute_relay_actions
|
||||
|
||||
```rust
|
||||
fn compute_relay_actions(
|
||||
target: &HashMap<String, SyncTarget>,
|
||||
applied: &HashMap<String, SyncTarget>,
|
||||
) -> Vec<RelayAction> {
|
||||
let mut actions = Vec::new();
|
||||
|
||||
for (relay_url, target_state) in target {
|
||||
match applied.get(relay_url) {
|
||||
None => {
|
||||
// New relay - spawn it
|
||||
let mut repos_and_events = HashMap::new();
|
||||
for repo in &target_state.repos {
|
||||
// Get events for this specific repo
|
||||
let events = target_state.root_events.clone(); // simplified
|
||||
repos_and_events.insert(repo.clone(), events);
|
||||
}
|
||||
actions.push(RelayAction::SpawnRelay {
|
||||
relay_url: relay_url.clone(),
|
||||
repos_and_root_events: repos_and_events,
|
||||
});
|
||||
}
|
||||
Some(applied_state) => {
|
||||
// Existing relay - check for new repos/events
|
||||
let new_repos: HashSet<_> = target_state.repos
|
||||
.difference(&applied_state.repos)
|
||||
.cloned()
|
||||
.collect();
|
||||
let new_events: HashSet<_> = target_state.root_events
|
||||
.difference(&applied_state.root_events)
|
||||
.cloned()
|
||||
.collect();
|
||||
|
||||
if !new_repos.is_empty() || !new_events.is_empty() {
|
||||
let mut repos_and_events = HashMap::new();
|
||||
for repo in &new_repos {
|
||||
repos_and_events.insert(repo.clone(), new_events.clone());
|
||||
}
|
||||
// Also handle new events for existing repos
|
||||
if !new_events.is_empty() && new_repos.is_empty() {
|
||||
for repo in &applied_state.repos {
|
||||
repos_and_events.insert(repo.clone(), new_events.clone());
|
||||
}
|
||||
}
|
||||
|
||||
actions.push(RelayAction::AddFilters {
|
||||
relay_url: relay_url.clone(),
|
||||
repos_and_new_root_event: repos_and_events,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Future: detect relay removal (in applied but not in target)
|
||||
|
||||
actions
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Handling Announcement Updates
|
||||
|
||||
When an announcement is **updated** and changes its relay list:
|
||||
|
||||
```mermaid
|
||||
flowchart TD
|
||||
A[repo_a announcement updated] --> B[Old: relays X,Y]
|
||||
B --> C[New: relays Y,Z]
|
||||
C --> D[RepoIndex updated: repo_a.relays = Y,Z]
|
||||
D --> E[derive_relay_targets]
|
||||
E --> F[Target: X=empty, Y=repo_a, Z=repo_a]
|
||||
F --> G[Diff with Applied: X=repo_a, Y=repo_a]
|
||||
G --> H1[X: repo_a removed - future RemoveFilters]
|
||||
G --> H2[Z: new relay - SpawnRelay]
|
||||
```
|
||||
|
||||
The current RelayAction types only support growth (SpawnRelay, AddFilters). Removal would need a new `RemoveFilters` action type - this is a future enhancement.
|
||||
|
||||
---
|
||||
|
||||
## Name Mappings
|
||||
|
||||
| Current | Proposed | Semantics |
|
||||
|---------|----------|-----------|
|
||||
| `FollowingRepoRootEvents` | `RepoIndex` | Per-repo: relays + root events |
|
||||
| `SyncRelays` | `RelayIndex` | Per-relay: what we're syncing (applied state) |
|
||||
| - | `SyncTarget` | Struct for repos + events |
|
||||
| - | `RepoInfo` | Struct for relay set + event set |
|
||||
|
||||
---
|
||||
|
||||
## Data Flow Summary
|
||||
|
||||
```mermaid
|
||||
flowchart TB
|
||||
subgraph Batch Input
|
||||
RA[30617 Announcements]
|
||||
RE[Root Events 1617-1621]
|
||||
end
|
||||
|
||||
subgraph Step 1: Update Source
|
||||
RI[RepoIndex]
|
||||
end
|
||||
|
||||
subgraph Step 2: Derive Target
|
||||
DT[derive_relay_targets]
|
||||
TGT[Target HashMap]
|
||||
end
|
||||
|
||||
subgraph Step 3: Diff
|
||||
RLI[RelayIndex - Applied]
|
||||
DIFF[compute_relay_actions]
|
||||
end
|
||||
|
||||
subgraph Step 4: Apply
|
||||
ACT[RelayActions]
|
||||
SM[SyncManager]
|
||||
end
|
||||
|
||||
RA --> RI
|
||||
RE --> RI
|
||||
RI --> DT
|
||||
DT --> TGT
|
||||
TGT --> DIFF
|
||||
RLI --> DIFF
|
||||
DIFF --> ACT
|
||||
ACT --> SM
|
||||
ACT --> |update| RLI
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Changes |
|
||||
|------|---------|
|
||||
| [`src/sync/mod.rs`](src/sync/mod.rs) | Replace type aliases with RepoIndex/RelayIndex + structs |
|
||||
| [`src/sync/self_subscriber.rs`](src/sync/self_subscriber.rs) | Rewrite process_batch with new algorithm |
|
||||
|
||||
---
|
||||
|
||||
## Questions for Approval
|
||||
|
||||
1. **Naming**: Are `RepoIndex`/`RelayIndex` and `RepoInfo`/`SyncTarget` clear enough?
|
||||
|
||||
2. **When to update RelayIndex**: Should we:
|
||||
- (a) Update immediately when generating action (optimistic) ← proposed above
|
||||
- (b) Update only after SyncManager confirms action succeeded
|
||||
|
||||
3. **Bootstrap relay**: Keep special-casing it in RelayIndex (always present)?
|
||||
|
||||
4. **Future work**: Add `RemoveFilters` action for relay removal, or defer?
|
||||
|
||||
---
|
||||
|
||||
## Benefits
|
||||
|
||||
1. **Logical flow**: Source → Derived → Diff → Actions
|
||||
2. **Single source of truth**: RepoIndex is the authoritative data
|
||||
3. **Clear transformation**: `derive_relay_targets()` is a pure function
|
||||
4. **Handles updates**: Replacing `repo.relays` naturally handles announcement changes
|
||||
5. **Testable**: Each step can be unit tested independently
|
||||
@@ -1,159 +0,0 @@
|
||||
# Phase 1: Sync Test Baseline
|
||||
|
||||
**Timestamp:** 2025-12-18T16:50:07Z (UTC)
|
||||
**Git Commit:** (pre-refactoring baseline)
|
||||
|
||||
## Test Execution Command
|
||||
```bash
|
||||
cargo test --test sync
|
||||
```
|
||||
|
||||
## Summary Statistics
|
||||
|
||||
- **Total Tests:** 40
|
||||
- **Passed:** 38 (95%)
|
||||
- **Failed:** 2 (5%)
|
||||
- **Ignored:** 0
|
||||
- **Filtered Out:** 0
|
||||
- **Execution Time:** 8.05s
|
||||
|
||||
## Passing Tests (38)
|
||||
|
||||
### Common Module Tests (7)
|
||||
- `common::relay::tests::test_find_free_port`
|
||||
- `common::sync_helpers::tests::test_parse_empty_metrics`
|
||||
- `common::sync_helpers::tests::test_parse_counter_with_labels`
|
||||
- `common::sync_helpers::tests::test_parse_gauge_without_labels`
|
||||
- `common::sync_helpers::tests::test_parse_metric_with_relay_url_label`
|
||||
- `common::sync_helpers::tests::test_repo_coord_format`
|
||||
- `common::sync_helpers::tests::test_build_layer3_comment_with_uppercase_e`
|
||||
|
||||
### Sync Helper Builder Tests (6)
|
||||
- `common::sync_helpers::tests::test_build_layer3_comment_kind_1`
|
||||
- `common::sync_helpers::tests::test_build_layer3_quote_with_q`
|
||||
- `common::sync_helpers::tests::test_build_layer3_comment_kind_1111`
|
||||
- `common::sync_helpers::tests::test_build_layer2_issue_event`
|
||||
- `common::sync_helpers::tests::test_build_layer3_reply_with_e_tag`
|
||||
- `common::sync_helpers::tests::test_build_layer2_issue_with_uppercase_a`
|
||||
- `common::sync_helpers::tests::test_build_layer2_issue_with_q_tag`
|
||||
|
||||
### Metrics Tests (6 passing)
|
||||
- `sync::metrics::test_metric_values_are_numeric`
|
||||
- `sync::metrics::test_concurrent_metrics_requests`
|
||||
- `sync::metrics::test_metrics_availability_during_sync`
|
||||
- `sync::metrics::test_connection_failure_increments_counter`
|
||||
- `sync::metrics::test_prometheus_format_valid`
|
||||
- `sync::metrics::test_relay_connected_status`
|
||||
- `sync::metrics::test_health_state_degrades_on_failure`
|
||||
- `sync::metrics::test_startup_sync_event_count`
|
||||
|
||||
### Live Sync Tests (3)
|
||||
- `sync::live_sync::test_live_sync_layer2_events`
|
||||
- `sync::live_sync::test_live_sync_layer3_events`
|
||||
- `sync::live_sync::test_live_sync_event_ordering`
|
||||
|
||||
### Bootstrap Tests (3)
|
||||
- `sync::bootstrap::test_announcement_not_listing_relay_is_not_synced`
|
||||
- `sync::bootstrap::test_history_sync_without_negentropy`
|
||||
- `sync::bootstrap::test_bootstrap_syncs_existing_layer2_events`
|
||||
- `sync::bootstrap::test_relay_replays_events_after_restart`
|
||||
|
||||
### Discovery Tests (3)
|
||||
- `sync::discovery::test_layer2_discovery_with_chain`
|
||||
- `sync::discovery::test_discovers_layer3_via_layer2`
|
||||
- `sync::discovery::test_recursive_relay_discovery_syncs_announcement`
|
||||
|
||||
### Tag Variations Tests (6)
|
||||
- `sync::tag_variations::test_layer2_sync_with_lowercase_a_tag`
|
||||
- `sync::tag_variations::test_layer2_sync_with_q_tag`
|
||||
- `sync::tag_variations::test_layer2_sync_with_uppercase_a_tag`
|
||||
- `sync::tag_variations::test_layer3_sync_with_lowercase_e_tag`
|
||||
- `sync::tag_variations::test_layer3_sync_with_q_tag`
|
||||
- `sync::tag_variations::test_layer3_sync_with_uppercase_e_tag`
|
||||
|
||||
## Failing Tests (2)
|
||||
|
||||
### 1. sync::metrics::test_live_sync_event_count
|
||||
|
||||
**Location:** `tests/sync/metrics.rs:444`
|
||||
|
||||
**Error Type:** Assertion failure
|
||||
|
||||
**Details:**
|
||||
```
|
||||
assertion `left == right` failed: Should have 2 live events
|
||||
left: None
|
||||
right: Some(2)
|
||||
```
|
||||
|
||||
**Root Cause:** Live event counting metric is not being populated. The metric parser is returning `None` when it should find a count of 2 live synced events.
|
||||
|
||||
**Output Sample:**
|
||||
```
|
||||
Live events synced: None
|
||||
```
|
||||
|
||||
**Impact:** This suggests that the `sync_events_total{sync_type="live"}` metric either:
|
||||
- Is not being incremented correctly during live sync
|
||||
- Is using a different metric name/label than expected
|
||||
- Is not being exposed in the metrics endpoint
|
||||
|
||||
---
|
||||
|
||||
### 2. sync::metrics::test_multi_source_aggregate_counts
|
||||
|
||||
**Location:** `tests/sync/metrics.rs:603`
|
||||
|
||||
**Error Type:** Assertion failure
|
||||
|
||||
**Details:**
|
||||
```
|
||||
assertion `left == right` failed: Should have 0 connected
|
||||
left: Some(1)
|
||||
right: Some(0)
|
||||
```
|
||||
|
||||
**Root Cause:** After stopping a relay connection, the `sync_relays_connected_total` metric is not being decremented. The test expects 0 connected relays after calling stop, but the metric still shows 1.
|
||||
|
||||
**Output Sample:**
|
||||
```
|
||||
Tracked total: Some(1)
|
||||
Connected total: Some(1)
|
||||
After stop - Tracked total: Some(1)
|
||||
After stop - Connected total: Some(1)
|
||||
```
|
||||
|
||||
**Impact:** This indicates that relay disconnection is not properly updating the connection count metric. This could be:
|
||||
- A lifecycle issue where the metric update happens asynchronously after the test assertion
|
||||
- A bug where the disconnect handler doesn't decrement the counter
|
||||
- A race condition in the test timing
|
||||
|
||||
---
|
||||
|
||||
## Analysis
|
||||
|
||||
### Test Health
|
||||
The sync test suite is in relatively good shape with a 95% pass rate. The failures are both isolated to the metrics module and appear to be either timing/synchronization issues or metric collection bugs rather than fundamental sync logic problems.
|
||||
|
||||
### Pre-existing Issues
|
||||
Both failing tests appear to be pre-existing issues unrelated to the planned refactoring work. They should be tracked separately and not conflated with any issues introduced during the refactor.
|
||||
|
||||
### Refactoring Risk Assessment
|
||||
- **Low Risk Areas:** Bootstrap, discovery, live_sync, tag_variations modules are all passing
|
||||
- **Medium Risk Area:** Metrics tests have 2 failures, but they're specific to metric collection, not sync functionality
|
||||
- **Safe to Refactor:** The core sync logic tests are passing, so structural refactoring of test helpers and organization should not affect test outcomes
|
||||
|
||||
## Next Steps
|
||||
|
||||
This baseline will be used to:
|
||||
1. Verify that refactoring doesn't introduce new failures
|
||||
2. Distinguish pre-existing failures from regressions
|
||||
3. Track if the refactoring inadvertently fixes the existing failures
|
||||
4. Ensure that after refactoring, we still have 38 passing tests (or more if we fix the failing ones)
|
||||
|
||||
## Notes
|
||||
|
||||
- Both failures are in `tests/sync/metrics.rs`
|
||||
- The failures appear to be metric collection/timing issues rather than sync logic bugs
|
||||
- All functional sync tests (bootstrap, discovery, live_sync, tag_variations) are passing
|
||||
- The refactoring should not affect these test results unless we accidentally change metric collection timing
|
||||
Reference in New Issue
Block a user