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

Add Buffer Size Parameter to dump create #234

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gugacavalieri
Copy link

@gugacavalieri gugacavalieri commented Nov 5, 2022

Changelog

  • Add Argument to set buffer_size when dumping the database
  • Set the default size to 100 (This was the previous value)

Motivation

I was running into the same issue when dumping tables bigger than 100MB (#208). The order of the Statements were wrong and I was not able to restore the dumps. I still don't know 100% what is cause the issue but it's probably related when the compressed chunks are being read from the datastore and are being restored in the wrong order.

So, this PR aims to make the arguments more flexible and serve as a workaround for the issue above.

PS. I am still learning Rust, so please let me know if this is a good way to move parameters around 😄

@gugacavalieri gugacavalieri marked this pull request as ready for review November 5, 2022 19:16
@evoxmusic
Copy link
Contributor

Thank you for the contribution. I'm going to take a look this weekend

replibyte/src/commands/dump.rs Outdated Show resolved Hide resolved
replibyte/src/commands/dump.rs Outdated Show resolved Hide resolved
replibyte/src/commands/dump.rs Outdated Show resolved Hide resolved
replibyte/src/commands/dump.rs Outdated Show resolved Hide resolved
replibyte/src/commands/dump.rs Outdated Show resolved Hide resolved
replibyte/src/commands/dump.rs Outdated Show resolved Hide resolved
}

impl<'a, S> FullDumpTask<'a, S>
where
S: Source,
{
pub fn new(source: S, datastore: Box<dyn Datastore>, options: SourceOptions<'a>) -> Self {
pub fn new(source: S, datastore: Box<dyn Datastore>, options: SourceOptions<'a>, arg_buffer_size: usize) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

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

My 2 comments:

  1. Include arg_buffer_size into options and remove arg_buffer_size parameter
  2. rename options.arg_buffer_size into something more explicit like dump_chunk_size or anything else

@gugacavalieri
Copy link
Author

Good idea! I will do some work on the fixes.

@gugacavalieri
Copy link
Author

gugacavalieri commented Nov 20, 2022

@evoxmusic, added dump_chunk_size to the SourceOptions struct.

Also have to play a little bit with the test methods 😃

@evoxmusic
Copy link
Contributor

@gugacavalieri can you check tests errors?

@michaelrode
Copy link

Thanks for adding this, can you also update the README with an example of how to use this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants