vinothchandar commented on a change in pull request #3955:
URL: https://github.com/apache/hudi/pull/3955#discussion_r746734023



##########
File path: 
hudi-common/src/test/java/org/apache/hudi/common/util/collection/TestExternalSpillableMap.java
##########
@@ -339,7 +343,42 @@ public void 
testDataCorrectnessWithoutHoodieMetadata(ExternalSpillableMap.DiskMa
 
   // TODO : come up with a performance eval test for spillableMap
   @Test
-  public void testLargeInsertUpsert() {}
+  public void testLargeInsertUpsert() {
+  }
+
+  @Test
+  public void testPayloadSizeEstimate() throws IOException, URISyntaxException 
{
+    final ExternalSpillableMap.DiskMapType diskMapType = 
ExternalSpillableMap.DiskMapType.BITCASK;
+    final boolean isCompressionEnabled = false;
+    final Schema schema = SchemaTestUtil.getSimpleSchema();
+
+    ExternalSpillableMap<String, HoodieRecord<? extends HoodieRecordPayload>> 
records =
+        new ExternalSpillableMap<>(16L, basePath, new DefaultSizeEstimator(),
+            new HoodieRecordSizeEstimator(schema), diskMapType, 
isCompressionEnabled);
+
+    List<String> recordKeys = new ArrayList<>();
+
+    // Put a single record. Payload size estimation happens as part of this 
initial put.
+    HoodieRecord seedRecord = 
SchemaTestUtil.generateHoodieTestRecordsWithoutHoodieMetadata(0, 1).get(0);
+    records.put(seedRecord.getRecordKey(), seedRecord);
+
+    // Remove the key immediately to make the map empty again.
+    records.remove(seedRecord.getRecordKey());
+
+    // Payload size re-estimation should not happen as the map
+    // size has not reached the minimum size threshold for
+    // recalculation.
+    records.put(seedRecord.getRecordKey(), seedRecord);
+
+    // Put more records than the threshold to trigger payload size 
re-estimation
+    while (records.getDiskBasedMapNumEntries() < 1) {

Review comment:
       do we need the lines 371-379. they are just trying to validate no error 
is thrown? in general, tests should have clear asserts. My suggestion would be 
to scope down this test to just testing the divide by zero scenario. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to