Skip to content

Commit

Permalink
Bugfix: HollowIncrementalProducer was failing with NPE when collectio…
Browse files Browse the repository at this point in the history 
…n element schemas are not yet defined
  • Loading branch information
@dkoszewnik
dkoszewnik committed Jun 14, 2023
1 parent 203046c commit d564dac
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 22 deletions.
48 changes: 26 additions & 22 deletions hollow/src/main/java/com/netflix/hollow/tools/traverse/TransitiveSetTraverser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,26 +183,28 @@ private static void addTransitiveMatches(HollowReadStateEngine stateEngine, Holl
BitSet matchingOrdinals = getOrCreateBitSet(matches, schema.getName(), typeState.maxOrdinal());

HollowTypeReadState childTypeState = stateEngine.getTypeState(schema.getElementType());
BitSet childOrdinals = getOrCreateBitSet(matches, schema.getElementType(), childTypeState.maxOrdinal());

int ordinal = matchingOrdinals.nextSetBit(0);
while(ordinal != -1) {
try {
HollowOrdinalIterator iter = typeState.ordinalIterator(ordinal);
int elementOrdinal = iter.next();
while(elementOrdinal != HollowOrdinalIterator.NO_MORE_ORDINALS) {
childOrdinals.set(elementOrdinal);
elementOrdinal = iter.next();
if(childTypeState != null && childTypeState.maxOrdinal() >= 0) {
BitSet childOrdinals = getOrCreateBitSet(matches, schema.getElementType(), childTypeState.maxOrdinal());

int ordinal = matchingOrdinals.nextSetBit(0);
while(ordinal != -1) {
try {
HollowOrdinalIterator iter = typeState.ordinalIterator(ordinal);
int elementOrdinal = iter.next();
while(elementOrdinal != HollowOrdinalIterator.NO_MORE_ORDINALS) {
childOrdinals.set(elementOrdinal);
elementOrdinal = iter.next();
}
} catch(Exception e) {
log.log(Level.SEVERE, "Add transitive matches failed", e);
}
} catch(Exception e) {
log.log(Level.SEVERE, "Add transitive matches failed", e);

ordinal = matchingOrdinals.nextSetBit(ordinal + 1);
}

if(!childOrdinals.isEmpty()) {
matches.put(schema.getElementType(), childOrdinals);
}

ordinal = matchingOrdinals.nextSetBit(ordinal + 1);
}

if(!childOrdinals.isEmpty()) {
matches.put(schema.getElementType(), childOrdinals);
}
}

Expand All @@ -213,15 +215,17 @@ private static void addTransitiveMatches(HollowReadStateEngine stateEngine, Holl
HollowTypeReadState keyTypeState = stateEngine.getTypeState(schema.getKeyType());
HollowTypeReadState valueTypeState = stateEngine.getTypeState(schema.getValueType());

BitSet keyOrdinals = getOrCreateBitSet(matches, schema.getKeyType(), keyTypeState.maxOrdinal());
BitSet valueOrdinals = getOrCreateBitSet(matches, schema.getValueType(), valueTypeState.maxOrdinal());
BitSet keyOrdinals = keyTypeState == null || keyTypeState.maxOrdinal() < 0 ? null : getOrCreateBitSet(matches, schema.getKeyType(), keyTypeState.maxOrdinal());
BitSet valueOrdinals = valueTypeState == null || valueTypeState.maxOrdinal() < 0 ? null : getOrCreateBitSet(matches, schema.getValueType(), valueTypeState.maxOrdinal());

int ordinal = matchingOrdinals.nextSetBit(0);
while(ordinal != -1) {
HollowMapEntryOrdinalIterator iter = typeState.ordinalIterator(ordinal);
while(iter.next()) {
keyOrdinals.set(iter.getKey());
valueOrdinals.set(iter.getValue());
if(keyOrdinals != null)
keyOrdinals.set(iter.getKey());
if(valueOrdinals != null)
valueOrdinals.set(iter.getValue());
}

ordinal = matchingOrdinals.nextSetBit(ordinal + 1);
Expand Down
62 changes: 62 additions & 0 deletions hollow/src/test/java/com/netflix/hollow/api/producer/HollowIncrementalProducerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,22 @@
import com.netflix.hollow.core.read.engine.HollowReadStateEngine;
import com.netflix.hollow.core.read.engine.HollowTypeReadState;
import com.netflix.hollow.core.util.AllHollowRecordCollection;
import com.netflix.hollow.core.write.HollowWriteStateEngine;
import com.netflix.hollow.core.write.objectmapper.HollowObjectMapper;
import com.netflix.hollow.core.write.objectmapper.HollowPrimaryKey;
import com.netflix.hollow.core.write.objectmapper.HollowTypeName;
import com.netflix.hollow.core.write.objectmapper.RecordPrimaryKey;
import com.netflix.hollow.core.write.objectmapper.flatrecords.FakeHollowSchemaIdentifierMapper;
import com.netflix.hollow.core.write.objectmapper.flatrecords.FlatRecordWriter;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import org.junit.Assert;
import org.junit.Before;
Expand Down Expand Up @@ -161,6 +168,61 @@ public void addIfAbsentWillInitializeNewRecordsButNotOverwriteExistingRecords()
assertTypeA(idx, 102, "one hundred and two", 9995L);
assertTypeA(idx, 103, "one hundred and three", 9992L);
}

@Test
public void updateStateWithUnavailableCollectionElementTypes() {
// Producer is created but not initialized IncrementalProducer will directly initialize the first snapshot
HollowProducer producer = createInMemoryProducer();
HollowWriteStateEngine dataset = new HollowWriteStateEngine();
HollowObjectMapper mapper = new HollowObjectMapper(dataset);
mapper.doNotUseDefaultHashKeys();
mapper.initializeTypeState(TypeWithChildCollections.class);

producer.initializeDataModel(dataset.getSchema("TypeWithChildCollections"), dataset.getSchema("SetOfElementType"), dataset.getSchema("ListOfElementType"), dataset.getSchema("MapOfElementTypeToElementType"));

/// add/modify state of a producer with an empty previous state. delete requests for non-existent records will be ignored
HollowIncrementalProducer incrementalProducer = new HollowIncrementalProducer(producer);

FlatRecordWriter writer = new FlatRecordWriter(dataset, new FakeHollowSchemaIdentifierMapper(dataset));
mapper.writeFlat(new TypeWithChildCollections(100, null), writer);

incrementalProducer.addOrModify(writer.generateFlatRecord());

long version = incrementalProducer.runCycle();

incrementalProducer.delete(new RecordPrimaryKey("TypeWithChildCollections", new Object[] {100}));
writer.reset();
mapper.writeFlat(new TypeWithChildCollections(101, null), writer);
incrementalProducer.addOrModify(writer.generateFlatRecord());

long nextVersion = incrementalProducer.runCycle();
Assert.assertTrue(nextVersion > 0);
}

@HollowPrimaryKey(fields="id")
public static class TypeWithChildCollections {
int id;
Set<ElementType> set;
List<ElementType> list;
Map<ElementType, ElementType> map;

public TypeWithChildCollections(int id, Integer val) {
ElementType el = val == null ? null : new ElementType(val);
this.id = id;
this.set = el == null ? Collections.emptySet() : Collections.singleton(el);
this.list = el == null ? Collections.emptyList() : Collections.singletonList(el);
this.map = el == null ? Collections.emptyMap() : Collections.singletonMap(el, el);
}
}

public static class ElementType {
int value;

public ElementType(int value) {
this.value = value;
}
}


@Test
public void publishAndLoadASnapshotDirectly() {
Expand Down

0 comments on commit d564dac

Please sign in to comment.