Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/22708 )
Change subject: [compaction] Add memory tracking for better OOM issues triaging ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/22708/2/src/kudu/tablet/compaction-test.cc File src/kudu/tablet/compaction-test.cc: http://gerrit.cloudera.org:8080/#/c/22708/2/src/kudu/tablet/compaction-test.cc@355 PS2, Line 355: fs::IOContext io_context({ "test-tablet" }); > i see that in this file we just need to plumb the two new trackers with nul Good catch! It was a left over (from a different approach) that I missed to remove before sending for review. Removed this and all other occurrences. http://gerrit.cloudera.org:8080/#/c/22708/2/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: http://gerrit.cloudera.org:8080/#/c/22708/2/src/kudu/tablet/compaction.cc@67 PS2, Line 67: namespace kudu { : class MemTracker; : } // namespace kudu > here and in other source files: do we need this forward declaration for Mem Not needed. Removed here and in rowset.cc. http://gerrit.cloudera.org:8080/#/c/22708/2/src/kudu/tablet/compaction.cc@294 PS2, Line 294: StringPrintf > here, and other StringPrintf occurrences: any reason why not Substitute is Good idea! And also there is no argument ordering constraints and is probably faster than StringPrintf. Replaced here and in delta_compaction.cc (memory tracking for major delta) with Substitute. -- To view, visit http://gerrit.cloudera.org:8080/22708 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I700fdf5ee0fbf0f2826b863d23d599f03ad4ac49 Gerrit-Change-Number: 22708 Gerrit-PatchSet: 2 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Mon, 07 Apr 2025 10:37:31 +0000 Gerrit-HasComments: Yes
