Clean exit when no space left on device (#6339)

Fixes #6305

substrate/Cargo.lock

@@ -7654,6 +7654,7 @@ dependencies = [
  "parking_lot 0.10.2",
  "sp-block-builder",
  "sp-consensus",
+ "sp-database",
  "sp-runtime",
  "sp-state-machine",
 ]

substrate/client/api/src/leaves.rs

@@ -314,7 +314,7 @@ mod tests {
 		let mut tx = Transaction::new();
 
 		set.prepare_transaction(&mut tx, 0, PREFIX);
-		db.commit(tx);
+		db.commit(tx).unwrap();
 
 		let set2 = LeafSet::read_from_db(&*db, 0, PREFIX).unwrap();
 		assert_eq!(set, set2);
@@ -348,12 +348,12 @@ mod tests {
 
 		let mut tx = Transaction::new();
 		set.prepare_transaction(&mut tx, 0, PREFIX);
-		db.commit(tx);
+		db.commit(tx).unwrap();
 
 		let _ = set.finalize_height(11);
 		let mut tx = Transaction::new();
 		set.prepare_transaction(&mut tx, 0, PREFIX);
-		db.commit(tx);
+		db.commit(tx).unwrap();
 
 		assert!(set.contains(11, 11_1));
 		assert!(set.contains(11, 11_2));

substrate/client/db/src/cache/mod.rs

@@ -342,8 +342,9 @@ impl<Block: BlockT> BlockchainCache<Block> for DbCacheSync<Block> {
 			EntryType::Genesis,
 		)?;
 		let tx_ops = tx.into_ops();
-		db.commit(dbtx);
+		db.commit(dbtx)?;
 		cache.commit(tx_ops)?;
+
 		Ok(())
 	}
 

substrate/client/db/src/changes_tries_storage.rs

@@ -719,7 +719,7 @@ mod tests {
 				None,
 				None,
 			).unwrap();
-			backend.storage.db.commit(tx);
+			backend.storage.db.commit(tx).unwrap();
 			backend.changes_tries_storage.post_commit(Some(cache_ops));
 		};
 

substrate/client/db/src/children.rs

@@ -99,7 +99,7 @@ mod tests {
 		children2.push(1_6);
 		write_children(&mut tx, 0, PREFIX, 1_2, children2);
 
-		db.commit(tx.clone());
+		db.commit(tx.clone()).unwrap();
 
 		let r1: Vec<u32> = read_children(&*db, 0, PREFIX, 1_1).expect("(1) Getting r1 failed");
 		let r2: Vec<u32> = read_children(&*db, 0, PREFIX, 1_2).expect("(1) Getting r2 failed");
@@ -108,7 +108,7 @@ mod tests {
 		assert_eq!(r2, vec![1_4, 1_6]);
 
 		remove_children(&mut tx, 0, PREFIX, 1_2);
-		db.commit(tx);
+		db.commit(tx).unwrap();
 
 		let r1: Vec<u32> = read_children(&*db, 0, PREFIX, 1_1).expect("(2) Getting r1 failed");
 		let r2: Vec<u32> = read_children(&*db, 0, PREFIX, 1_2).expect("(2) Getting r2 failed");

substrate/client/db/src/lib.rs

@@ -1243,7 +1243,7 @@ impl<Block: BlockT> Backend<Block> {
 			None
 		};
 
-		self.storage.db.commit(transaction);
+		self.storage.db.commit(transaction)?;
 
 		if let Some((
 			number,
@@ -1356,7 +1356,7 @@ impl<Block> sc_client_api::backend::AuxStore for Backend<Block> where Block: Blo
 		for k in delete {
 			transaction.remove(columns::AUX, k);
 		}
-		self.storage.db.commit(transaction);
+		self.storage.db.commit(transaction)?;
 		Ok(())
 	}
 
@@ -1438,7 +1438,7 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
 			&mut changes_trie_cache_ops,
 			&mut displaced,
 		)?;
-		self.storage.db.commit(transaction);
+		self.storage.db.commit(transaction)?;
 		self.blockchain.update_meta(hash, number, is_best, is_finalized);
 		self.changes_tries_storage.post_commit(changes_trie_cache_ops);
 		Ok(())
@@ -1536,7 +1536,7 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
 						transaction.set_from_vec(columns::META, meta_keys::BEST_BLOCK, key);
 						transaction.remove(columns::KEY_LOOKUP, removed.hash().as_ref());
 						children::remove_children(&mut transaction, columns::META, meta_keys::CHILDREN_PREFIX, best_hash);
-						self.storage.db.commit(transaction);
+						self.storage.db.commit(transaction)?;
 						self.changes_tries_storage.post_commit(Some(changes_trie_cache_ops));
 						self.blockchain.update_meta(best_hash, best_number, true, update_finalized);
 					}
@@ -1555,7 +1555,7 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
 
 			leaves.revert(best_hash, best_number);
 			leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX);
-			self.storage.db.commit(transaction);
+			self.storage.db.commit(transaction)?;
 
 			Ok(())
 		};

substrate/client/db/src/light.rs

@@ -402,7 +402,8 @@ impl<Block> AuxStore for LightStorage<Block>
 		for k in delete {
 			transaction.remove(columns::AUX, k);
 		}
-		self.db.commit(transaction);
+		self.db.commit(transaction)?;
+
 		Ok(())
 	}
 
@@ -495,7 +496,7 @@ impl<Block> Storage<Block> for LightStorage<Block>
 
 			debug!("Light DB Commit {:?} ({})", hash, number);
 
-			self.db.commit(transaction);
+			self.db.commit(transaction)?;
 			cache.commit(cache_ops)
 				.expect("only fails if cache with given name isn't loaded yet;\
 						cache is already loaded because there are cache_ops; qed");
@@ -513,8 +514,9 @@ impl<Block> Storage<Block> for LightStorage<Block>
 
 			let mut transaction = Transaction::new();
 			self.set_head_with_transaction(&mut transaction, hash.clone(), (number.clone(), hash.clone()))?;
-			self.db.commit(transaction);
+			self.db.commit(transaction)?;
 			self.update_meta(hash, header.number().clone(), true, false);
+
 			Ok(())
 		} else {
 			Err(ClientError::UnknownBlock(format!("Cannot set head {:?}", id)))
@@ -552,7 +554,7 @@ impl<Block> Storage<Block> for LightStorage<Block>
 					)?
 					.into_ops();
 
-				self.db.commit(transaction);
+				self.db.commit(transaction)?;
 				cache.commit(cache_ops)
 					.expect("only fails if cache with given name isn't loaded yet;\
 							cache is already loaded because there are cache_ops; qed");

substrate/client/db/src/offchain.rs

@@ -25,6 +25,7 @@ use std::{
 
 use crate::{columns, Database, DbHash, Transaction};
 use parking_lot::Mutex;
+use log::error;
 
 /// Offchain local storage
 #[derive(Clone)]
@@ -64,7 +65,9 @@ impl sp_core::offchain::OffchainStorage for LocalStorage {
 		let mut tx = Transaction::new();
 		tx.set(columns::OFFCHAIN, &key, value);
 
-		self.db.commit(tx);
+		if let Err(err) = self.db.commit(tx) {
+			error!("Error setting on local storage: {}", err)
+		}
 	}
 
 	fn remove(&mut self, prefix: &[u8], key: &[u8]) {
@@ -72,7 +75,9 @@ impl sp_core::offchain::OffchainStorage for LocalStorage {
 		let mut tx = Transaction::new();
 		tx.remove(columns::OFFCHAIN, &key);
 
-		self.db.commit(tx);
+		if let Err(err) = self.db.commit(tx) {
+			error!("Error removing on local storage: {}", err)
+		}
 	}
 
 	fn get(&self, prefix: &[u8], key: &[u8]) -> Option<Vec<u8>> {

substrate/client/db/src/parity_db.rs

@@ -17,7 +17,7 @@
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 /// A `Database` adapter for parity-db.
 
-use sp_database::{Database, Change, Transaction, ColumnId};
+use sp_database::{Database, Change, ColumnId, Transaction, error::DatabaseError};
 use crate::utils::NUM_COLUMNS;
 use crate::columns;
 
@@ -44,7 +44,7 @@ pub fn open<H: Clone>(path: &std::path::Path) -> parity_db::Result<std::sync::Ar
 }
 
 impl<H: Clone> Database<H> for DbAdapter {
-	fn commit(&self, transaction: Transaction<H>) {
+	fn commit(&self, transaction: Transaction<H>) -> Result<(), DatabaseError> {
 		handle_err(self.0.commit(transaction.0.into_iter().map(|change|
 			match change {
 				Change::Set(col, key, value) => (col as u8, key, Some(value)),
@@ -52,6 +52,8 @@ impl<H: Clone> Database<H> for DbAdapter {
 				_ => unimplemented!(),
 			}))
 		);
+
+		Ok(())
 	}
 
 	fn get(&self, col: ColumnId, key: &[u8]) -> Option<Vec<u8>> {

substrate/client/db/src/utils.rs

@@ -297,7 +297,7 @@ pub fn check_database_type(db: &dyn Database<DbHash>, db_type: DatabaseType) ->
 		None => {
 			let mut transaction = Transaction::new();
 			transaction.set(COLUMN_META, meta_keys::TYPE, db_type.as_str().as_bytes());
-			db.commit(transaction)
+			db.commit(transaction)?;
 		},
 	}
 

substrate/primitives/blockchain/Cargo.toml

@@ -22,3 +22,4 @@ sp-consensus = { version = "0.8.0-rc4", path = "../consensus/common" }
 sp-runtime = { version = "2.0.0-rc4", path = "../runtime" }
 sp-block-builder = { version = "2.0.0-rc4", path = "../block-builder" }
 sp-state-machine = { version = "0.8.0-rc4", path = "../state-machine" }
+sp-database = { version = "2.0.0-rc4", path = "../database" }

substrate/primitives/blockchain/src/error.rs

@@ -130,6 +130,8 @@ pub enum Error {
 	IncompletePipeline,
 	#[display(fmt = "Transaction pool not ready for block production.")]
 	TransactionPoolNotReady,
+	#[display(fmt = "Database: {}", _0)]
+	DatabaseError(sp_database::error::DatabaseError),
 	/// A convenience variant for String
 	#[display(fmt = "{}", _0)]
 	Msg(String),

substrate/primitives/database/src/error.rs

@@ -0,0 +1,35 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2020 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+/// The error type for database operations.
+#[derive(Debug)]
+pub struct DatabaseError(pub Box<dyn std::error::Error + Send>);
+
+impl std::fmt::Display for DatabaseError {
+	fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+		write!(f, "{}", self.0)
+	}
+}
+
+impl std::error::Error for DatabaseError {
+	fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+		None
+	}
+}
+
+/// A specialized `Result` type for database operations.
+pub type Result<T> = std::result::Result<T, DatabaseError>;

substrate/primitives/database/src/kvdb.rs

@@ -19,7 +19,7 @@
 
 use ::kvdb::{DBTransaction, KeyValueDB};
 
-use crate::{Database, Change, Transaction, ColumnId};
+use crate::{Database, Change, ColumnId, Transaction, error};
 
 struct DbAdapter<D: KeyValueDB + 'static>(D);
 
@@ -38,7 +38,7 @@ pub fn as_database<D: KeyValueDB + 'static, H: Clone>(db: D) -> std::sync::Arc<d
 }
 
 impl<D: KeyValueDB, H: Clone> Database<H> for DbAdapter<D> {
-	fn commit(&self, transaction: Transaction<H>) {
+	fn commit(&self, transaction: Transaction<H>) -> error::Result<()> {
 		let mut tx = DBTransaction::new();
 		for change in transaction.0.into_iter() {
 			match change {
@@ -47,7 +47,7 @@ impl<D: KeyValueDB, H: Clone> Database<H> for DbAdapter<D> {
 				_ => unimplemented!(),
 			}
 		}
-		handle_err(self.0.write(tx));
+		self.0.write(tx).map_err(|e| error::DatabaseError(Box::new(e)))
 	}
 
 	fn get(&self, col: ColumnId, key: &[u8]) -> Option<Vec<u8>> {

substrate/primitives/database/src/lib.rs

@@ -17,6 +17,7 @@
 
 //! The main database trait, allowing Substrate to store data persistently.
 
+pub mod error;
 mod mem;
 mod kvdb;
 
@@ -82,20 +83,22 @@ impl<H> Transaction<H> {
 pub trait Database<H: Clone>: Send + Sync {
 	/// Commit the `transaction` to the database atomically. Any further calls to `get` or `lookup`
 	/// will reflect the new state.
-	fn commit(&self, transaction: Transaction<H>) {
+	fn commit(&self, transaction: Transaction<H>) -> error::Result<()> {
 		for change in transaction.0.into_iter() {
 			match change {
 				Change::Set(col, key, value) => self.set(col, &key, &value),
 				Change::Remove(col, key) => self.remove(col, &key),
 				Change::Store(hash, preimage) => self.store(&hash, &preimage),
 				Change::Release(hash) => self.release(&hash),
-			}
+			}?;
 		}
+
+		Ok(())
 	}
 
 	/// Commit the `transaction` to the database atomically. Any further calls to `get` or `lookup`
 	/// will reflect the new state.
-	fn commit_ref<'a>(&self, transaction: &mut dyn Iterator<Item=ChangeRef<'a, H>>) {
+	fn commit_ref<'a>(&self, transaction: &mut dyn Iterator<Item=ChangeRef<'a, H>>) -> error::Result<()> {
 		let mut tx = Transaction::new();
 		for change in transaction {
 			match change {
@@ -105,13 +108,13 @@ pub trait Database<H: Clone>: Send + Sync {
 				ChangeRef::Release(hash) => tx.release(hash),
 			}
 		}
-		self.commit(tx);
+		self.commit(tx)
 	}
 
 	/// Retrieve the value previously stored against `key` or `None` if
 	/// `key` is not currently in the database.
 	fn get(&self, col: ColumnId, key: &[u8]) -> Option<Vec<u8>>;
-	
+
 	/// Call `f` with the value previously stored against `key`.
 	///
 	/// This may be faster than `get` since it doesn't allocate.
@@ -119,24 +122,24 @@ pub trait Database<H: Clone>: Send + Sync {
 	fn with_get(&self, col: ColumnId, key: &[u8], f: &mut dyn FnMut(&[u8])) {
 		self.get(col, key).map(|v| f(&v));
 	}
-	
+
 	/// Set the value of `key` in `col` to `value`, replacing anything that is there currently.
-	fn set(&self, col: ColumnId, key: &[u8], value: &[u8]) {
+	fn set(&self, col: ColumnId, key: &[u8], value: &[u8]) -> error::Result<()> {
 		let mut t = Transaction::new();
 		t.set(col, key, value);
-		self.commit(t);
+		self.commit(t)
 	}
 	/// Remove the value of `key` in `col`.
-	fn remove(&self, col: ColumnId, key: &[u8]) {
+	fn remove(&self, col: ColumnId, key: &[u8]) -> error::Result<()> {
 		let mut t = Transaction::new();
 		t.remove(col, key);
-		self.commit(t);
+		self.commit(t)
 	}
 
 	/// Retrieve the first preimage previously `store`d for `hash` or `None` if no preimage is
 	/// currently stored.
 	fn lookup(&self, hash: &H) -> Option<Vec<u8>>;
-	
+
 	/// Call `f` with the preimage stored for `hash` and return the result, or `None` if no preimage
 	/// is currently stored.
 	///
@@ -145,23 +148,23 @@ pub trait Database<H: Clone>: Send + Sync {
 	fn with_lookup(&self, hash: &H, f: &mut dyn FnMut(&[u8])) {
 		self.lookup(hash).map(|v| f(&v));
 	}
-	
+
 	/// Store the `preimage` of `hash` into the database, so that it may be looked up later with
 	/// `Database::lookup`. This may be called multiple times, but `Database::lookup` but subsequent
 	/// calls will ignore `preimage` and simply increase the number of references on `hash`.
-	fn store(&self, hash: &H, preimage: &[u8]) {
+	fn store(&self, hash: &H, preimage: &[u8]) -> error::Result<()> {
 		let mut t = Transaction::new();
 		t.store(hash.clone(), preimage);
-		self.commit(t);
+		self.commit(t)
 	}
-	
+
 	/// Release the preimage of `hash` from the database. An equal number of these to the number of
 	/// corresponding `store`s must have been given before it is legal for `Database::lookup` to
 	/// be unable to provide the preimage.
-	fn release(&self, hash: &H) {
+	fn release(&self, hash: &H) -> error::Result<()> {
 		let mut t = Transaction::new();
 		t.release(hash.clone());
-		self.commit(t);
+		self.commit(t)
 	}
 }
 

substrate/primitives/database/src/mem.rs

@@ -18,7 +18,7 @@
 //! In-memory implementation of `Database`
 
 use std::collections::HashMap;
-use crate::{Database, Transaction, ColumnId, Change};
+use crate::{Database, Change, ColumnId, Transaction, error};
 use parking_lot::RwLock;
 
 #[derive(Default)]
@@ -29,7 +29,7 @@ pub struct MemDb<H: Clone + Send + Sync + Eq + PartialEq + Default + std::hash::
 impl<H> Database<H> for MemDb<H>
 	where H: Clone + Send + Sync + Eq + PartialEq + Default + std::hash::Hash
 {
-	fn commit(&self, transaction: Transaction<H>) {
+	fn commit(&self, transaction: Transaction<H>) -> error::Result<()> {
 		let mut s = self.0.write();
 		for change in transaction.0.into_iter() {
 			match change {
@@ -39,6 +39,8 @@ impl<H> Database<H> for MemDb<H>
 				Change::Release(hash) => { s.1.remove(&hash); },
 			}
 		}
+
+		Ok(())
 	}
 
 	fn get(&self, col: ColumnId, key: &[u8]) -> Option<Vec<u8>> {