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

(7 comments)

Thanks for your reviews.

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

http://gerrit.cloudera.org:8080/#/c/18503/11/src/kudu/tablet/delta_tracker.cc@60
PS11, Line 60: /util/trace.h"
> In fact you can use tablet_history_max_age_sec instead of introducing a new
Done


http://gerrit.cloudera.org:8080/#/c/18503/11/src/kudu/tablet/delta_tracker.cc@60
PS11, Line 60: /util/trace.h"
> nit: all_delete_op_delta_file_outdated_for_compact_sec
Done


http://gerrit.cloudera.org:8080/#/c/18503/11/src/kudu/tablet/delta_tracker.cc@439
PS11, Line 439:
> There are some cases that the compaction not actually run, but you still up
Yes, you made a good suggestion. I deleted some irrelevant updates. Please help 
me to confirm it.


http://gerrit.cloudera.org:8080/#/c/18503/11/src/kudu/tablet/delta_tracker.cc@972
PS11, Line 972:     column_ids_for_compact.insert(schema_column_ids.begin(),
> Sorry, ignore this.
Done


http://gerrit.cloudera.org:8080/#/c/18503/11/src/kudu/tablet/delta_tracker.cc@973
PS11, Line 973:                                 schema_column_ids.end());
              :   }
              :   col_ids->assign(column_ids_for_compact.begin(), 
column_ids_for_compact.end());
              : }
              :
> I meant use:
Done


http://gerrit.cloudera.org:8080/#/c/18503/11/src/kudu/tablet/delta_tracker.cc@973
PS11, Line 973:                                 schema_column_ids.end());
              :   }
              :   col_ids->assign(column_ids_for_compact.begin(), 
column_ids_for_compact.end());
              : }
              :
> nit: these code can be simplyfied.
Done


http://gerrit.cloudera.org:8080/#/c/18503/11/src/kudu/tablet/delta_tracker.cc@980
PS11, Line 980:  (stores == UNDOS_AND_REDOS || stores == UNDOS_ONLY) {
              :     for (const shared_ptr<DeltaStore>& ds : undo_delta_stores_) 
{
              :
> nit: how about column_ids_for_compact.insert(column_ids_for_compact.begin()
Done



-- 
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: 12
Gerrit-Owner: KeDeng <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[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, 28 Nov 2022 07:53:58 +0000
Gerrit-HasComments: Yes

Reply via email to