Skip to content

Allow Native::marshal_load to load "old style" marshal_dumped bloomfilters #29

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

marcheiligers
Copy link

  • Fixed various compile time warnings and silenced output during tests
  • Updated CBloomFilter bf_load to check string size and load old style if needed
  • Added tests to check that both old and new style can be loaded
  • Added a getter to return the options for an instance
  • Took a chance and bumped the minor version number in the hope this will be released ;)

@igrigorik
Copy link
Owner

Added tests to check that both old and new style can be loaded

Hmm, do we actually want that? Generally, I'm not a fan of carrying this kind of stuff forward -- makes future work unnecessary complicated. I understand the issue of broken upgrade, but to address this, we could provide a different tool to perform the conversion (if you such thing). Thoughts?

@marcheiligers
Copy link
Author

From my angle, I've got several million records stored in a Riak cluster that are taking up far too much space. I'd like to silently upgrade and not worry about 'em. I don't believe the code added to handle this has any performance impact.

Also I'm a big fan of backwards compatibility ;)

@igrigorik
Copy link
Owner

@marcheiligers it's not performance impact, as much as unnecessary anchors / code moving forward. With that said, I guess it's not too big of a hurdle here.

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.

2 participants