KeDeng 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 8: (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: whic > which Done 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: // but only any updates that correspond to column 'col_idx' of 'dst_schema'. : // Any indirect data is copied into 'arena' if non-NULL. : > Is it needed to insert when failed? I am very sorry that I found my error when adding unit tests to the code here. This has been deleted in the latest code. http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/common/row_changelist.h@358 PS6, Line 358: // the new state of the row. If it is an UPDATE, this call has no effect. > Could you add some comments to explain why deal the RCL like this? I am very sorry that I found my error when adding unit tests to the code here. This has been deleted in the latest code. 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: > nit: you can omit the ‘std::’ namespace in the way of 'vector'. I am very sorry that I found my error when adding unit tests to the code here. This has been deleted in the latest code. http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/tablet/deltafile.cc@425 PS6, Line 425: preparer_(std::move(opts)), : prepared_(fal > nit: 's' is not used in the following code, you can simplify the code to on I am very sorry that I found my error when adding unit tests to the code here. This has been deleted in the latest code. http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/tablet/deltafile.cc@435 PS6, Line 435: if (spec) { > Seems this operation is expensive indeed.. I am very sorry that I found my error when adding unit tests to the code here. This has been deleted in the latest code. http://gerrit.cloudera.org:8080/#/c/18503/6/src/kudu/tablet/deltafile.cc@449 PS6, Line 449: RETURN_NOT_OK(dfr_->Init(preparer_.opts().io_context)); > Why always ignore_result? I am very sorry that I found my error when adding unit tests to the code here. This has been deleted in the latest code. -- 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: 8 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: Mon, 30 May 2022 08:01:15 +0000 Gerrit-HasComments: Yes
