fix

src/client/replication_ddl_client.cpp

@@ -318,8 +318,7 @@ error_s replication_ddl_client::list_apps(dsn::app_status::type status,
     return list_apps(status, {}, utils::pattern_match_type::PMT_MATCH_ALL, apps);
 }
 
-error_s replication_ddl_client::list_apps(bool show_all,
-                                          bool detailed,
+error_s replication_ddl_client::list_apps(bool detailed,
                                           bool json,
                                           const std::string &output_file,
                                           dsn::app_status::type status,
@@ -336,9 +335,6 @@ error_s replication_ddl_client::list_apps(bool show_all,
 
     size_t max_app_name_size = 20;
     for (const auto &app : apps) {
-        if (!show_all && app.status != app_status::AS_AVAILABLE) {
-            continue;
-        }
         max_app_name_size = std::max(max_app_name_size, app.app_name.size() + 2);
     }
 
@@ -357,11 +353,7 @@ error_s replication_ddl_client::list_apps(bool show_all,
     tp_general.add_column("envs_count");
 
     int available_app_count = 0;
-    for (int i = 0; i < apps.size(); i++) {
-        dsn::app_info info = apps[i];
-        if (!show_all && info.status != app_status::AS_AVAILABLE) {
-            continue;
-        }
+    for (const auto &info : apps) {
         std::string status_str = enum_to_string(info.status);
         status_str = status_str.substr(status_str.find("AS_") + 3);
         std::string create_time = "-";
@@ -416,7 +408,7 @@ error_s replication_ddl_client::list_apps(bool show_all,
         tp_health.add_column("unhealthy");
         tp_health.add_column("write_unhealthy");
         tp_health.add_column("read_unhealthy");
-        for (auto &info : apps) {
+        for (const auto &info : apps) {
             if (info.status != app_status::AS_AVAILABLE) {
                 continue;
             }
@@ -489,19 +481,13 @@ error_s replication_ddl_client::list_apps(bool show_all,
     return error_s::ok();
 }
 
-error_s replication_ddl_client::list_apps(bool show_all,
-                                          bool detailed,
+error_s replication_ddl_client::list_apps(bool detailed,
                                           bool json,
                                           const std::string &output_file,
                                           const dsn::app_status::type status)
 {
-    return list_apps(show_all,
-                     detailed,
-                     json,
-                     output_file,
-                     status,
-                     {},
-                     utils::pattern_match_type::PMT_MATCH_ALL);
+    return list_apps(
+        detailed, json, output_file, status, {}, utils::pattern_match_type::PMT_MATCH_ALL);
 }
 
 dsn::error_code replication_ddl_client::list_nodes(
@@ -1406,6 +1392,9 @@ dsn::error_code replication_ddl_client::get_app_envs(const std::string &app_name
     }
 
     for (const auto &app : apps) {
+        // Once the meta server does not support `app_name` and `match_type` (still the old
+        // version) for `RPC_CM_LIST_APPS`, the response would include all available tables.
+        // Thus here we should still check if the table name in the response is the target.
         if (app.app_name != app_name) {
             continue;
         }

src/client/replication_ddl_client.h

@@ -99,16 +99,13 @@ class replication_ddl_client
     // `output_file` is empty, tables would be listed to stdout.
     //
     // Choose tables according to following parameters:
-    // `show_all`: whether to show all tables, not only the available, but also the ones in
-    // other status, e.g. the dropped tables.
     // `detailed`: whether to show healthy/unhealthy details.
     // `json`: whether to output as json format.
     // `status`: the status of the tables chosen to be listed. `app_status::AS_INVALID` means
     // no restriction.
     // `app_name_pattern`: the name pattern of the tables chosen to be listed.
     // `match_type`: the type in which the name pattern would be matched.
-    error_s list_apps(bool show_all,
-                      bool detailed,
+    error_s list_apps(bool detailed,
                       bool json,
                       const std::string &output_file,
                       dsn::app_status::type status,
@@ -117,8 +114,7 @@ class replication_ddl_client
 
     // The same as the above, except that there's no restriction on table name; in other
     // words, the match type is `PMT_MATCH_ALL`.
-    error_s list_apps(bool show_all,
-                      bool detailed,
+    error_s list_apps(bool detailed,
                       bool json,
                       const std::string &output_file,
                       dsn::app_status::type status);
@@ -363,7 +359,9 @@ class replication_ddl_client
         return request_meta(code, req, timeout_milliseconds, 0);
     }
 
-    // The same as the above, except that the timeout for the RPC request is set to 0.
+    // The same as the above, except that `timeout_milliseconds` for the RPC request is set to
+    // 0, which means `rpc_timeout_milliseconds` configured for each task would be used as the
+    // timeout. See `message_ex::create_request()` for details.
     template <typename TRequest>
     rpc_response_task_ptr request_meta(const dsn::task_code &code, std::shared_ptr<TRequest> &req)
     {
@@ -451,7 +449,9 @@ class replication_ddl_client
         return request_meta_and_wait_response(code, req, resp, timeout_milliseconds, 0);
     }
 
-    // The same as the above, except that the timeout for the RPC request is set to 0.
+    // The same as the above, except that `timeout_milliseconds` for the RPC request is set to
+    // 0, which means `rpc_timeout_milliseconds` configured for each task would be used as the
+    // timeout. See `message_ex::create_request()` for details.
     template <typename TRequest, typename TResponse>
     error_s request_meta_and_wait_response(const dsn::task_code &code,
                                            std::shared_ptr<TRequest> &req,

src/meta/duplication/meta_duplication_service.cpp

@@ -107,25 +107,17 @@ void meta_duplication_service::list_duplication_info(const duplication_list_requ
             continue;
         }
 
-        const auto &err =
+        const auto &result =
             utils::pattern_match(app_name, request.app_name_pattern, request.match_type);
-        if (err == ERR_NOT_MATCHED) {
+        if (result.code() == ERR_NOT_MATCHED) {
             continue;
         }
 
-        if (err == ERR_NOT_IMPLEMENTED) {
-            const auto &msg = fmt::format("match_type {} is not supported now",
-                                          static_cast<int>(request.match_type));
-            response.err = err;
-            response.hint_message = msg;
+        if (result.code() != ERR_OK) {
+            response.err = result.code();
+            response.hint_message = result.message();
+            LOG_ERROR("{}, app_name_pattern={}", result.description(), request.app_name_pattern);
 
-            LOG_ERROR("{}: app_name_pattern={}", msg, request.app_name_pattern);
-
-            return;
-        }
-
-        if (err != ERR_OK) {
-            response.err = err;
             return;
         }
 

src/meta/meta_http_service.cpp

@@ -401,8 +401,8 @@ void meta_http_service::list_node_handler(const http_request &req, http_response
         tmp_map.emplace(node, list_nodes_helper(node.to_string(), "UNALIVE"));
     }
 
-    size_t alive_node_count = (_service->_alive_set).size();
-    size_t unalive_node_count = (_service->_dead_set).size();
+    size_t alive_node_count = _service->_alive_set.size();
+    size_t unalive_node_count = _service->_dead_set.size();
 
     if (detailed) {
         INIT_AND_CALL_LIST_APPS(app_status::AS_AVAILABLE, list_apps_resp, resp);

src/meta/server_state.cpp

@@ -1626,35 +1626,27 @@ void server_state::list_apps(const configuration_list_apps_request &request,
     for (const auto &[_, app] : _all_apps) {
         // Any table chosen to be listed must match the pattern, if any.
         if (request.__isset.app_name_pattern && request.__isset.match_type) {
-            const auto &err =
+            const auto &result =
                 utils::pattern_match(app->app_name, request.app_name_pattern, request.match_type);
-            if (err == ERR_NOT_MATCHED) {
+            if (result.code() == ERR_NOT_MATCHED) {
                 continue;
             }
 
-            if (err == ERR_NOT_IMPLEMENTED) {
-                const auto &msg = fmt::format("match_type {} is not supported now",
-                                              static_cast<int>(request.match_type));
-                response.err = err;
-                response.__set_hint_message(msg);
-
-                LOG_ERROR("{}: app_name_pattern={}", msg, request.app_name_pattern);
+            if (result.code() != ERR_OK) {
+                response.err = result.code();
+                response.__set_hint_message(result.message());
+                LOG_ERROR(
+                    "{}, app_name_pattern={}", result.description(), request.app_name_pattern);
 
                 return;
             }
-
-            if (err != ERR_OK) {
-                response.err = err;
-                return;
-            }
         }
 
         // Only in the following two cases would a table be chosen to be listed, according to
         // the requested status:
-        //
-        // * `app_status::AS_INVALID` means no filter, in other words, any table with any status
+        // - `app_status::AS_INVALID` means no filter, in other words, any table with any status
         // could be chosen;
-        // * or, current status of a table is the same as the requested status.
+        // - or, current status of a table is the same as the requested status.
         if (request.status != app_status::AS_INVALID && request.status != app->status) {
             continue;
         }

src/shell/commands/table_management.cpp

@@ -93,7 +93,13 @@ bool ls_apps(command_executor *e, shell_context *sc, arguments args)
 
     const std::string status_str(cmd({"-s", "--status"}, "").str());
     auto status = dsn::app_status::AS_INVALID;
-    if (!status_str.empty() && status_str != "all") {
+    if (status_str.empty()) {
+        // `show_all` functions only when target `status` is not specified.
+        if (!show_all) {
+            // That `show_all` is not given means just showing available tables.
+            status = dsn::app_status::AS_AVAILABLE;
+        }
+    } else if (status_str != "all") {
         status = type_from_string(dsn::_app_status_VALUES_TO_NAMES,
                                   fmt::format("as_{}", status_str),
                                   dsn::app_status::AS_INVALID);
@@ -111,7 +117,7 @@ bool ls_apps(command_executor *e, shell_context *sc, arguments args)
     PARSE_OPT_ENUM(match_type, dsn::utils::pattern_match_type::PMT_INVALID, {"-m", "--match_type"});
 
     const auto &result = sc->ddl_client->list_apps(
-        show_all, detailed, json, output_file, status, app_name_pattern, match_type);
+        detailed, json, output_file, status, app_name_pattern, match_type);
     if (!result) {
         fmt::println("list apps failed, error={}", result);
     }

src/utils/errors.h

@@ -114,6 +114,8 @@ class error_s
 
     [[nodiscard]] error_code code() const { return _info ? _info->code : ERR_OK; }
 
+    [[nodiscard]] std::string message() const { return _info ? _info->msg : ""; }
+
     error_s &operator<<(const char str[])
     {
         if (_info) {

src/utils/strings.cpp

@@ -114,9 +114,9 @@ bool mequals(const void *lhs, const void *rhs, size_t n)
 
 #undef CHECK_NULL_PTR
 
-error_code pattern_match(const std::string &str,
-                         const std::string &pattern,
-                         pattern_match_type::type match_type)
+error_s pattern_match(const std::string &str,
+                      const std::string &pattern,
+                      pattern_match_type::type match_type)
 {
     bool matched = false;
     switch (match_type) {
@@ -145,10 +145,13 @@ error_code pattern_match(const std::string &str,
     case pattern_match_type::PMT_MATCH_REGEX:
 
     default:
-        return ERR_NOT_IMPLEMENTED;
+        return FMT_ERR(ERR_NOT_IMPLEMENTED,
+                       "match_type is not supported: val={}, str={}",
+                       static_cast<int>(match_type),
+                       enum_to_string(match_type));
     }
 
-    return matched ? ERR_OK : ERR_NOT_MATCHED;
+    return matched ? error_s::ok() : error_s::make(ERR_NOT_MATCHED);
 }
 
 std::string get_last_component(const std::string &input, const char splitters[])

src/utils/strings.h

@@ -36,7 +36,7 @@
 
 #include "utils_types.h"
 #include "utils/enum_helper.h"
-#include "utils/error_code.h"
+#include "utils/errors.h"
 
 namespace dsn::utils {
 
@@ -75,9 +75,14 @@ bool iequals(const char *lhs, const std::string &rhs, size_t n);
 // Decide whether the first n bytes of two memory areas are equal, even if one of them is NULL.
 bool mequals(const void *lhs, const void *rhs, size_t n);
 
-error_code pattern_match(const std::string &str,
-                         const std::string &pattern,
-                         pattern_match_type::type match_type);
+// Try to match target string `str` with provided `pattern` in `match_type`. Return the code
+// representing matched, not matched, or some error with additional message(if any) as follows:
+// - ERR_OK:                matched;
+// - ERR_NOT_MATCHED:       not matched;
+// - ERR_NOT_IMPLEMENTED:   `match_type` is not supported.
+error_s pattern_match(const std::string &str,
+                      const std::string &pattern,
+                      pattern_match_type::type match_type);
 
 // Split the `input` string by the only character `separator` into tokens. Leading and trailing
 // spaces of each token will be stripped. Once the token is empty, or become empty after

src/utils/test/utils.cpp

@@ -313,7 +313,7 @@ TEST_P(PatternMatchTest, PatternMatch)
 {
     const auto &test_case = GetParam();
     const auto actual_err = pattern_match(test_case.str, test_case.pattern, test_case.match_type);
-    EXPECT_EQ(test_case.expected_err, actual_err);
+    EXPECT_EQ(test_case.expected_err, actual_err.code());
 }
 
 INSTANTIATE_TEST_SUITE_P(StringTest, PatternMatchTest, testing::ValuesIn(pattern_match_tests));