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 <t...@apache.org>
Gerrit-Reviewer: Anonymous Coward #174
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to