Skip to content

Fixed valkey-search configuration section #285

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 1 commit into
base: main
Choose a base branch
from

Conversation

eifrah-aws
Copy link
Contributor

No description provided.

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
sizes may improve performance by enhancing CPU cache efficiency, but come at the cost of increased memory usage due
to pre-allocation for potential future growth. (Default: 10K)
5. `--log-level` Controls the log verbosity level. Possible values are: `debug`, `verbose`, `notice` and `warning`. (Default: Valkey's log level)
1. `--use-coordinator`: (Optional) Cluster mode enabler. Default: `false`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. `--use-coordinator`: (Optional) Cluster mode enabler. Default: `false`.
1. `--search.use-coordinator`: (Optional) Cluster mode enabler. Default: `false`.

Works as long as it's on a separate line. The search modules has special logic to also parse it as argv/argc. You should use the LOADEX module instead:

https://valkey.io/commands/module-loadex/

Comment on lines +190 to +195
**NOTE**
---

All runtime configuration settings listed above can also be applied when starting the module. To do this, simply replace
the `search.` prefix with `--` in the parameter name. For instance, if you want to set the `search.hnsw-block-size` parameter
at startup, use `--hnsw-block-size <VALUE>` in the `loadmodule` configuration line.
Copy link
Member

Choose a reason for hiding this comment

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

This is a custom implementation. I think we should just use the standard mechanism.

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