David Ribeiro Alves has posted comments on this change. Change subject: compaction: Add additional validation in DeltaTracker ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/5919/1/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: PS1, Line 177: CHECK_OK weird that this is a check instead of dcheck when this is only executed in debug mode. PS1, Line 181: CHECK_LT add error messages. same below PS1, Line 184: CHECK_GT(first->delta_stats().max_timestamp(), second->delta_stats().max_timestamp()); are we really sure that the timestamps don't overlap here? I ran into a buch of corner cases like this when adding reinserts with fuzz itest. Say that we have a big batch of updates that touch a bunch of rows, then later on a compaction comes that ends up merging the rowsets and thus generating new deltas undo stores. if the batch is large enough there is the possibility that the updates will span more than one delta store i.e. that the max update of the two stores is the same -- To view, visit http://gerrit.cloudera.org:8080/5919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05fed515bc5d6a09484fadb61e73ac1346f6654a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
