Skip to content

Commit 5100ed5

Browse files
spjeganJegan
andauthored
Fixed OOM during error message construction (#790)
Co-authored-by: Jegan <jponrama@netflix.com>
1 parent 81ea130 commit 5100ed5

File tree

4 files changed

+89
-2
lines changed

4 files changed

+89
-2
lines changed

hollow/src/main/java/com/netflix/hollow/api/producer/validation/DuplicateDataDetectionValidator.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
* record's type will never fail.
4747
*/
4848
public class DuplicateDataDetectionValidator implements ValidatorListener {
49+
private static final int MAX_DISPLAYED_DUPLICATE_KEYS = 100;
4950
private static final String DUPLICATE_KEYS_FOUND_ERRRO_MSG_FORMAT =
5051
"Duplicate keys found for type %s. Primarykey in schema is %s. "
5152
+ "Duplicate IDs are: %s";
@@ -189,7 +190,17 @@ public int hashCode() {
189190
}
190191

191192
private String duplicateKeysToString(Collection<Object[]> duplicateKeys) {
192-
return duplicateKeys.stream().map(Arrays::toString).collect(Collectors.joining(","));
193+
long totalCount = duplicateKeys.size();
194+
String duplicateKeysString = duplicateKeys.stream()
195+
.limit(MAX_DISPLAYED_DUPLICATE_KEYS)
196+
.map(Arrays::toString)
197+
.collect(Collectors.joining(", "));
198+
199+
if (totalCount > MAX_DISPLAYED_DUPLICATE_KEYS) {
200+
return String.format("%s ... (showing %d of %d duplicates)",
201+
duplicateKeysString, MAX_DISPLAYED_DUPLICATE_KEYS, totalCount);
202+
}
203+
return duplicateKeysString;
193204
}
194205

195206
/**

hollow/src/main/java/com/netflix/hollow/api/producer/validation/NullPrimaryKeyFieldValidator.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
* instantiating this validator.
2828
*/
2929
public class NullPrimaryKeyFieldValidator implements ValidatorListener {
30+
private static final int MAX_DISPLAYED_NULL_KEYS = 100;
3031
private static final String NAME = NullPrimaryKeyFieldValidator.class.getName();
3132
private static final String NULL_PRIMARY_KEYS_FOUND_ERROR_MSG_FORMAT =
3233
"Null primary key fields found for type %s. Primary Key in schema is %s. "
@@ -171,10 +172,18 @@ private Object getPrimaryKeyFieldValue(HollowObjectTypeReadState typeState, int[
171172
}
172173

173174
private String nullKeysToString(Map<Integer, Object[]> nullPrimaryKeyValues) {
174-
return nullPrimaryKeyValues.entrySet().stream()
175+
long totalCount = nullPrimaryKeyValues.size();
176+
String nullPrimaryKeysString = nullPrimaryKeyValues.entrySet().stream()
177+
.limit(MAX_DISPLAYED_NULL_KEYS)
175178
.map(entry -> {
176179
return "(ordinal=" + entry.getKey() + ", key=" + Arrays.toString(entry.getValue()) + ")";
177180
})
178181
.collect(Collectors.joining(", "));
182+
183+
if (totalCount > MAX_DISPLAYED_NULL_KEYS) {
184+
return String.format("%s ... (showing %d of %d null keys)",
185+
nullPrimaryKeysString, MAX_DISPLAYED_NULL_KEYS, totalCount);
186+
}
187+
return nullPrimaryKeysString;
179188
}
180189
}

hollow/src/test/java/com/netflix/hollow/api/producer/validation/DuplicateDataDetectionValidatorTests.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.netflix.hollow.api.producer.HollowProducer;
2020
import com.netflix.hollow.api.producer.fs.HollowInMemoryBlobStager;
21+
import com.netflix.hollow.core.write.objectmapper.HollowPrimaryKey;
2122
import com.netflix.hollow.test.InMemoryBlobStore;
2223
import org.junit.Assert;
2324
import org.junit.Before;
@@ -44,4 +45,36 @@ public void failTestMissingSchema() {
4445
.endsWith("(see initializeDataModel)"));
4546
}
4647
}
48+
49+
@Test
50+
public void test_errorMessageIsTruncatedWhenThereAreManyDuplicates() {
51+
HollowProducer producer = HollowProducer.withPublisher(blobStore)
52+
.withBlobStager(new HollowInMemoryBlobStager())
53+
.withListener(new DuplicateDataDetectionValidator(TypeWithPrimaryKey.class))
54+
.build();
55+
56+
try {
57+
producer.runCycle(writeState -> {
58+
// Add 1000 records with the same primary key
59+
for (int i = 0; i < 1000; i++) {
60+
writeState.add(new TypeWithPrimaryKey(1, "Duplicate" + i));
61+
}
62+
});
63+
Assert.fail("Expected ValidationStatusException");
64+
} catch (ValidationStatusException expected) {
65+
String message = expected.getValidationStatus().getResults().get(0).getMessage();
66+
Assert.assertTrue(message.contains("showing 100 of 499500 duplicates")); // 499500 is the number of duplicate pairs for 1000 identical keys (n * (n-1)/2)
67+
}
68+
}
69+
70+
@HollowPrimaryKey(fields = {"id"})
71+
static class TypeWithPrimaryKey {
72+
int id;
73+
String name;
74+
75+
TypeWithPrimaryKey(int id, String name) {
76+
this.id = id;
77+
this.name = name;
78+
}
79+
}
4780
}

hollow/src/test/java/com/netflix/hollow/api/producer/validation/NullPrimaryKeyFieldValidatorTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,29 @@ public void testInvalidNullNestedPrimaryKey() {
154154
}
155155
}
156156

157+
@Test
158+
public void test_errorMessageIsTruncatedWhenThereAreManyNullKeys() {
159+
HollowProducer producer = HollowProducer.withPublisher(blobStore)
160+
.withBlobStager(new HollowInMemoryBlobStager())
161+
.withListener(new NullPrimaryKeyFieldValidator(TypeWithNullablePrimaryKeyAndData.class))
162+
.build();
163+
164+
try {
165+
producer.runCycle(state -> {
166+
// Add 1000 records with null primary keys but different data fields
167+
for (int i = 0; i < 1000; i++) {
168+
state.add(new TypeWithNullablePrimaryKeyAndData(null, "data" + i));
169+
}
170+
});
171+
fail("Expected ValidationStatusException");
172+
} catch (Exception e) {
173+
assertTrue(e instanceof ValidationStatusException);
174+
ValidationStatusException expected = (ValidationStatusException) e;
175+
String message = expected.getValidationStatus().getResults().get(0).getMessage();
176+
assertTrue(message.contains("showing 100 of 1000 null keys"));
177+
}
178+
}
179+
157180

158181
@HollowPrimaryKey(fields = "id")
159182
static class TypeWithSinglePrimaryKey {
@@ -191,4 +214,15 @@ public TypeWithoutPrimaryKey(Integer id) {
191214
this.id = id;
192215
}
193216
}
217+
218+
@HollowPrimaryKey(fields = "id")
219+
static class TypeWithNullablePrimaryKeyAndData {
220+
private final Integer id;
221+
private final String data;
222+
223+
public TypeWithNullablePrimaryKeyAndData(Integer id, String data) {
224+
this.id = id;
225+
this.data = data;
226+
}
227+
}
194228
}

0 commit comments

Comments
 (0)