[GitHub] [iceberg] rdblue commented on a change in pull request #1991: Avro: Schema conversions should preserve field docs

[GitBox <gi...@apache.org>]

rdblue commented on a change in pull request #1991:
URL: https://github.com/apache/iceberg/pull/1991#discussion_r549450197



##########
File path: core/src/test/java/org/apache/iceberg/TestMergeAppend.java
##########
@@ -239,8 +239,10 @@ public void testMergeWithExistingManifest() {
   public void testManifestMergeMinCount() throws IOException {
     Assert.assertEquals("Table should start empty", 0, listManifestFiles().size());
     table.updateProperties().set(TableProperties.MANIFEST_MIN_MERGE_COUNT, "2")
-        // each manifest file is 5227 bytes, so 12000 bytes limit will give us 2 bins with 3 manifest/data files.
-        .set(TableProperties.MANIFEST_TARGET_SIZE_BYTES, "12000")
+        // Each initial v1/v2 ManifestFile is 5661/6397 bytes respectively. Merging two of the given
+        // manifests make one v1/v2 ManifestFile of 5672/6408 bytes respectively, so 12850 bytes
+        // limit will give us two bins with three manifest/data files.
+        .set(TableProperties.MANIFEST_TARGET_SIZE_BYTES, "12850")

Review comment:
       3x the smaller size would be around 17k, and we need it to be at least about 13k, which is 2x the larger size. I'd probably set this to 15k to split the difference and hopefully avoid needing to update this again as tests change. This is minor, though.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org