Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use sandbox folder for txhashset validation on state sync #2685

Merged
merged 11 commits into from
Mar 28, 2019
Merged

use sandbox folder for txhashset validation on state sync #2685

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
15825f5
use sandbox folder for txhashset validation on state sync
garyyu Mar 18, 2019
5533bc1
rustfmt
garyyu Mar 18, 2019
dd7af4a
use temp directory as the sandbox instead actual db_root txhashset dir
garyyu Mar 22, 2019
5851288
rustfmt
garyyu Mar 22, 2019
61b08f1
move txhashset overwrite to the end of full validation
garyyu Mar 22, 2019
5db4341
fix travis-ci test
garyyu Mar 22, 2019
eb4adef
rustfmt
garyyu Mar 22, 2019
2a6182b
fix: hashset have 2 folders including txhashset and header
garyyu Mar 23, 2019
6dcd64a
rustfmt
garyyu Mar 23, 2019
87a927d
(1)switch to rebuild_header_mmr instead of copy the sandbox header mm…
garyyu Mar 23, 2019
7213ac9
minor improve on sandbox_dir
garyyu Mar 23, 2019
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
62 changes: 31 additions & 31 deletions chain/src/chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ use crate::util::secp::pedersen::{Commitment, RangeProof};
use crate::util::{Mutex, RwLock, StopState};
use grin_store::Error::NotFoundErr;
use std::collections::HashMap;
use std::env;
use std::fs::File;
use std::path::PathBuf;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;
use std::time::{Duration, Instant};
Expand Down Expand Up @@ -181,7 +183,7 @@ impl Chain {

// open the txhashset, creating a new one if necessary
let mut txhashset =
txhashset::TxHashSet::open(db_root.clone(), store.clone(), None, false)?;
txhashset::TxHashSet::open(db_root.clone(), store.clone(), None, None)?;

setup_head(&genesis, &store, &mut txhashset)?;
Chain::log_heads(&store)?;
Expand Down Expand Up @@ -845,7 +847,7 @@ impl Chain {

/// Clean the temporary sandbox folder
pub fn clean_txhashset_sandbox(&self) {
txhashset::clean_txhashset_sandbox(self.db_root.clone());
txhashset::clean_txhashset_folder(&env::temp_dir());
}

/// Writes a reading view on a txhashset state that's been provided to us.
Expand All @@ -869,20 +871,20 @@ impl Chain {

let header = self.get_block_header(&h)?;

// Write hashset to sandbox
txhashset::clean_txhashset_sandbox(self.db_root.clone());
txhashset::zip_write(
self.db_root.clone(),
txhashset_data.try_clone()?,
&header,
true,
)?;
// Write txhashset to sandbox
txhashset::clean_txhashset_folder(&env::temp_dir());
txhashset::zip_write(env::temp_dir(), txhashset_data.try_clone()?, &header)?;

let mut txhashset = txhashset::TxHashSet::open(
self.db_root.clone(),
self.store.clone(),
Some(&header),
true,
Some(
env::temp_dir()
.to_str()
.expect("invalid temp folder")
.to_owned(),
),
)?;

// The txhashset.zip contains the output, rangeproof and kernel MMRs.
Expand All @@ -892,26 +894,6 @@ impl Chain {
// Validate the full kernel history (kernel MMR root for every block header).
self.validate_kernel_history(&header, &txhashset)?;

// sandbox validation ok, go to overwrite txhashset and clean the sandbox
{
// Before overwriting, drop file handles in underlying txhashset
{
let mut txhashset_ref = self.txhashset.write();
txhashset_ref.release_backend_files();
}

txhashset::zip_write(self.db_root.clone(), txhashset_data, &header, false)?;

txhashset = txhashset::TxHashSet::open(
self.db_root.clone(),
self.store.clone(),
Some(&header),
false,
)?;

txhashset::clean_txhashset_sandbox(self.db_root.clone());
}

// all good, prepare a new batch and update all the required records
debug!("txhashset_write: rewinding a 2nd time (writeable)");

Expand Down Expand Up @@ -955,6 +937,24 @@ impl Chain {

debug!("txhashset_write: finished committing the batch (head etc.)");

// sandbox full validation ok, go to overwrite txhashset on db root
{
// Before overwriting, drop file handles in underlying txhashset
{
let mut txhashset_ref = self.txhashset.write();
txhashset_ref.release_backend_files();
}

txhashset::txhashset_replace(env::temp_dir(), PathBuf::from(self.db_root.clone()))?;

txhashset = txhashset::TxHashSet::open(
self.db_root.clone(),
self.store.clone(),
Some(&header),
None,
)?;
}

// Replace the chain txhashset with the newly built one.
{
let mut txhashset_ref = self.txhashset.write();
Expand Down
59 changes: 36 additions & 23 deletions chain/src/txhashset/txhashset.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ use std::time::{Instant, SystemTime, UNIX_EPOCH};

const HEADERHASHSET_SUBDIR: &'static str = "header";
const TXHASHSET_SUBDIR: &'static str = "txhashset";
const TXHASHSET_SANDBOX_SUBDIR: &'static str = "txhashset_sandbox";

const HEADER_HEAD_SUBDIR: &'static str = "header_head";
const SYNC_HEAD_SUBDIR: &'static str = "sync_head";
Expand Down Expand Up @@ -115,12 +114,12 @@ impl TxHashSet {
root_dir: String,
commit_index: Arc<ChainStore>,
header: Option<&BlockHeader>,
sandbox: bool,
sandbox_dir: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

We can just pass either root_dir or sandbox_dir as the first param and we don't need to handle the precedence rule here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @antiochp for the review.

You have header/header_head and header/sync_head/ reading on this open() function, but header/ is at same level as txhashset/ so surely it doesn't exist in sandbox folder.

That's why I keep this first root_dir param.

Could you explain more about why we need put header\ also into the sandbox folder?

			header_pmmr_h: PMMRHandle::new(
				&root_dir,
				HEADERHASHSET_SUBDIR,
				HEADER_HEAD_SUBDIR,
				false,
				None,
			)?,
			sync_pmmr_h: PMMRHandle::new(
				&root_dir,
				HEADERHASHSET_SUBDIR,
				SYNC_HEAD_SUBDIR,
				false,
				None,
			)?,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, perhaps that rebuild did the job :-)

		self.rebuild_header_mmr(&Tip::from_header(&header), &mut txhashset)?;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but then what's the best to handle this header/ folder in sandbox?

  1. also move to the db root
  2. call a rebuild_header_mmr() again after moving the txhashset/ folder

Which one do you prefer? Is the rebuild_header_mmr() a fast or slow processing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried with moving the header/ folder, but see a lot of Corrupted storage log which could be caused by not-atomic processing on store during txhashset_write processing?

Switch to rebuild_header_mmr() solution now.
And regarding the speed, the rebuilding spent 10 seconds on my slow mac air for current main chain height. not bad.

Copy link
Member

@antiochp antiochp Mar 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Ahh ok.

chain_data/txhashset <- txhashset.zip unzips to here
chain_data/header    <- local data only, not included in the zip file (rebuilt from headers from db)

The original intent was to keep the header dir out the txhashset dir to make it more explicit what originated from txhashset.zip and what was purely built locally.

Maybe we consider making the sandbox dir a couple of levels deep?

So we can have something like -

chain_data/txhashset
chain_data/header

$sandbox_dir/txhashset
$sandbox_dir/header

Would this work?

I don't see any reasons why we would not be able to copy across both the txhashset dir and the header dir on successful validation.
A txhashset will have file references to files under both these dirs so we'd need to be careful with ordering.
i.e. copy them across then reopen the newly updated txhashset.

Edit: rebuild_header_mmr would work but we should not need to do this. Its not based on data from txhashset.zip so is effectively wasted effort as we simply read the headers from the db and rebuild the header MMR a second time this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@antiochp I don't know why, I tried the header dir moving on 2a6182b, but there were a lot of Corrupted storage logs. So I switch to rebuild_header_mmr solution.

) -> Result<TxHashSet, Error> {
let txhashset_subdir = if sandbox {
TXHASHSET_SANDBOX_SUBDIR
let txhashset_rootdir = if let Some(sandbox) = sandbox_dir {
sandbox.clone()
} else {
TXHASHSET_SUBDIR
root_dir.clone()
};
Ok(TxHashSet {
header_pmmr_h: PMMRHandle::new(
Expand All @@ -138,22 +137,22 @@ impl TxHashSet {
None,
)?,
output_pmmr_h: PMMRHandle::new(
&root_dir,
txhashset_subdir,
&txhashset_rootdir,
TXHASHSET_SUBDIR,
OUTPUT_SUBDIR,
true,
header,
)?,
rproof_pmmr_h: PMMRHandle::new(
&root_dir,
txhashset_subdir,
&txhashset_rootdir,
TXHASHSET_SUBDIR,
RANGE_PROOF_SUBDIR,
true,
header,
)?,
kernel_pmmr_h: PMMRHandle::new(
&root_dir,
txhashset_subdir,
&txhashset_rootdir,
TXHASHSET_SUBDIR,
KERNEL_SUBDIR,
false,
None,
Expand Down Expand Up @@ -1457,29 +1456,43 @@ pub fn zip_read(root_dir: String, header: &BlockHeader, rand: Option<u32>) -> Re
/// Extract the txhashset data from a zip file and writes the content into the
/// txhashset storage dir
pub fn zip_write(
root_dir: String,
root_dir: PathBuf,
txhashset_data: File,
header: &BlockHeader,
sandbox: bool,
) -> Result<(), Error> {
let txhashset_path = Path::new(&root_dir).join(if sandbox {
TXHASHSET_SANDBOX_SUBDIR
} else {
TXHASHSET_SUBDIR
});
debug!("zip_write on path: {:?}", root_dir);
let txhashset_path = root_dir.clone().join(TXHASHSET_SUBDIR);
fs::create_dir_all(txhashset_path.clone())?;
zip::decompress(txhashset_data, &txhashset_path, expected_file)
.map_err(|ze| ErrorKind::Other(ze.to_string()))?;
check_and_remove_files(&txhashset_path, header)
}

/// Clean the temporary sandbox folder
pub fn clean_txhashset_sandbox(root_dir: String) {
let txhashset_path = Path::new(&root_dir).join(TXHASHSET_SANDBOX_SUBDIR);
/// Rename a folder to another
pub fn txhashset_replace(from: PathBuf, to: PathBuf) -> Result<(), Error> {
debug!("txhashset_replace: move from {:?} to {:?}", from, to);

// clean the 'to' folder firstly
clean_txhashset_folder(&to);

// rename the 'from' folder as the 'to' folder
let txhashset_from_path = from.clone().join(TXHASHSET_SUBDIR);
let txhashset_to_path = to.clone().join(TXHASHSET_SUBDIR);
if let Err(e) = fs::rename(txhashset_from_path, txhashset_to_path) {
error!("txhashset_replace fail. err: {}", e);
Err(ErrorKind::TxHashSetErr(format!("txhashset replacing fail")).into())
} else {
Ok(())
}
}

/// Clean the txhashset folder
pub fn clean_txhashset_folder(root_dir: &PathBuf) {
let txhashset_path = root_dir.clone().join(TXHASHSET_SUBDIR);
if txhashset_path.exists() {
if let Err(e) = fs::remove_dir_all(&txhashset_path) {
if let Err(e) = fs::remove_dir_all(txhashset_path.clone()) {
warn!(
"clean_txhashset_sandbox: fail on {:?}. err: {}",
"clean_txhashset_folder: fail on {:?}. err: {}",
txhashset_path, e
);
}
Expand Down
10 changes: 3 additions & 7 deletions chain/tests/test_txhashset.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,12 @@ fn test_unexpected_zip() {
let db_env = Arc::new(store::new_env(db_root.clone()));
let chain_store = ChainStore::new(db_env).unwrap();
let store = Arc::new(chain_store);
txhashset::TxHashSet::open(db_root.clone(), store.clone(), None, false).unwrap();
txhashset::TxHashSet::open(db_root.clone(), store.clone(), None, None).unwrap();
// First check if everything works out of the box
assert!(txhashset::zip_read(db_root.clone(), &BlockHeader::default(), Some(rand)).is_ok());
let zip_path = Path::new(&db_root).join(format!("txhashset_snapshot_{}.zip", rand));
let zip_file = File::open(&zip_path).unwrap();
assert!(
txhashset::zip_write(db_root.clone(), zip_file, &BlockHeader::default(), false).is_ok()
);
assert!(txhashset::zip_write(db_root.clone(), zip_file, &BlockHeader::default()).is_ok());
// Remove temp txhashset dir
fs::remove_dir_all(Path::new(&db_root).join(format!("txhashset_zip_{}", rand))).unwrap();
// Then add strange files in the original txhashset folder
Expand All @@ -66,9 +64,7 @@ fn test_unexpected_zip() {
fs::remove_dir_all(Path::new(&db_root).join(format!("txhashset_zip_{}", rand))).unwrap();

let zip_file = File::open(zip_path).unwrap();
assert!(
txhashset::zip_write(db_root.clone(), zip_file, &BlockHeader::default(), false).is_ok()
);
assert!(txhashset::zip_write(db_root.clone(), zip_file, &BlockHeader::default()).is_ok());
// Check that the txhashset dir dos not contains the strange files
let txhashset_path = Path::new(&db_root).join("txhashset");
assert!(txhashset_contains_expected_files(
Expand Down