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.

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

[1.1.0] Support variable size PMMR (optional size_file) #2734

Merged

Conversation

antiochp
Copy link
Member

@antiochp antiochp commented Apr 5, 2019

Rebasing changes from #2733 across to 1.1.0 branch.

Support fixed size data file via an optional elmt_size.
Support variable size data file via optional size_file.
@antiochp antiochp added this to the 1.1.0 milestone Apr 5, 2019
@yeastplume
Copy link
Member

yeastplume commented Apr 5, 2019

for windows fix, save_prune in store/src/types.rs should be (just scoping the tmp file):

	pub fn save_prune(&mut self, prune_pos: &[u64]) -> io::Result<()> {
		let tmp_path = self.path.with_extension("tmp");

		{
			let reader = File::open(&self.path)?;
			let mut buf_reader = BufReader::new(reader);
			let mut streaming_reader =
				StreamingReader::new(&mut buf_reader, time::Duration::from_secs(1));

			let mut buf_writer = BufWriter::new(File::create(&tmp_path)?);
			let mut bin_writer = BinWriter::new(&mut buf_writer);

			let mut current_pos = 0;
			let mut prune_pos = prune_pos;
			while let Ok(elmt) = T::read(&mut streaming_reader) {
				if prune_pos.contains(&current_pos) {
					// Pruned pos, moving on.
					prune_pos = &prune_pos[1..];
				} else {
					// Not pruned, write to file.
					elmt.write(&mut bin_writer)
						.map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
				}
				current_pos += 1;
			}
			buf_writer.flush()?;
		}

		// Replace the underlying file -
		// pmmr_data.tmp -> pmmr_data.bin
		self.replace(&tmp_path)?;

		// Now rebuild our size file to reflect the pruned data file.
		// This will replace the underlying file internally.
		if let Some(_) = &self.size_file {
			self.rebuild_size_file()?;
		}

		// Now (re)init the file and associated size_file so everything is consistent.
		self.init()?;

		Ok(())
	}

@antiochp
Copy link
Member Author

antiochp commented Apr 5, 2019

@yeastplume thanks for debugging this. I'll fix and push again later on.

@yeastplume
Copy link
Member

Besides that, it all looks good and glad you've done this. I won't pretend I've exhaustively looked through all the rewind and pruning logic changes, but I know how hairy that gets and trust it was sufficiently fiddly to get working.

Do we want to get this into 1.1.0? Given the changes are all underneath, we can do a beta 2 of the server portion. I do think there is still an issue with sync on Windows that might require more changes, give me a day or so to investigate.

@antiochp
Copy link
Member Author

antiochp commented Apr 9, 2019

Do we want to get this into 1.1.0? Given the changes are all underneath, we can do a beta 2 of the server portion.

I think its worth considering if we get to the point where Windows sync appears robust/fixed.
👍

@yeastplume
Copy link
Member

Absolutely fine on windows now, turns out my issues were local. Happy to merge this and do a beta 2 of the grin server (perhaps another merge from master to get a couple of other bits and pieces in)

@ignopeverell ignopeverell merged commit 3d817f6 into mimblewimble:milestone/1.1.0 Apr 12, 2019
@antiochp antiochp deleted the 1_1_0_variable_size_mmr branch April 13, 2019 11:58
@antiochp antiochp added the release notes To be included in release notes (of relevant milestone). label Jun 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement release notes To be included in release notes (of relevant milestone).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants