David Ribeiro Alves has posted comments on this change. Change subject: KUDU-237 (part 1) - Support proper mutation encoding for reinserts ......................................................................
Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/4791/5/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: Line 728: // row but disregard the undo reinsert that is created. > Given that this is a fairly rare case, I think we should go for the cleaner Just some final thoughts on the alternate implementation to make sure we're on the same page: We currently keep DELETEs as REDOs and we likely don't want to generate a REINSERT for all deletes and keep it on the side in case it's needed, right? (just for the ones that are followed by a REINSERT). So, in this if block, we would run ApplyRowChanges on the REDO delete (which would be a special case because we would then discard it) to get the REINSERT and then run it again on the REINSERT to apply the new state to the row and get the new DELETE. We would then still do the dance where we discard the REDO delete and assemble the UNDO list with the new UNDO REINSERT and UNDO DELETE. Maybe we should change the name of ApplyRowChanges or add an alternate method then? Feels unnatural to ApplyRowChanges on a delete (not applying anything to 'dst_row'). -- To view, visit http://gerrit.cloudera.org:8080/4791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2b0a95e3f618bf20eb18ca4f4b03b9e2ce5704e8 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <[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-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
