Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(duplication): improve dups command to make the output info easier to understand #2167

Merged
merged 6 commits into from
Dec 13, 2024
Merged

feat(duplication): improve dups command to make the output info easier to understand #2167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7dc381e
feat(duplication): improve the display of `dups` command
empiredan Dec 12, 2024
2713740
format
empiredan Dec 12, 2024
7622b96
fix IWYU
empiredan Dec 12, 2024
503a498
fix params and flags
empiredan Dec 12, 2024
0b0489f
fix comments and IWYU
empiredan Dec 12, 2024
4784418
fix IWYU
empiredan Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/replica/replica_backup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include <map>
#include <memory>
#include <string>
#include <type_traits>
#include <utility>
#include <vector>

Expand Down
40 changes: 24 additions & 16 deletions src/shell/commands/duplication.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the row names are too long, the displayed table row may be too wide and hard to be filled on screen ?

Could you please give a sampe output?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it's key/value pair under a summary tag:

[summary]
total_app_count              : 8
duplicating_app_count        : 3
unfinished_app_count         : 0
duplication_count            : 3
DS_LOG_count                 : 3
target_cluster_count         : 3
total_partition_count        : 64
duplicating_partition_count  : 24
unfinished_partition_count   : 0

}

// Add stats for partitions.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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<std::string> params = {
"a", "app_name_pattern", "m", "match_type", "g", "progress_gap"};
static const std::set<std::string> flags = {"p", "list_partitions", "u", "show_unfinishd"};
"a", "app_name_pattern", "m", "match_type", "g", "progress_gap", "o", "output"};
static const std::set<std::string> flags = {
"p", "list_partitions", "u", "show_unfinishd", "j", "json"};

argh::parser cmd(args.argc, args.argv, argh::parser::PREFER_PARAM_FOR_UNREG_OPTION);

Expand Down
Loading