Skip to content
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

Star tree codec changes #14514

Merged
merged 7 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.opensearch.common.Nullable;
import org.opensearch.common.collect.MapBuilder;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.codec.composite.Composite99Codec;
import org.opensearch.index.mapper.MapperService;

import java.util.Map;
Expand Down Expand Up @@ -73,10 +74,20 @@
codecs.put(BEST_COMPRESSION_CODEC, new Lucene99Codec(Mode.BEST_COMPRESSION));
codecs.put(ZLIB, new Lucene99Codec(Mode.BEST_COMPRESSION));
} else {
codecs.put(DEFAULT_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger));
codecs.put(LZ4, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger));
codecs.put(BEST_COMPRESSION_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger));
codecs.put(ZLIB, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger));
// CompositeCodec still delegates to PerFieldMappingPostingFormatCodec
// We can still support all the compression codecs when composite index is present
// hence we're defining the codecs like below
if (mapperService.isCompositeIndexPresent()) {
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
codecs.put(DEFAULT_CODEC, new Composite99Codec(Mode.BEST_SPEED, mapperService, logger));
codecs.put(LZ4, new Composite99Codec(Mode.BEST_SPEED, mapperService, logger));
codecs.put(BEST_COMPRESSION_CODEC, new Composite99Codec(Mode.BEST_COMPRESSION, mapperService, logger));
codecs.put(ZLIB, new Composite99Codec(Mode.BEST_COMPRESSION, mapperService, logger));

Check warning on line 84 in server/src/main/java/org/opensearch/index/codec/CodecService.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/CodecService.java#L81-L84

Added lines #L81 - L84 were not covered by tests
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
} else {
codecs.put(DEFAULT_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger));
codecs.put(LZ4, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger));
codecs.put(BEST_COMPRESSION_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger));
codecs.put(ZLIB, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger));
}
}
codecs.put(LUCENE_DEFAULT_CODEC, Codec.getDefault());
for (String codec : Codec.availableCodecs()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.index.codec.composite;

import org.apache.lucene.codecs.DocValuesConsumer;
import org.apache.lucene.codecs.DocValuesFormat;
import org.apache.lucene.codecs.DocValuesProducer;
import org.apache.lucene.codecs.lucene90.Lucene90DocValuesFormat;
import org.apache.lucene.index.SegmentReadState;
import org.apache.lucene.index.SegmentWriteState;
import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.index.mapper.MapperService;

import java.io.IOException;

/**
* DocValues format to handle composite indices
*
* @opensearch.experimental
*/
@ExperimentalApi
public class Composite90DocValuesFormat extends DocValuesFormat {
/**
* Creates a new docvalues format.
*
* <p>The provided name will be written into the index segment in some configurations (such as
* when using {@code PerFieldDocValuesFormat}): in such configurations, for the segment to be read
* this class should be registered with Java's SPI mechanism (registered in META-INF/ of your jar
* file, etc).
*/
private final DocValuesFormat delegate;
private final MapperService mapperService;

// needed for SPI
public Composite90DocValuesFormat() {
this(new Lucene90DocValuesFormat(), null);
}

Check warning on line 43 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java#L42-L43

Added lines #L42 - L43 were not covered by tests

public Composite90DocValuesFormat(MapperService mapperService) {
this(new Lucene90DocValuesFormat(), mapperService);
}

Check warning on line 47 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java#L46-L47

Added lines #L46 - L47 were not covered by tests

public Composite90DocValuesFormat(DocValuesFormat delegate, MapperService mapperService) {
super(delegate.getName());
this.delegate = delegate;
this.mapperService = mapperService;
}

Check warning on line 53 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java#L50-L53

Added lines #L50 - L53 were not covered by tests

@Override
public DocValuesConsumer fieldsConsumer(SegmentWriteState state) throws IOException {
return new Composite90DocValuesWriter(delegate.fieldsConsumer(state), state, mapperService);

Check warning on line 57 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java#L57

Added line #L57 was not covered by tests
}

@Override
public DocValuesProducer fieldsProducer(SegmentReadState state) throws IOException {
return new Composite90DocValuesReader(delegate.fieldsProducer(state), state, mapperService);

Check warning on line 62 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesFormat.java#L62

Added line #L62 was not covered by tests
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.index.codec.composite;

import org.apache.lucene.codecs.DocValuesProducer;
import org.apache.lucene.index.BinaryDocValues;
import org.apache.lucene.index.FieldInfo;
import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.index.SegmentReadState;
import org.apache.lucene.index.SortedDocValues;
import org.apache.lucene.index.SortedNumericDocValues;
import org.apache.lucene.index.SortedSetDocValues;
import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.index.mapper.CompositeMappedFieldType;
import org.opensearch.index.mapper.MapperService;

import java.io.IOException;
import java.util.List;
import java.util.Set;

/**
* Reader for star tree index and star tree doc values from the segments
*
* @opensearch.experimental
*/
@ExperimentalApi
public class Composite90DocValuesReader extends DocValuesProducer implements CompositeIndexReader {
private DocValuesProducer delegate;
Set<CompositeMappedFieldType> compositeMappedFieldTypes;
MapperService mapperService;

public Composite90DocValuesReader(DocValuesProducer producer, SegmentReadState state, MapperService mapperService) throws IOException {
this.delegate = producer;
this.mapperService = mapperService;
this.compositeMappedFieldTypes = mapperService.getCompositeFieldTypes();

Check warning on line 41 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L38-L41

Added lines #L38 - L41 were not covered by tests
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
// TODO : read star tree files
}

Check warning on line 43 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L43

Added line #L43 was not covered by tests

@Override
public NumericDocValues getNumeric(FieldInfo field) throws IOException {
return delegate.getNumeric(field);

Check warning on line 47 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L47

Added line #L47 was not covered by tests
}

@Override
public BinaryDocValues getBinary(FieldInfo field) throws IOException {
return delegate.getBinary(field);

Check warning on line 52 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L52

Added line #L52 was not covered by tests
}

@Override
public SortedDocValues getSorted(FieldInfo field) throws IOException {
return delegate.getSorted(field);

Check warning on line 57 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L57

Added line #L57 was not covered by tests
}

@Override
public SortedNumericDocValues getSortedNumeric(FieldInfo field) throws IOException {
return delegate.getSortedNumeric(field);

Check warning on line 62 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L62

Added line #L62 was not covered by tests
}

@Override
public SortedSetDocValues getSortedSet(FieldInfo field) throws IOException {
return delegate.getSortedSet(field);

Check warning on line 67 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L67

Added line #L67 was not covered by tests
}

@Override
public void checkIntegrity() throws IOException {
delegate.checkIntegrity();

Check warning on line 72 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L72

Added line #L72 was not covered by tests
// Todo : check integrity of composite index related [star tree] files
}

Check warning on line 74 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L74

Added line #L74 was not covered by tests

@Override
public void close() throws IOException {
delegate.close();

Check warning on line 78 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L78

Added line #L78 was not covered by tests
// Todo: close composite index related files [star tree] files
}

Check warning on line 80 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L80

Added line #L80 was not covered by tests

@Override
public List<String> getCompositeIndexFields() {
// todo : read from file formats and get the field names.
return null;

Check warning on line 85 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L85

Added line #L85 was not covered by tests
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public CompositeIndexValues getCompositeIndexValues(String field, CompositeMappedFieldType.CompositeFieldType fieldType)
throws IOException {
// TODO : read compositeIndexValues [starTreeValues] from star tree files
return null;

Check warning on line 92 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesReader.java#L92

Added line #L92 was not covered by tests
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.index.codec.composite;

import org.apache.lucene.codecs.DocValuesConsumer;
import org.apache.lucene.codecs.DocValuesProducer;
import org.apache.lucene.index.FieldInfo;
import org.apache.lucene.index.MergeState;
import org.apache.lucene.index.SegmentWriteState;
import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.index.mapper.CompositeMappedFieldType;
import org.opensearch.index.mapper.MapperService;

import java.io.IOException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

/**
* This class write the star tree index and star tree doc values
* based on the doc values structures of the original index
*
* @opensearch.experimental
*/
@ExperimentalApi
public class Composite90DocValuesWriter extends DocValuesConsumer {
private final DocValuesConsumer delegate;
private final SegmentWriteState state;
private final MapperService mapperService;
private MergeState mergeState = null;

Check warning on line 37 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L37

Added line #L37 was not covered by tests
private final Set<CompositeMappedFieldType> compositeMappedFieldTypes;
private final Set<String> compositeFieldSet;

private final Map<String, DocValuesProducer> fieldProducerMap = new HashMap<>();
private final Map<String, FieldInfo> fieldToFieldInfoMap = new HashMap<>();

Check warning on line 42 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L41-L42

Added lines #L41 - L42 were not covered by tests

public Composite90DocValuesWriter(DocValuesConsumer delegate, SegmentWriteState segmentWriteState, MapperService mapperService)
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
throws IOException {

Check warning on line 45 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L45

Added line #L45 was not covered by tests

this.delegate = delegate;
this.state = segmentWriteState;
this.mapperService = mapperService;
this.compositeMappedFieldTypes = mapperService.getCompositeFieldTypes();
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
compositeFieldSet = new HashSet<>();

Check warning on line 51 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L47-L51

Added lines #L47 - L51 were not covered by tests
for (CompositeMappedFieldType type : compositeMappedFieldTypes) {
compositeFieldSet.add(type.name());
}
}

Check warning on line 55 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L53-L55

Added lines #L53 - L55 were not covered by tests

@Override
public void addNumericField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException {
delegate.addNumericField(field, valuesProducer);
}

Check warning on line 60 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L59-L60

Added lines #L59 - L60 were not covered by tests

@Override
public void addBinaryField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException {
delegate.addBinaryField(field, valuesProducer);
}

Check warning on line 65 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L64-L65

Added lines #L64 - L65 were not covered by tests

@Override
public void addSortedField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException {
delegate.addSortedField(field, valuesProducer);
}

Check warning on line 70 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L69-L70

Added lines #L69 - L70 were not covered by tests

@Override
public void addSortedNumericField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException {
delegate.addSortedNumericField(field, valuesProducer);

Check warning on line 74 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L74

Added line #L74 was not covered by tests
// Perform this only during flush flow
if (mergeState == null) {
createCompositeIndicesIfPossible(valuesProducer, field);

Check warning on line 77 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L77

Added line #L77 was not covered by tests
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
}
}

Check warning on line 79 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L79

Added line #L79 was not covered by tests

@Override
public void addSortedSetField(FieldInfo field, DocValuesProducer valuesProducer) throws IOException {
delegate.addSortedSetField(field, valuesProducer);
}

Check warning on line 84 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L83-L84

Added lines #L83 - L84 were not covered by tests

@Override
public void close() throws IOException {

}

Check warning on line 89 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L89

Added line #L89 was not covered by tests

private void createCompositeIndicesIfPossible(DocValuesProducer valuesProducer, FieldInfo field) throws IOException {
if (compositeFieldSet.isEmpty()) return;
if (compositeFieldSet.contains(field.name)) {
fieldProducerMap.put(field.name, valuesProducer);
sachinpkale marked this conversation as resolved.
Show resolved Hide resolved
fieldToFieldInfoMap.put(field.name, field);
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
compositeFieldSet.remove(field.name);

Check warning on line 96 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L94-L96

Added lines #L94 - L96 were not covered by tests
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
}
// we have all the required fields to build composite fields
if (compositeFieldSet.isEmpty()) {
for (CompositeMappedFieldType mappedType : compositeMappedFieldTypes) {
if (mappedType.getCompositeIndexType().equals(CompositeMappedFieldType.CompositeFieldType.STAR_TREE)) {
// TODO : Call StarTree builder
}
}

Check warning on line 104 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L104

Added line #L104 was not covered by tests
}
}

Check warning on line 106 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L106

Added line #L106 was not covered by tests

@Override
public void merge(MergeState mergeState) throws IOException {
// TODO : check if class variable will cause concurrency issues
this.mergeState = mergeState;
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
super.merge(mergeState);

Check warning on line 112 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L111-L112

Added lines #L111 - L112 were not covered by tests
// TODO : handle merge star tree
// mergeStarTreeFields(mergeState);
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
}

Check warning on line 115 in server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite90DocValuesWriter.java#L115

Added line #L115 was not covered by tests
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.index.codec.composite;

import org.apache.logging.log4j.Logger;
import org.apache.lucene.codecs.Codec;
import org.apache.lucene.codecs.DocValuesFormat;
import org.apache.lucene.codecs.FilterCodec;
import org.apache.lucene.codecs.lucene99.Lucene99Codec;
import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.index.codec.PerFieldMappingPostingFormatCodec;
import org.opensearch.index.mapper.MapperService;

/**
* Extends the Codec to support new file formats for composite indices eg: star tree index
* based on the mappings.
*
* @opensearch.experimental
*/
@ExperimentalApi
public class Composite99Codec extends FilterCodec {
sachinpkale marked this conversation as resolved.
Show resolved Hide resolved
public static final String COMPOSITE_INDEX_CODEC_NAME = "Composite99Codec";
private final MapperService mapperService;

public Composite99Codec() {
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
this(COMPOSITE_INDEX_CODEC_NAME, new Lucene99Codec(), null);
}

Check warning on line 33 in server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java#L32-L33

Added lines #L32 - L33 were not covered by tests

public Composite99Codec(Lucene99Codec.Mode compressionMode, MapperService mapperService, Logger logger) {
Copy link
Collaborator

@reta reta Aug 21, 2024

Choose a reason for hiding this comment

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

@bharath-techie @msfroh I think we have an issue here with PerFieldMappingPostingFormatCodec, as per invariants, PerFieldMappingPostingFormatCodec always uses the latest available codec


    static {
        assert Codec.forName(Lucene.LATEST_CODEC).getClass().isAssignableFrom(PerFieldMappingPostingFormatCodec.class)
            : "PerFieldMappingPostingFormatCodec must subclass the latest " + "lucene codec: " + Lucene.LATEST_CODEC;
    }

but Composite99Codec is explicitly bound to Lucene 9.9 (the latest for the current Apache Lucene version). I think we need to make changes here while it is not too late (Apache Lucene 9.12 comes with new codecs)

Copy link
Member

Choose a reason for hiding this comment

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

@reta I believe that should be fine.

Lets consider the case when we upgrade to Lucene-912.

  • The parametrized constructor for Composite99Codec breaks at compile time as PerFieldMappingPostingFormatCodec will accept only Lucene-912 Compression mode and the parameteized constructor in Composite99Codec will hence be removed (This is expected and preferred as we won't use this codec for any writes after upgrade).
  • For writes which have happened using Composite99Codec, the reads will use the SPI constructor which is hardcoded to point to Lucene99Codec and does not cause issues.
  • The new writes will happen via Composite9-12Codec which is obtained through CompositeCodecFactory as the implementation will switch to use Composite9-12Codec. The parametrized constructor will now only need to be present in this new codec.

For reference, this is how the custom-codecs plugin handles this as well.

this(COMPOSITE_INDEX_CODEC_NAME, new PerFieldMappingPostingFormatCodec(compressionMode, mapperService, logger), mapperService);
}

Check warning on line 37 in server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java#L36-L37

Added lines #L36 - L37 were not covered by tests

/**
* Sole constructor. When subclassing this codec, create a no-arg ctor and pass the delegate codec and a unique name to
* this ctor.
*
* @param name name of the codec
* @param delegate codec delegate
* @param mapperService mapper service instance
*/
protected Composite99Codec(String name, Codec delegate, MapperService mapperService) {
super(name, delegate);
this.mapperService = mapperService;
}

Check warning on line 50 in server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java#L48-L50

Added lines #L48 - L50 were not covered by tests

@Override
public DocValuesFormat docValuesFormat() {
return new Composite90DocValuesFormat(mapperService);

Check warning on line 54 in server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/codec/composite/Composite99Codec.java#L54

Added line #L54 was not covered by tests
}
}
Loading
Loading