Skip to content

Comments

Lock database state globally and implement state watching#117

Merged
dknopik merged 5 commits intosigp:unstablefrom
dknopik:global-state-lock
Feb 4, 2025
Merged

Lock database state globally and implement state watching#117
dknopik merged 5 commits intosigp:unstablefrom
dknopik:global-state-lock

Conversation

@dknopik
Copy link
Member

@dknopik dknopik commented Jan 28, 2025

Issue Addressed

Before this PR, the in-memory database was a bit spooky: As operations (such as adding/removing validators) touch several DashMaps without locking the whole state, readers might read inconsistent data across these maps. Reasoning about these maps and safe orders in accessing/updating them is kind of hard, especially as the client is still evolving rapidly.

Furthermore, other parts of the client are interested in DB updates, but currently have no way of receiving them except polling regularly.

Proposed Changes

Instead of relying on multiple DashMaps and Atomic*, we now lock the whole NetworkState until we are fully done modifying it. We do this by using tokio::sync::watch, which is basically a Arc<RwLock<T>> with some extra infrastructure for task notification if the value changed. We use this notification feature in validator_store to only load new validators if the state changed, and will use it in network to figure out the subnets we want to subscribe to.

@dknopik dknopik added ready-for-review This PR is ready to be reviewed database labels Jan 28, 2025
@Zacholme7
Copy link
Member

small comments/questions, otherwise lgtm gj

@dknopik dknopik mentioned this pull request Jan 28, 2025
@dknopik dknopik merged commit 5afc8f9 into sigp:unstable Feb 4, 2025
10 checks passed
@dknopik dknopik deleted the global-state-lock branch February 5, 2025 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

database ready-for-review This PR is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants