From c2c070b6673f5f36bc7f46ae7046c3201be4dbde Mon Sep 17 00:00:00 2001 From: Dan Wang Date: Fri, 13 Dec 2024 14:21:48 +0800 Subject: [PATCH] feat(duplication): improve `dups` command to make the output info easier to understand (#2167) Improvements for the command of listing duplications have been made in the following aspects: - change the description for the stats of duplication by status and follower cluster to avoid misunderstanding. - add `DS_LOG` as the prerequisite for a duplication to be considered as finished. - introduce a new partition-level column showing the gap between confirmed decree and last committed decree. - fix the options for json format and output file. --- src/shell/commands/duplication.cpp | 40 ++++++++++++++++++------------ 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/shell/commands/duplication.cpp b/src/shell/commands/duplication.cpp index 55c770922e..89156e754d 100644 --- a/src/shell/commands/duplication.cpp +++ b/src/shell/commands/duplication.cpp @@ -135,12 +135,13 @@ void attach_dups_stat(const list_dups_stat &stat, dsn::utils::multi_table_printe printer.add_row_name_and_data("unfinished_app_count", stat.unfinished_app_count); // Add stats for duplications. - printer.add_row_name_and_data("duplication_count", stat.duplication_count); + printer.add_row_name_and_data("total_duplication_count", stat.duplication_count); for (const auto &[status, cnt] : stat.dup_status_stats) { - printer.add_row_name_and_data(fmt::format("{}_count", status), cnt); + printer.add_row_name_and_data(fmt::format("duplication_count_by_status({})", status), cnt); } for (const auto &[remote_cluster, cnt] : stat.dup_remote_cluster_stats) { - printer.add_row_name_and_data(fmt::format("{}_count", remote_cluster), cnt); + printer.add_row_name_and_data( + fmt::format("duplication_count_by_follower_cluster({})", remote_cluster), cnt); } // Add stats for partitions. @@ -191,15 +192,18 @@ void stat_dups(const ls_app_dups_map &app_states, uint32_t progress_gap, list_du } for (const auto &[partition_id, partition_state] : dup.partition_states) { - if (partition_state.last_committed_decree < partition_state.confirmed_decree) { - // This is unlikely to happen. - continue; - } - - if (partition_state.last_committed_decree - partition_state.confirmed_decree <= - progress_gap) { - // This partition is defined as "finished". - continue; + // Only in the status of `DS_LOG`could a duplication be considered as "finished". + if (dup.status == duplication_status::DS_LOG) { + if (partition_state.last_committed_decree < partition_state.confirmed_decree) { + // This is unlikely to happen. + continue; + } + + if (partition_state.last_committed_decree - partition_state.confirmed_decree <= + progress_gap) { + // This partition is defined as "finished". + continue; + } } // Just assign with 1 to dedup, in case calculated multiple times. @@ -231,14 +235,15 @@ void add_titles_for_dups(bool list_partitions, dsn::utils::table_printer &printe printer.add_column("dup_id", tp_alignment::kRight); printer.add_column("create_time", tp_alignment::kRight); printer.add_column("status", tp_alignment::kRight); - printer.add_column("remote_cluster", tp_alignment::kRight); - printer.add_column("remote_app_name", tp_alignment::kRight); + printer.add_column("follower_cluster", tp_alignment::kRight); + printer.add_column("follower_app_name", tp_alignment::kRight); if (list_partitions) { // Partition-level info. printer.add_column("partition_id", tp_alignment::kRight); printer.add_column("confirmed_decree", tp_alignment::kRight); printer.add_column("last_committed_decree", tp_alignment::kRight); + printer.add_column("decree_gap", tp_alignment::kRight); } } @@ -300,6 +305,8 @@ void add_row_for_dups(int32_t app_id, printer.append_data(partition_id); printer.append_data(partition_state.confirmed_decree); printer.append_data(partition_state.last_committed_decree); + printer.append_data(partition_state.last_committed_decree - + partition_state.confirmed_decree); } } @@ -631,8 +638,9 @@ bool ls_dups(command_executor *e, shell_context *sc, arguments args) // All valid parameters and flags are given as follows. static const std::set params = { - "a", "app_name_pattern", "m", "match_type", "g", "progress_gap"}; - static const std::set flags = {"p", "list_partitions", "u", "show_unfinishd"}; + "a", "app_name_pattern", "m", "match_type", "g", "progress_gap", "o", "output"}; + static const std::set flags = { + "p", "list_partitions", "u", "show_unfinishd", "j", "json"}; argh::parser cmd(args.argc, args.argv, argh::parser::PREFER_PARAM_FOR_UNREG_OPTION);