David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1601. Delete ancient UNDO delta blocks in the background
......................................................................


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4363/17/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:

PS17, Line 383:   // First, find the boundary of where we should stop 
initializing.
              :   // We use binary search to find the oldest delta store with 
max_timestamp >= AHM.
              :   // Using binary search here allows us to get a better 
estimate from
              :   // EstimateBytesInPotentiallyAncientUndoDeltas() later.
              :   if (ancient_history_mark != Timestamp::kInvalidTimestamp) {
              :     size_t pos_lower_bound_incl = 0;
              :     size_t pos_upper_bound_excl = undos_newest_first.size();
              :     while (pos_lower_bound_incl < pos_upper_bound_excl) {
              :       if (deadline.Initialized() && MonoTime::Now() >= 
deadline) break;
              :       size_t midpoint = (pos_lower_bound_incl + 
pos_upper_bound_excl) / 2;
              :       auto& undo = undos_newest_first[midpoint];
              :       if (!undo->Initted()) {
              :         RETURN_NOT_OK(undo->Init());
              :       }
              :       if (undo->delta_stats().max_timestamp() < 
ancient_history_mark) {
              :         pos_upper_bound_excl = midpoint; // Undos are stored 
newest-first.
              :       } else {
              :         pos_lower_bound_incl = midpoint;
              :       }
              :     }
              :   }
think you might have forgotten to post the replies to my comments. It's still 
not totally clear to me why and how much binary search helps here. If we have a 
long undo history the we'd find the last one we could delete (which could 
change the next time we did this) but then below we go at it from the first 
one. Unless you have some numbers showing this helps I'm pro keeping it simple.


-- 
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: 17
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

Reply via email to