-
Notifications
You must be signed in to change notification settings - Fork 58
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
docs: extend waku store config tutorial of the operator docs #1359
Conversation
Jenkins BuildsClick to see older builds (10)
|
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.
LGTM, thanks!
3f5cc52
to
53a4213
Compare
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.
LGTM.
Left some minor comments.
|
||
## Configuring the node as a store service node | ||
|
||
If the waku store node is enabled (the `--store` option is set to `true`) the node will store historical messages and will be able to serve those messages to the waku store clients. |
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.
Does that mean storing messages is default? (Because --store=true
is default)
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.
I'd make it explicit above that you need to set --store=false
and set --storenode
to setup a client only store-node.
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.
Does that mean storing messages is default? (Because
--store=true
is default)
Yes, that's what I meant with "the node will store historical messages".
I'd make it explicit above that you need to set
--store=false
and set--storenode
to setup a client only store-node.
This is not the case. Both things (server and client) are independent. You can have both on, both off, or one on and the other off.
This is what I meant with this: https://github.com/status-im/nwaku/pull/1359/files?diff=split&w=0#diff-a2d58beafcccb9e2b626c20cb0e31383f1107c520e7a427e20d2896763f71eb7R16 😁
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.
True, but it may be worth spelling out in the client section that to configure a client only store
must explicitly be set to false
. This is clearly implied, of course, but we should assume that operators will just skip to the section they're interested in and may be surprised that following the client guidelines lead to a node that's also persisting messages.
Not part of this conversation, but wondering if shouldn't rather make the default for store
false
. Otherwise many operators upgrading after the release will suddenly start persisting messages. We can warn them, of course, but I think the sensible default for a nwaku node should just be a relay
node.
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.
lgtm! thanks!
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.
LGTM! Thanks. Apologies for only reviewing after merging. Comments below are not critical at all and can be addressed in future. A good idea to follow semantic line breaks in future. :)
Currently store service nodes use, by default, a message store backed by an in-disk SQLite database. Most Waku messages average a size of 1KB - 2KB, implying a minimum memory requirement of at least ~250MB |
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.
Great to have some numbers here!
* `--store-message-retention-policy`: This option controls the retention policy i.e., how long certain messages will be persisted. Two different retention policies are supported: | ||
+ The time retention policy,`time:<duration-in-seconds>` (e.g., `time:14400`) | ||
+ The capacity retention policy,`capacity:<messages-count>` (e.g, `capacity:25000`) | ||
+ To disable the retention policy, explicitly, set this option to to `""`, an empty string. |
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.
Can we spell out the implications? Something like: Disabling the retention policy implies that, from nwaku's perspective, messages will either be persisted indefinitely or that retention is managed by a third party outside of the node.
|
||
## Configuring the node as a store service node | ||
|
||
If the waku store node is enabled (the `--store` option is set to `true`) the node will store historical messages and will be able to serve those messages to the waku store clients. |
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.
True, but it may be worth spelling out in the client section that to configure a client only store
must explicitly be set to false
. This is clearly implied, of course, but we should assume that operators will just skip to the section they're interested in and may be surprised that following the client guidelines lead to a node that's also persisting messages.
Not part of this conversation, but wondering if shouldn't rather make the default for store
false
. Otherwise many operators upgrading after the release will suddenly start persisting messages. We can warn them, of course, but I think the sensible default for a nwaku node should just be a relay
node.
This PR covers the work pending of #1293
Same here: