Remove skip building graph check for quantization use case #2430
Conversation
VijayanB
force-pushed
the
remove-threshold-for-quants
branch
from
473c7db to
ced2035
Compare
VijayanB
requested review from
heemin32,
navneet1v,
vamshin,
jmazanec15,
naveentatikonda,
junqiu-lei,
martin-gaievski,
ryanbogan,
luyuncheng,
shatejas and
0ctopus13prime
as code owners
|
I am raising directly to 2.x since i am blocked because of lucene codec issue. Will backport this to main once it is fixed. |
|
Currently quantization state is not null only for on-disk mode. We don't support in-memory quantization at this moment. Hence, this check is good enough. However if we want to check for on-disk, we have to pass this as parameter and add it to check. @jmazanec15 @shatejas @Vikasht34 @navneet1v What do you think? |
For quantization indices, we don't have to apply building graph check since it is already faster, this is now only applied for fp32/16 indices and where threshold is configured. Signed-off-by: Vijayan Balasubramanian <[email protected]>
VijayanB
force-pushed
the
remove-threshold-for-quants
branch
from
ced2035 to
db017b6
Compare
|
In general, the "mode" is meant to select defaults for other parameters, so it should not be directly used to disable and enable certain features. Instead, it may resolve to a parameter that then makes this decision. Anyway, in the PR, it appears that you actually disable if quantization is used. I think in general something like this is okay, but might be misleading/confusing to users who set the setting anticipating that graph builds will be skipped. That being said, is this just a concern of performance, or is it a functional bug? If it is performance related only, Id recommend using mode to resolve a per-field value for skipping graph build. Then, users could override if need be. This would be consistent with current approach around mode. |
Yes, i will update that this parameter will be no-hop for lucene, and on-disk |
For quantization indices, we don't have to apply building graph check since it is already faster, this is now only applied for fp32/16 indices and where threshold is configured. Signed-off-by: Vijayan Balasubramanian <[email protected]> (cherry picked from commit 8d13c74)
Description
For on-disk quantization indices, we don't have to apply building graph check since it is already faster, and we found is causing latency spike during search. Hence, done check for threshold if quantization criteria is met.
Related Issues
part of #2215
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.