Allow to set LMDB map size in convert-db script #690
No reviewers
Labels
No Label
AdminAPI
Bug
Check AWS
CI
Correctness
Critical
Documentation
Ideas
Improvement
Low priority
Newcomer
Performance
S3 Compatibility
Testing
Usability
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Deuxfleurs/garage#690
Loading…
Reference in New Issue
No description provided.
Delete Branch "rdelaage/garage:feat/set_lmdb_map_size_convert-db"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fix #684.
This commit add the ability to set the LMDB map size in the convert-db script using a new flag called
lmdb_map_size
.Feel free to suggest modifications, this is my first code in Rust
@rdelaage Oh snap, I noticed your PR only after creating my own for the same issue :-)
Yours look simpler than mine, but harder to extend with more optional overrides (not sure it it will be needed/wanted).
The one thing I'm not sure about is using the LMDB map size override for opening both input and output databases. My thinking was that if converting from LMDB, the map_size override should/will already be set in config file. And therefore the
convert-db
command should set it only for output database. Did I miss something?@zdenek.crha Your PR seems cleaner. Don't if the convert-db script should read the configuration file for the input database, this is not the case right now. The user might want to convert a db from an other deployment and the configuration file does not reflect the database parameters.
@rdelaage Db config not read now? I see. I also did not realize that lmdb might not be output db, because
sqlite
db is not being deprecated. You are right, user may need ability to pass overrides to input db too.What concerns me is edge-case where both input/output db format is the same and user wants to override only one of them. Because that would require two sets of override parameters, one for input and other for output.
We can get around that by saying that input/output format must be different, as this command is for conversion and not tweaking database parameters. Then we would have only one set of override parameters. We could use one subset for opening input db and another, disjoint subset for opening the output db.
What do you think? Anything I'm still missing?
I think the assumption that the input format and output format will be different will hold almost all the time, so it's fine to have only one set of parameters which can apply to both the input and output db.
@lx Great!
@rdelaage If you don't mind, I would like to move discussion and changes to #691. The CLI option structs there are bit better prepared for adding multiple overrides for different formats. I think I'll have time to update it tonight.
I am closing this MR since MR #691 will fix the original issue
Pull request closed