Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22079 )
Change subject: [compaction] Add memory tracking for better OOM issues triaging ...................................................................... Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/22079/9/src/kudu/tablet/compaction-highmem-test.cc File src/kudu/tablet/compaction-highmem-test.cc: http://gerrit.cloudera.org:8080/#/c/22079/9/src/kudu/tablet/compaction-highmem-test.cc@261 PS9, Line 261: } // namespace tablet > Do you think it makes sense adding a test scenario to make sure the usage o Ah, I missed the code in the MajorDeltaCompaction's destructor -- with that, such a test doesn't make much sense, actually. So, please ignore this comment then. http://gerrit.cloudera.org:8080/#/c/22079/9/src/kudu/tablet/delta_compaction.cc File src/kudu/tablet/delta_compaction.cc: http://gerrit.cloudera.org:8080/#/c/22079/9/src/kudu/tablet/delta_compaction.cc@268 PS9, Line 268: vector<DeltaKeyAndUpdate> out; Now with DeltaPreparer::delta_blocks_mem_size(), the memory footprint of the vector of similar structures is tracked. Does it makes sense to track memory footprint of this vector as well then? http://gerrit.cloudera.org:8080/#/c/22079/9/src/kudu/tablet/delta_store.h File src/kudu/tablet/delta_store.h: http://gerrit.cloudera.org:8080/#/c/22079/9/src/kudu/tablet/delta_store.h@479 PS9, Line 479: repared_deltas_.size() nit: if tracking just the memory footprint of the vector itself (as implementation in PS9 does), the actual memory footprint of a vector is provided by its capacity, not the current size (I don't see a call to vector::shrink_to_fit() for prepared_deltas_ anywhere): https://en.cppreference.com/w/cpp/container/vector/capacity -- To view, visit http://gerrit.cloudera.org:8080/22079 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic2037582d433730884212e83359bb75bad0d5394 Gerrit-Change-Number: 22079 Gerrit-PatchSet: 9 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: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Fri, 17 Jan 2025 09:00:45 +0000 Gerrit-HasComments: Yes
