Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24412 )
Change subject: KUDU-3429 Add extensive test for delta generation (3/n) ...................................................................... Patch Set 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/24412/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24412/2//COMMIT_MSG@12 PS2, Line 12: eligibile > eligible Done http://gerrit.cloudera.org:8080/#/c/24412/2//COMMIT_MSG@17 PS2, Line 17: * Use pseudo-random byte pattern to minimize the effect of compression. : If there are repeated characters, compression would bring memory : requirements of a CFile block memory to a minimum, keeping RSS flat : during compaction. > I'm not sure I understand this logic. If there is some compression, then I Sorry for the confusion here. It really doesn't matter whether the byte pattern is random or not because the buffers (that last during rowset compaction lifecycle) to hold the data from deltas are holding uncompressed version of it. So, for both the cases, the size of data (in-memory, not on-disk) as well RSS would remain the same. The main reason to have pseudo-random pattern is for the following reasons: 1. Total time taken by pseudo-random byte pattern case is less, possibly because of time saved during read when it turns into a trivial buffer copy. 2. It keeps the estimation values in assertions close in the code. One could argue that disk space consumption-wise, repeated-char payload might seem better but the aim here is not to save disk space but ensure more RSS usage. Made some corrections to wording here to remove any confusion. http://gerrit.cloudera.org:8080/#/c/24412/2//COMMIT_MSG@21 PS2, Line 21: defacto > nit: usually it's spelled as two words, i.e. 'de facto' Done http://gerrit.cloudera.org:8080/#/c/24412/2/src/kudu/tablet/compaction-highmem-test.cc File src/kudu/tablet/compaction-highmem-test.cc: http://gerrit.cloudera.org:8080/#/c/24412/2/src/kudu/tablet/compaction-highmem-test.cc@77 PS2, Line 77: // Wide column used by GenHighMemConsumptionDeltasWithFrequentFlush to make : // each delta mutation ~1 KB on disk. This lets the test reach a large total : // delta volume with fewer tablet operations in a short span of time. : // Existing tests leave this column NULL so their behaviour is unchanged. : CHECK_OK(builder.AddNullableColumn("large_val", STRING)); > Did you explore specifying some custom/non-default type of encodings for th For delta files, it is always PLAIN_ENCODING regardless of whether any encoding type is chosen during during schema creation or not. The explicit encoding only applies to base data which only a small fraction of total size of block data that is read into memory during compaction. I haven't done any targeted analysis but PLAIN_ENCODING should result in one of the most (if not the most) memory usage encoding type for STRING, BINARY, INT32, INT64 mostly because it doesn't apply any de-duplication or prefix compression. The most contributing column (in terms of memory consumption) here is 'large_val' that is of type STRING and default encoding set for this column is AUTO_ENCODING which may or may not be the biggest consumer of memory but since this is only for base data, it wouldn't have valuable impact on the total usage. In other words, PLAIN_ENCODING (that is hard-coded for delats) on STRING type column already makes delta sizes the largest. For base data, it wouldn't matter much because those only account for a small fraction of the total size. http://gerrit.cloudera.org:8080/#/c/24412/2/src/kudu/tablet/compaction-highmem-test.cc@471 PS2, Line 471: … > Here and elsewhere in this file: please avoid using non-ASCII characters in Done http://gerrit.cloudera.org:8080/#/c/24412/2/src/kudu/tablet/compaction-highmem-test.cc@498 PS2, Line 498: TestHighDeltaVolumeWithFrequentFlushForRowsetCompaction > nit: there isn't much need in adding 'Test' into the scenario name, it's al Done http://gerrit.cloudera.org:8080/#/c/24412/2/src/kudu/tablet/compaction-highmem-test.cc@501 PS2, Line 501: 30-40 seconds > How long does it take to run with TSAN instrumentation? It makes sense to skip this test for both ASAN and TSAN build. The run time for TSAN build is quite high comparatively (~ 10 minutes on my local linux system). Could be much more on dist cluster slaves. For ASAN build, dist slaves are killing the test with OOM error, likely because of extra padding of redzones with every heap allocations. The memory overhead for ASAN is about 400MB that pushes it over the physical limit on dist cluster slaves. Runtime is also considerably high. Debug and Release just made it within the memory limits on dist cluster. 50 iterations of both ran fine. http://gerrit.cloudera.org:8080/#/c/24412/2/src/kudu/tablet/compaction-highmem-test.cc@546 PS2, Line 546: ASSERT_STR_MATCHES( : JoinStrings(sink.logged_msgs(), "\n"), : "beyond hard memory limit of.*Rowset merge compaction ops consumption:"); > Since the hard memory limit is set deterministically to 1GiB, is there any There is no threshold expectation here. The assert is to just record the fact that current delta generation takes compaction memory usage beyond the hard memory limit set in the test. For the record, it goes over the limit of 1GiB by a significant margin of ~10GiB. I don't think any assert is required for the threshold because this is just a reference for any testing required for custom sized delta generation scenario. -- To view, visit http://gerrit.cloudera.org:8080/24412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c337dba87a9d1cf549f9b53813da8d50e86b6b4 Gerrit-Change-Number: 24412 Gerrit-PatchSet: 2 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Mon, 13 Jul 2026 11:22:13 +0000 Gerrit-HasComments: Yes
