Marton Greber 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: Code-Review+1 (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 nullptr such that the test run. Correct me if I'm wrong. Why is the fs::IOContext necessary now? 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 MemTracker? 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 used here? (for me Substitute is a bit more readable) -- 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: Fri, 04 Apr 2025 13:03:53 +0000 Gerrit-HasComments: Yes
