Skip to content

Commit

Permalink
addressing review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Bharathwaj G <[email protected]>
  • Loading branch information
bharath-techie authored and sarthakaggarwal97 committed Jul 5, 2024
1 parent 910e863 commit 4232692
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public class CodecService {
* the raw unfiltered lucene default. useful for testing
*/
public static final String LUCENE_DEFAULT_CODEC = "lucene_default";
private final CompositeCodecFactory compositeCodecFactory = new CompositeCodecFactory();

public CodecService(@Nullable MapperService mapperService, IndexSettings indexSettings, Logger logger) {
final MapBuilder<String, Codec> codecs = MapBuilder.<String, Codec>newMapBuilder();
Expand All @@ -76,10 +77,8 @@ public CodecService(@Nullable MapperService mapperService, IndexSettings indexSe
} else {
// 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()) {
CompositeCodecFactory compositeCodecFactory = new CompositeCodecFactory();
codecs.putAll(compositeCodecFactory.getCompositeCodecs(mapperService, logger));
codecs.putAll(compositeCodecFactory.getCompositeIndexCodecs(mapperService, logger));
} else {
codecs.put(DEFAULT_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger));
codecs.put(LZ4, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public class Composite90DocValuesWriter extends DocValuesConsumer {
private final Set<String> compositeFieldSet;

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

public Composite90DocValuesWriter(DocValuesConsumer delegate, SegmentWriteState segmentWriteState, MapperService mapperService)
throws IOException {
Expand All @@ -50,7 +49,7 @@ public Composite90DocValuesWriter(DocValuesConsumer delegate, SegmentWriteState
this.compositeMappedFieldTypes = mapperService.getCompositeFieldTypes();
compositeFieldSet = new HashSet<>();
for (CompositeMappedFieldType type : compositeMappedFieldTypes) {
compositeFieldSet.add(type.name());
compositeFieldSet.addAll(type.fields());
}
}

Expand Down Expand Up @@ -85,14 +84,13 @@ public void addSortedSetField(FieldInfo field, DocValuesProducer valuesProducer)

@Override
public void close() throws IOException {

delegate.close();
}

private void createCompositeIndicesIfPossible(DocValuesProducer valuesProducer, FieldInfo field) throws IOException {
if (compositeFieldSet.isEmpty()) return;
if (compositeFieldSet.contains(field.name)) {
fieldProducerMap.put(field.name, valuesProducer);
fieldToFieldInfoMap.put(field.name, field);
compositeFieldSet.remove(field.name);
}
// we have all the required fields to build composite fields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
public class CompositeCodecFactory {
public CompositeCodecFactory() {}

public Map<String,Codec> getCompositeCodecs(MapperService mapperService, Logger logger) {
public Map<String, Codec> getCompositeIndexCodecs(MapperService mapperService, Logger logger) {
Map<String, Codec> codecs = new HashMap<>();
codecs.put(DEFAULT_CODEC, new Composite99Codec(Lucene99Codec.Mode.BEST_SPEED, mapperService, logger));
codecs.put(LZ4, new Composite99Codec(Lucene99Codec.Mode.BEST_SPEED, mapperService, logger));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ public enum MergeReason {

private final BooleanSupplier idFieldDataEnabled;

private volatile Set<CompositeMappedFieldType> compositeMappedFieldTypes;

public MapperService(
IndexSettings indexSettings,
IndexAnalyzers indexAnalyzers,
Expand Down Expand Up @@ -542,6 +544,9 @@ private synchronized Map<String, DocumentMapper> internalMerge(DocumentMapper ma
}

assert results.values().stream().allMatch(this::assertSerialization);

// initialize composite fields post merge
this.compositeMappedFieldTypes = getCompositeFieldTypesFromMapper();
return results;
}

Expand Down Expand Up @@ -655,6 +660,10 @@ public boolean isCompositeIndexPresent() {
}

public Set<CompositeMappedFieldType> getCompositeFieldTypes() {
return compositeMappedFieldTypes;
}

private Set<CompositeMappedFieldType> getCompositeFieldTypesFromMapper() {
Set<CompositeMappedFieldType> compositeMappedFieldTypes = new HashSet<>();
if (this.mapper == null) {
return Collections.emptySet();
Expand Down
44 changes: 22 additions & 22 deletions server/src/test/java/org/opensearch/index/codec/CodecTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,52 +79,52 @@ public void testDefault() throws Exception {
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_SPEED, codec);
}

public void testBestCompression() throws Exception {
Codec codec = createCodecService(false).codec("best_compression");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec);
}

public void testLZ4() throws Exception {
Codec codec = createCodecService(false).codec("lz4");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_SPEED, codec);
assert codec instanceof PerFieldMappingPostingFormatCodec;
}

public void testZlib() throws Exception {
Codec codec = createCodecService(false).codec("zlib");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec);
assert codec instanceof PerFieldMappingPostingFormatCodec;
}

public void testResolveDefaultCodecsWithCompositeIndex() throws Exception {
CodecService codecService = createCodecService(false, true);
assertThat(codecService.codec("default"), instanceOf(Composite99Codec.class));
}

public void testDefaultWithCompositeIndex() throws Exception {
Codec codec = createCodecService(false, true).codec("default");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_SPEED, codec);
assert codec instanceof Composite99Codec;
}

public void testBestCompression() throws Exception {
Codec codec = createCodecService(false).codec("best_compression");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec);
}

public void testBestCompressionWithCompositeIndex() throws Exception {
Codec codec = createCodecService(false, true).codec("best_compression");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec);
assert codec instanceof Composite99Codec;
}

public void testLZ4() throws Exception {
Codec codec = createCodecService(false).codec("lz4");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_SPEED, codec);
assert codec instanceof PerFieldMappingPostingFormatCodec;
}

public void testLZ4WithCompositeIndex() throws Exception {
Codec codec = createCodecService(false, true).codec("lz4");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_SPEED, codec);
assert codec instanceof Composite99Codec;
}

public void testZlib() throws Exception {
Codec codec = createCodecService(false).codec("zlib");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec);
assert codec instanceof PerFieldMappingPostingFormatCodec;
}

public void testZlibWithCompositeIndex() throws Exception {
Codec codec = createCodecService(false, true).codec("zlib");
assertStoredFieldsCompressionEquals(Lucene99Codec.Mode.BEST_COMPRESSION, codec);
assert codec instanceof Composite99Codec;
}

public void testResolveDefaultCodecsWithCompositeIndex() throws Exception {
CodecService codecService = createCodecService(false, true);
assertThat(codecService.codec("default"), instanceOf(Composite99Codec.class));
}

public void testBestCompressionWithCompressionLevel() {
final Settings settings = Settings.builder()
.put(INDEX_CODEC_COMPRESSION_LEVEL_SETTING.getKey(), randomIntBetween(1, 6))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* 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.datacube.startree;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.codecs.Codec;
import org.apache.lucene.codecs.lucene99.Lucene99Codec;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.SortedNumericDocValuesField;
import org.apache.lucene.index.IndexWriterConfig;
import org.apache.lucene.store.Directory;
import org.apache.lucene.tests.index.BaseDocValuesFormatTestCase;
import org.apache.lucene.tests.index.RandomIndexWriter;
import org.apache.lucene.tests.util.LuceneTestCase;
import org.opensearch.common.Rounding;
import org.opensearch.index.codec.composite.Composite99Codec;
import org.opensearch.index.compositeindex.datacube.DateDimension;
import org.opensearch.index.compositeindex.datacube.Dimension;
import org.opensearch.index.compositeindex.datacube.Metric;
import org.opensearch.index.compositeindex.datacube.MetricStat;
import org.opensearch.index.compositeindex.datacube.NumericDimension;
import org.opensearch.index.compositeindex.datacube.startree.StarTreeField;
import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration;
import org.opensearch.index.mapper.MapperService;
import org.opensearch.index.mapper.StarTreeMapper;

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

import org.mockito.Mockito;

/**
* Star tree doc values Lucene tests
*/
@LuceneTestCase.SuppressSysoutChecks(bugUrl = "we log a lot on purpose")
public class StarTreeDocValuesFormatTests extends BaseDocValuesFormatTestCase {
@Override
protected Codec getCodec() {
MapperService service = Mockito.mock(MapperService.class);
Mockito.when(service.getCompositeFieldTypes()).thenReturn(Set.of(getStarTreeFieldType()));
final Logger testLogger = LogManager.getLogger(StarTreeDocValuesFormatTests.class);
return new Composite99Codec(Lucene99Codec.Mode.BEST_SPEED, service, testLogger);
}

private StarTreeMapper.StarTreeFieldType getStarTreeFieldType() {
List<MetricStat> m1 = new ArrayList<>();
m1.add(MetricStat.MAX);
Metric metric = new Metric("sndv", m1);
List<Rounding.DateTimeUnit> d1CalendarIntervals = new ArrayList<>();
d1CalendarIntervals.add(Rounding.DateTimeUnit.HOUR_OF_DAY);
StarTreeField starTreeField = getStarTreeField(d1CalendarIntervals, metric);

return new StarTreeMapper.StarTreeFieldType("star_tree", starTreeField);
}

private static StarTreeField getStarTreeField(List<Rounding.DateTimeUnit> d1CalendarIntervals, Metric metric1) {
DateDimension d1 = new DateDimension("field", d1CalendarIntervals);
NumericDimension d2 = new NumericDimension("dv");

List<Metric> metrics = List.of(metric1);
List<Dimension> dims = List.of(d1, d2);
StarTreeFieldConfiguration config = new StarTreeFieldConfiguration(
100,
Collections.emptySet(),
StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP
);

return new StarTreeField("starTree", dims, metrics, config);
}

public void testStarTreeDocValues() throws IOException {
Directory directory = newDirectory();
IndexWriterConfig conf = newIndexWriterConfig(null);
conf.setMergePolicy(newLogMergePolicy());
RandomIndexWriter iw = new RandomIndexWriter(random(), directory, conf);
Document doc = new Document();
doc.add(new SortedNumericDocValuesField("sndv", 1));
doc.add(new SortedNumericDocValuesField("dv", 1));
doc.add(new SortedNumericDocValuesField("field", 1));
iw.addDocument(doc);
doc.add(new SortedNumericDocValuesField("sndv", 1));
doc.add(new SortedNumericDocValuesField("dv", 1));
doc.add(new SortedNumericDocValuesField("field", 1));
iw.addDocument(doc);
iw.forceMerge(1);
doc.add(new SortedNumericDocValuesField("sndv", 2));
doc.add(new SortedNumericDocValuesField("dv", 2));
doc.add(new SortedNumericDocValuesField("field", 2));
iw.addDocument(doc);
doc.add(new SortedNumericDocValuesField("sndv", 2));
doc.add(new SortedNumericDocValuesField("dv", 2));
doc.add(new SortedNumericDocValuesField("field", 2));
iw.addDocument(doc);
iw.forceMerge(1);
iw.close();

// TODO : validate star tree structures that got created
directory.close();
}
}

0 comments on commit 4232692

Please sign in to comment.