feat(valkey): add Valkey backend - #858
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bluayer The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/assign @XuanYang-cn |
| batch.unlink([key]) | ||
| deleted += 1 | ||
| pending += 1 | ||
| if pending == config.NUM_PER_BATCH: |
There was a problem hiding this comment.
vectordb_bench/backend/clients/valkey/valkey.py line:135
Medium ---- drop_old cleanup flushes UNLINK in batches of config.NUM_PER_BATCH (default 100) and SCANs with count=100, so re-running a case over an existing dataset costs a large number of round trips (roughly 20k+ exec/scans for 1M keys, and over 1M for the 100M-vector cases) before the timed load even starts. Consider a dedicated, larger scan-count/flush constant, or issuing FT.DROPINDEX DD via custom_command when all prefix documents are known to be indexed (glide's ft.dropindex has no DD option at v2.5.1, so a raw command is needed).
|
|
||
| import numpy as np | ||
| import pytest | ||
| from glide_sync import Batch |
There was a problem hiding this comment.
tests/test_valkey.py line:6
Low ---- This module-level import of glide_sync is only satisfied by the [valkey] extra, which CI does not install (pull_request.yml installs ".[test]" only), so these new tests cannot be collected in the standard test environment and are not exercised by make unittest or make lint. Consider pytest.importorskip("glide_sync") (or adding the valkey extra to the [test] dependency set) and wiring the tests into the CI test command so this substantial coverage can actually run.
| password: SecretStr | None = None | ||
| host: SecretStr | ||
| port: int = Field(default=6379, ge=1, le=65535) | ||
| ssl: bool = True |
There was a problem hiding this comment.
vectordb_bench/backend/clients/valkey/config.py line:10
Low ---- TLS is enabled by default (ssl=True) but there is no CA-certificate path or insecure-TLS toggle, even though glide supports both (tls_config / use_insecure_tls in AdvancedGlide*ClientConfiguration) and the sibling Redis/MemoryDB backends expose ssl_ca_certs. Users with private-CA TLS Valkey cannot connect, and plain local Valkey setups silently fail until they discover --no-ssl. Consider adding ssl_ca_certs (or an insecure-tls flag) and wiring it through _create_client.
3 participants
Summary
Adds a Valkey backend using
valkey-glide-sync.