David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1582. Optimize budgeted compaction policy with an approximation ......................................................................
Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/4153/3/src/kudu/tablet/compaction_policy.cc File src/kudu/tablet/compaction_policy.cc: Line 294: // If we know that the upper bound here is worse than our currently known Am I wrong in my understanding that what we're in fact doing here is that we're testing whether our current solution is within the "aproximation ratio" of the upper bound, in which case we've done almost as good as we can do and we skip the knapsack solver. The verbage could be slightly improved to more closely match that (in particular, "upper bound is worse" sounds slightly fishy since it kind of implies that you could do better.) It's also slightly weird that we're iterating through the rowsetinfos above and that it corresponds to a knapsackproblem index. Could you change the for look to use an index and use the index to get the rsi and the corresponding best upper bound? Line 414: // do another pass using the real solver. maybe replace, "do another pass using the real solver" with "do another pass, using the real solver if the best solution we obtained in Pass 1 can be significantly improved" -- To view, visit http://gerrit.cloudera.org:8080/4153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4e611f161d66ddc47e97e3b5328bc1778a4ac958 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Anonymous Coward #174 Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
