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

feat: simplify pebble datastore creation with single set of options #39

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

gammazero
Copy link
Contributor

@gammazero gammazero commented Oct 2, 2024

Use a single set of options when creating the Pebble Datastore. One of the options allows passing a struct containing Pebble options. It is no longer necessary to pass nil when using the default configuration.

Make it easier to handle creation/teardown of a custom-sized Pebble shared block cache. Only the cache size needs to be specified as a creation option if using other the default size. Instead of requiring the caller to pass in a cache instance, and manage its lifetime, a WithCacheSize option can pass the size and let the pebble Datastore manage the cache lifetime. Passing in a cache can optionally still be done.

Use a single set of options when creating the Pebble Datastore. One of the options allows passing a struct containing all Pebble options.This means that no argumants other than path are to create a pebble datastore with the default settings.

Make it easier to handle creation/teardown of a custom-sized Pebble shared block cache. Only the cache size need to be specified as a creation option if using other the default size.
@gammazero gammazero requested a review from lidel October 2, 2024 08:13
gammazero added a commit to ipfs/kubo that referenced this pull request Oct 2, 2024
Pebble provides a high-performance alternative to leveldb as the datastore, and will serve as a replacement for badger1.

There are a number of tuning parameters available for tuning pebble's performance to your specific needs. Default values are used for any that are not configured or are set to the parameter's zero-value.

Requires ipfs/go-ds-pebble#39

Closes #10347
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

lgtm from perspective of kubo and rainbow, but would be good to get feedback from existing users like @hsanjuan (ipfs-cluster) and @Kubuxu (lotus) 🙏

@Kubuxu
Copy link
Member

Kubuxu commented Oct 3, 2024

Lotus does not use pebble so LGTM from my side.

lidel added a commit to lidel/ipfs-cluster that referenced this pull request Oct 3, 2024
Testing ipfs/go-ds-pebble#39
before tagging final release
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Tested in ipfs-cluster/ipfs-cluster#2099 and it should not cause big issue for existing users. Let's ship.

@gammazero mind merging and releasing, and switching ipfs/kubo#10530 to release version, so we can take it for a spin in Kubo 0.31 RC1 (ipfs/kubo#10499)?

@gammazero gammazero merged commit e45b34b into master Oct 3, 2024
6 checks passed
@gammazero gammazero deleted the simplify-creation branch October 3, 2024 20:09
gammazero added a commit to ipfs/kubo that referenced this pull request Oct 3, 2024
* include pebble as built-in plugin

Pebble provides a high-performance alternative to leveldb as the datastore, and will serve as a replacement for badger1.

There are a number of tuning parameters available for tuning pebble's performance to your specific needs. Default values are used for any that are not configured or are set to the parameter's zero-value.

Requires ipfs/go-ds-pebble#39

Closes #10347

* docs: remove mention of ipfs-ds-convert. Rationale: ipfs-inactive/ipfs-ds-convert#50
* docs: pebbleds profile
* test: meaningful t0025-datastores.sh
* Update config/init.go
* Update docs/config.md
* Do not hard-code zero values into pebble config
lidel added a commit to lidel/ipfs-cluster that referenced this pull request Oct 3, 2024
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