Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions prdoc/pr_9868.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
title: Fix the deadlock during statements gossiping
doc:
- audience: Node Dev
description: |-
During high load the statement store experiences deadlock-like behavior: every second statements were gossiping, locking the index which possibly caused the deadlock. After the fix, the observed behavior no longer occurs.

crates:
- name: sc-statement-store
bump: patch
25 changes: 12 additions & 13 deletions substrate/client/statement-store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,12 +488,7 @@ impl Store {
where
Block: BlockT,
Block::Hash: From<BlockHash>,
Client: ProvideRuntimeApi<Block>
+ HeaderBackend<Block>
+ sc_client_api::ExecutorProvider<Block>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to the subject, but should be removed as we don't need this trait. It only complicates the test setup.

+ Send
+ Sync
+ 'static,
Client: ProvideRuntimeApi<Block> + HeaderBackend<Block> + Send + Sync + 'static,
Client::Api: ValidateStatement<Block>,
{
let store = Arc::new(Self::new(path, options, client, keystore, prometheus)?);
Expand Down Expand Up @@ -671,21 +666,25 @@ impl Store {
/// Perform periodic store maintenance
pub fn maintain(&self) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to the current deadlock, but better to remove unnecessary reads. It makes the log more precise as the change of index between maintenance and logging is possible. Keeping the write lock during the DB commit is not necessary.

log::trace!(target: LOG_TARGET, "Started store maintenance");
let deleted = self.index.write().maintain(self.timestamp());
let (deleted, active_count, expired_count): (Vec<_>, usize, usize) = {
let mut index = self.index.write();
let deleted = index.maintain(self.timestamp());
(deleted, index.entries.len(), index.expired.len())
};
let deleted: Vec<_> =
deleted.into_iter().map(|hash| (col::EXPIRED, hash.to_vec(), None)).collect();
let count = deleted.len() as u64;
let deleted_count = deleted.len() as u64;
if let Err(e) = self.db.commit(deleted) {
log::warn!(target: LOG_TARGET, "Error writing to the statement database: {:?}", e);
} else {
self.metrics.report(|metrics| metrics.statements_pruned.inc_by(count));
self.metrics.report(|metrics| metrics.statements_pruned.inc_by(deleted_count));
}
log::trace!(
target: LOG_TARGET,
"Completed store maintenance. Purged: {}, Active: {}, Expired: {}",
count,
self.index.read().entries.len(),
self.index.read().expired.len()
deleted_count,
active_count,
expired_count
);
}

Expand Down Expand Up @@ -764,7 +763,7 @@ impl StatementStore for Store {
fn statements(&self) -> Result<Vec<(Hash, Statement)>> {
let index = self.index.read();
let mut result = Vec::with_capacity(index.entries.len());
for h in self.index.read().entries.keys() {
for h in index.entries.keys() {
let encoded = self.db.get(col::STATEMENTS, h).map_err(|e| Error::Db(e.to_string()))?;
if let Some(encoded) = encoded {
if let Ok(statement) = Statement::decode(&mut encoded.as_slice()) {
Expand Down
Loading