-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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: expose BlockKeyCacheSize and enable WriteThrough datastore options #10614
base: master
Are you sure you want to change the base?
Changes from 6 commits
00c68b6
44daae3
4b3aabc
066291c
512f468
b4b028c
0703a15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,8 +4,21 @@ import ( | |||||
"encoding/json" | ||||||
) | ||||||
|
||||||
// DefaultDataStoreDirectory is the directory to store all the local IPFS data. | ||||||
const DefaultDataStoreDirectory = "datastore" | ||||||
const ( | ||||||
// DefaultDataStoreDirectory is the directory to store all the local IPFS data. | ||||||
DefaultDataStoreDirectory = "datastore" | ||||||
|
||||||
// DefaultBlockKeyCacheSize is the size for the blockstore two-queue | ||||||
// cache which caches block keys and sizes. | ||||||
DefaultBlockKeyCacheSize = 64 << 10 | ||||||
|
||||||
// DefaultWriteThrough specifies whether to use a "write-through" | ||||||
// Blockstore and Blockservice. This means that they will write | ||||||
// without performing any reads to check if the incoming blocks are | ||||||
// already present in the datastore. Enable for datastores with fast | ||||||
// writes and slower reads. | ||||||
DefaultWriteThrough = true | ||||||
) | ||||||
|
||||||
// Datastore tracks the configuration of the datastore. | ||||||
type Datastore struct { | ||||||
|
@@ -21,8 +34,10 @@ type Datastore struct { | |||||
|
||||||
Spec map[string]interface{} | ||||||
|
||||||
HashOnRead bool | ||||||
BloomFilterSize int | ||||||
HashOnRead bool | ||||||
BloomFilterSize int | ||||||
BlockKeyCacheSize OptionalInteger `json:",omitempty"` | ||||||
WriteThrough bool | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hsanjuan are you planning to write a migration for existing users (who don't have this in config), or was it intentional to keep existing users stuck with Both feel like a potential headache :) Perhaps we should switch this from
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the option is not present, it will take the default which is true? Or you mean the config-parsing code does not know if its false or unset? |
||||||
} | ||||||
|
||||||
// DataStorePath returns the default data store path given a configuration root | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -207,12 +207,12 @@ func (api *CoreAPI) WithOptions(opts ...options.ApiOption) (coreiface.CoreAPI, e | |
return nil | ||
} | ||
|
||
if settings.Offline { | ||
cfg, err := n.Repo.Config() | ||
if err != nil { | ||
return nil, err | ||
} | ||
cfg, err := n.Repo.Config() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
if settings.Offline { | ||
cs := cfg.Ipns.ResolveCacheSize | ||
if cs == 0 { | ||
cs = node.DefaultIpnsCacheSize | ||
|
@@ -244,7 +244,11 @@ func (api *CoreAPI) WithOptions(opts ...options.ApiOption) (coreiface.CoreAPI, e | |
|
||
if settings.Offline || !settings.FetchBlocks { | ||
subAPI.exchange = offlinexch.Exchange(subAPI.blockstore) | ||
subAPI.blocks = bserv.New(subAPI.blockstore, subAPI.exchange) | ||
var bsopts []bserv.Option | ||
if cfg.Datastore.WriteThrough { | ||
bsopts = append(bsopts, bserv.WriteThrough()) | ||
} | ||
subAPI.blocks = bserv.New(subAPI.blockstore, subAPI.exchange, bsopts...) | ||
Comment on lines
+247
to
+251
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is ok, but is might be nicer to have a blockservice option subAPI.blocks = bserv.New(subAPI.blockstore, subAPI.exchange,
bserv.WithWriteThrough(cfg.Datastore.WriteThrough)) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right? We need to change that in boxo. I can take care, but we can merge this and fix that later for next boxo release. |
||
subAPI.dag = dag.NewDAGService(subAPI.blocks) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(if we switch to Flag)