David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1601. Delete ancient UNDO delta blocks in the background ......................................................................
Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/4363/11/src/kudu/tserver/tserver_mm_ops.cc File src/kudu/tserver/tserver_mm_ops.cc: PS11, Line 153: void UndoDeltaBlockGCOp::UpdateStats(MaintenanceOpStats* stats) { I see other folks' concerns about running this IO here. If this is a long running tablet server, as it is, this task will hog the maintenance manager getting delta stats for every delta block in every tablet. Even 50 percent of the time is likely too much, particularly because it's hogging the actual scheduler, not running in the background. This will happen every other time that FindBestOp() runs and as long as there is something to gc (data_gc_min_size_mb is > 0, the default). Say that a user sets undo_delta_block_gc_init_budget_millis to 10000 this will make the sheduler not run anything else for that time while there are delta files to open. My suggestion: - Move the IO in UpdateStats() to some ReadDeltaStats() method. - Perform() starts by running ReadDeltaStats() for some budget of time like you have currently (or maybe even none see the end of this comment). - If there are more undos to gc than the minimum (maybe move the flag here) then Perfom() actually gc's something, otherwise it returns. - UpdateStats() sets the op to runnable if a previous perform established that there are more undos to gc than the minimum. If there aren't it still set's the op to runnable with some probability (say 5 percent or something), so that eventually Perform() will run. Initially few of these run since their low pri in the types of ops and start at 0, but over time you get a complete picture of the delta files and stuff gets deleted. Since this is now lower probability and runs on the threadpool wont hog the scheduler you might even consider having one of these per tablet and maybe even having no budget. Finally you could consider merging the flag in the mm and the enable_ flag, (i.e if you set data_gc_min_size_mb to -1 undo gc never gets prioritized) http://gerrit.cloudera.org:8080/#/c/4363/11/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: PS11, Line 355: Look at ops that we can run quickly that free up data on disk. does this have the highest priority right after log retention? we moved those up in pri because of reboot not so much because of actually freeing space (even though they are more likely to free more space than undo gc). I suggest we move this before the perf ops -- To view, visit http://gerrit.cloudera.org:8080/4363 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0309bf7acfb6d018860c80f354012c3500da5c68 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
