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

Fix issue with incorrect database_size when requesting a smaller shared_file_size #29

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

greg7mdp
Copy link
Contributor

@greg7mdp greg7mdp commented Nov 7, 2023

Resolves #28 .

This makes sure our _database_size member tracks the actual size of the file we are mmap'ing into memory.

@@ -134,7 +134,8 @@ pinnable_mapped_file::pinnable_mapped_file(const std::filesystem::path& dir, boo
std::filesystem::resize_file(_data_file_path, shared_file_size);
}
else if(shared_file_size < existing_file_size) {
std::cerr << "CHAINBASE: \"" << _database_name << "\" requested size of " << shared_file_size << " is less than "
_database_size = existing_file_size;
std::cerr << "CHAINBASE: \"" << _database_name << "\" requested size of " << shared_file_size << " is less than "
Copy link
Member

Choose a reason for hiding this comment

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

I think this code would be clearer if the constructor used _database_size instead of shared_file_size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't disagree that it is not very clear. This is for a 5.0 issue, so I'd rather make a small change.

Also we have this (maybe unintuitive) behavior that we allow the user to request a smaller size, but if the database already exist and is larger we just use the existing size without shrinking it (which would be difficult).

@greg7mdp greg7mdp merged commit 91a7398 into main Nov 8, 2023
2 checks passed
@greg7mdp greg7mdp deleted the gh_1875 branch November 8, 2023 15:36
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.

[5.0] Fix chainbase issue when shrinking the database size.
3 participants