Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18503 )
Change subject: [KUDU-3367][compaction] Fix delta file with full of delete op can not be schedule to compact ...................................................................... Patch Set 6: (7 comments) http://gerrit.cloudera.org:8080/#/c/18503/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18503/6//COMMIT_MSG@9 PS6, Line 9: wich which http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/common/row_changelist.h File src/kudu/common/row_changelist.h: http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/common/row_changelist.h@350 PS6, Line 350: dec.col_id = id; : column_ids->insert(dec.col_id); : Is it needed to insert when failed? http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/common/row_changelist.h@358 PS6, Line 358: } Could you add some comments to explain why deal the RCL like this? http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/tablet/deltafile.cc File src/kudu/tablet/deltafile.cc: http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/tablet/deltafile.cc@420 PS6, Line 420: std::set nit: you can omit the ‘std::’ namespace in the way of 'vector'. http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/tablet/deltafile.cc@425 PS6, Line 425: Status s = NewDeltaIterator(opts, &iter); : RETURN_NOT_OK(s); nit: 's' is not used in the following code, you can simplify the code to one line. http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/tablet/deltafile.cc@435 PS6, Line 435: while (iter->HasNext()) { Seems this operation is expensive indeed.. http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/tablet/deltafile.cc@449 PS6, Line 449: ignore_result(decoder.GetAllIncludedColumnIds(col_ids)); Why always ignore_result? -- To view, visit http://gerrit.cloudera.org:8080/18503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b26737dffecc17688b42188da959b2ba16351ed Gerrit-Change-Number: 18503 Gerrit-PatchSet: 6 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Sun, 29 May 2022 09:21:32 +0000 Gerrit-HasComments: Yes
