Test compression segment serializer #2696
Conversation
Signed-off-by: Arun Ganesh <[email protected]>
Signed-off-by: Arun Ganesh <[email protected]>
Signed-off-by: Arun Ganesh <[email protected]>
Signed-off-by: Arun Ganesh <[email protected]>
Signed-off-by: Arun Ganesh <[email protected]>
Signed-off-by: Arun Ganesh <[email protected]>
oaganesh
requested review from
0ctopus13prime,
VijayanB,
Vikasht34,
heemin32,
jmazanec15,
junqiu-lei,
luyuncheng,
martin-gaievski,
naveentatikonda,
navneet1v,
ryanbogan,
shatejas and
vamshin
as code owners
…erializer Signed-off-by: oaganesh <[email protected]>
| segmentWriteState.segmentInfo.name, | ||
| segmentWriteState.segmentSuffix, | ||
| KNNConstants.QUANTIZATION_STATE_FILE_SUFFIX | ||
| // public KNN990QuantizationStateWriter(SegmentWriteState segmentWriteState) throws IOException { |
There was a problem hiding this comment.
We shouldn't leave commented code in the PR.
| // output = segmentWriteState.directory.createOutput(quantizationStateFileName, segmentWriteState.context); | ||
| // } | ||
|
|
||
| public KNN990QuantizationStateWriter(SegmentWriteState segmentWriteState, String fileSuffix) throws IOException { |
There was a problem hiding this comment.
Should we be using quantization state writer for this? If we're doing a different segment state we should probably have a separate writer. But this is ok for POC purposes
There was a problem hiding this comment.
That's a good idea. When trying to write another writer it required a lot of further refactoring as the other classes implement this.
| this(segmentWriteState, KNNConstants.QUANTIZATION_STATE_FILE_SUFFIX); | ||
| } | ||
|
|
||
| // public KNN990QuantizationStateWriter(SegmentWriteState segmentWriteState) throws IOException { |
There was a problem hiding this comment.
Let's not leave commented code here.
| ); | ||
| final QuantizationState quantizationState = train(field.getFieldInfo(), knnVectorValuesSupplier, totalLiveDocs); | ||
| SegmentProfilerState.profileVectors(knnVectorValuesSupplier); | ||
| profile(field.getFieldInfo(), knnVectorValuesSupplier, totalLiveDocs); |
There was a problem hiding this comment.
Are we correct in our assumption that the knnVectorValuesSupplier contains information on non-compressed vectors? i.e the profiling happens before the compression.
| private void initSegmentStateWriterIfNecessary() throws IOException { | ||
| if (segmentStateWriter == null) { | ||
| segmentStateWriter = new KNN990QuantizationStateWriter(segmentWriteState, KNNConstants.SEGMENT_PROFILE_STATE_FILE_SUFFIX); | ||
| segmentStateWriter.writeHeader(segmentWriteState); |
There was a problem hiding this comment.
If we're going with this approach we're going to have to refactor the QuantizationStateWriter to support generic file writing
|
|
||
| @Override | ||
| public void writeTo(StreamOutput streamOutput) throws IOException { | ||
| streamOutput.writeString(shardId); |
There was a problem hiding this comment.
I don't think we need this override? Did we confirm that's its even getting called in the API path?
| @Getter | ||
| public class KNNProfileRequest extends BroadcastRequest<KNNProfileRequest> { | ||
|
|
||
| private String index; |
| } | ||
|
|
||
| byte[] stateBytes = readStateBytes(input, position, length); | ||
| return SegmentProfilerState.fromBytes(stateBytes); |
There was a problem hiding this comment.
Overall this logic looks ok to me but we can probably clean it up later
Signed-off-by: Arun Ganesh <[email protected]>
2 participants
Description
Writing the segment profiler state to a file and profiling the different compression standards for benchmark testing.
Related Issues
Implements #2687
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 ✔️ .