Merge branch 'yugabyte:master' into master

Author: avoidik

java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressThirdPartyExtensionsHypopg.java

@@ -22,6 +22,8 @@
 import java.util.Collections;
 import java.util.Map;
 
+import static org.yb.AssertionWrappers.*;
+
 @RunWith(value=YBTestRunner.class)
 public class TestPgRegressThirdPartyExtensionsHypopg extends BasePgRegressTest {
   @Override
@@ -40,6 +42,13 @@ public void schedule_old_cost_model() throws Exception {
     Map<String, String> flagMap = super.getTServerFlags();
     flagMap.put("ysql_pg_conf_csv", "yb_enable_cbo=OFF");
     restartClusterWithFlags(Collections.emptyMap(), flagMap);
+
+    String enable_auto_analyze = miniCluster.getClient().getFlag(
+        miniCluster.getTabletServers().keySet().iterator().next(),
+        "ysql_enable_auto_analyze");
+    assertTrue("ysql_enable_auto_analyze should remain at default value false",
+        enable_auto_analyze.equals("false"));
+
     runPgRegressTest(new File(TestUtils.getBuildRootDir(),
                               "postgres_build/third-party-extensions/hypopg"),
                      "yb_old_cost_model_schedule");
@@ -50,6 +59,13 @@ public void schedule() throws Exception {
     Map<String, String> flagMap = super.getTServerFlags();
     flagMap.put("ysql_pg_conf_csv", "yb_enable_cbo=ON");
     restartClusterWithFlags(Collections.emptyMap(), flagMap);
+
+    String enable_auto_analyze = miniCluster.getClient().getFlag(
+        miniCluster.getTabletServers().keySet().iterator().next(),
+        "ysql_enable_auto_analyze");
+    assertTrue("ysql_enable_auto_analyze should be auto set to true",
+        enable_auto_analyze.equals("true"));
+
     runPgRegressTest(new File(TestUtils.getBuildRootDir(),
                               "postgres_build/third-party-extensions/hypopg"),
                      "yb_schedule");

managed/src/main/java/com/yugabyte/yw/commissioner/tasks/CreateContinuousBackup.java

@@ -10,7 +10,6 @@
 
 package com.yugabyte.yw.commissioner.tasks;
 
-import static com.yugabyte.yw.common.Util.NULL_UUID;
 import static play.mvc.Http.Status.INTERNAL_SERVER_ERROR;
 
 import com.google.inject.Inject;
@@ -23,6 +22,7 @@
 import com.yugabyte.yw.common.ShellResponse;
 import com.yugabyte.yw.common.StorageUtil;
 import com.yugabyte.yw.common.StorageUtilFactory;
+import com.yugabyte.yw.common.Util;
 import com.yugabyte.yw.common.ha.PlatformReplicationHelper;
 import com.yugabyte.yw.common.ha.PlatformReplicationManager;
 import com.yugabyte.yw.forms.AbstractTaskParams;
@@ -103,12 +103,11 @@ public void runScheduledBackup(
     ScheduleTask.create(taskUUID, schedule.getScheduleUUID());
     CustomerTask.create(
         customer,
-        NULL_UUID,
+        Util.NULL_UUID,
         taskUUID,
         CustomerTask.TargetType.Yba,
         CustomerTask.TaskType.CreateYbaBackup,
-        // TODO: Actually get platform IP
-        "platform_ip");
+        Util.getYwHostnameOrIP());
     log.info("Submitted continuous yba backup creation with task uuid = {}.", taskUUID);
   }
 

scripts/installation/bin/yb-ctl

@@ -1732,7 +1732,7 @@ class ClusterControl:
         print("-" * 100)
         print("| {:<96} |".format(title))
         print("-" * 100)
-        for k, v in body_kv_list:
+        for k, v in body_kv_list.items():
             print("| {:20}: {:<74} |".format(k, v))
         print("-" * 100)
 
@@ -1743,14 +1743,11 @@ class ClusterControl:
         title = "Node Count: {} | Replication Factor: {}".format(
             num_servers, self.get_replication_factor())
         info_kv_list = self.gen_status_tserver_connectivity(DaemonId(DAEMON_TYPE_TSERVER, 1))
-        info_kv_list.extend([
-            # TODO: this might cause issues if the first master is down...
-            ("Web UI", "http://{}/".format(
-                self.options.get_client_advertise_host_port(
-                    DaemonId(DAEMON_TYPE_MASTER, 1),
-                    "http"))),
-            ("Cluster Data", self.options.cluster_base_dir)
-        ])
+        # TODO: this might cause issues if the first master is down...
+        info_kv_list["Web UI"] = "http://{}/".format(self.options.get_client_advertise_host_port(
+            DaemonId(DAEMON_TYPE_MASTER, 1),
+            "http"))
+        info_kv_list["Cluster Data"] = self.options.cluster_base_dir
         self.print_status_box(title, info_kv_list)
         if more_info:
             status_cmd = "yb-ctl"
@@ -1761,7 +1758,7 @@ class ClusterControl:
             print("For more info, please use: {}".format(status_cmd))
 
     def gen_status_tserver_connectivity(self, tserver_daemon_id):
-        info_kv_list = []
+        info_kv_list = dict()
         if not tserver_daemon_id:
             return info_kv_list
 
@@ -1777,11 +1774,8 @@ class ClusterControl:
                 self.options.get_client_tool_path('ysqlsh'), ip_address, ysql_port,
                 YSQL_DEFAULT_PORT)
 
-            info_kv_list.extend([
-                ("JDBC", "jdbc:postgresql://{}:{}/yugabyte".format(
-                    ip_address, ysql_port)),
-                ("YSQL Shell", ysqlsh_cmd_line)
-            ])
+            info_kv_list["JDBC"] = "jdbc:postgresql://{}:{}/yugabyte".format(ip_address, ysql_port)
+            info_kv_list["YSQL Shell"] = ysqlsh_cmd_line
 
         # -----------------------------------------------------------------------------------------
         # cqlsh command line
@@ -1794,8 +1788,7 @@ class ClusterControl:
             if ycql_port != YCQL_DEFAULT_PORT:
                 cqlsh_cmd_line += ' %s' % ycql_port
 
-        info_kv_list.append(
-            ("YCQL Shell", cqlsh_cmd_line))
+        info_kv_list["YCQL Shell"] = cqlsh_cmd_line
 
         # -----------------------------------------------------------------------------------------
         # redis-cli command line
@@ -1806,7 +1799,7 @@ class ClusterControl:
         redis_cli_cmd_line = format_cmd_line_with_host_port(
             self.options.get_client_tool_path('redis-cli'), ip_address, yedis_port,
             YEDIS_DEFAULT_PORT)
-        info_kv_list.append(("YEDIS Shell", redis_cli_cmd_line))
+        info_kv_list["YEDIS Shell"] = redis_cli_cmd_line
         return info_kv_list
 
     def show_node_status(self, daemon_index, server_counts=None):
@@ -1818,19 +1811,18 @@ class ClusterControl:
         master_daemon_id = DaemonId(DAEMON_TYPE_MASTER, daemon_index) if is_master else None
 
         info_kv_list = self.gen_status_tserver_connectivity(tserver_daemon_id)
-        log_kv_list = []
+        log_kv_list = dict()
         for idx, node_dir in enumerate(self.get_base_node_dirs(daemon_index)):
             data_path = os.path.join(node_dir, "yb-data")
             if idx == 0:
                 if is_tserver:
-                    log_kv_list.append(
-                        ("yb-tserver Logs", os.path.join(data_path, DAEMON_TYPE_TSERVER, "logs")))
+                    log_kv_list["yb-tserver Logs"] = os.path.join(data_path, DAEMON_TYPE_TSERVER,
+                                                                  "logs")
                 if is_master:
-                    log_kv_list.append(
-                        ("yb-master Logs", os.path.join(data_path, DAEMON_TYPE_MASTER, "logs")))
-            info_kv_list.append(
-                ("data-dir[{}]".format(idx), data_path))
-        info_kv_list.extend(log_kv_list)
+                    log_kv_list["yb-master Logs"] = os.path.join(data_path, DAEMON_TYPE_MASTER,
+                                                                 "logs")
+            info_kv_list["data-dir[{}]".format(idx)] = data_path
+        info_kv_list |= log_kv_list
 
         master_pid = self.get_pid(master_daemon_id) if master_daemon_id else None
         process_list = []

src/yb/tserver/server_main_util.cc

@@ -13,7 +13,10 @@
 
 #include "yb/tserver/server_main_util.h"
 
+#include <algorithm>
 #include <iostream>
+#include <boost/algorithm/string/trim.hpp>
+#include "yb/util/string_case.h"
 
 #if YB_ABSL_ENABLED
 #include "absl/debugging/symbolize.h"
@@ -31,6 +34,7 @@
 #include "yb/util/debug/trace_event.h"
 #include "yb/util/flags.h"
 #include "yb/util/mem_tracker.h"
+#include "yb/util/pg_util.h"
 #include "yb/util/size_literals.h"
 #include "yb/util/status.h"
 
@@ -49,6 +53,8 @@ DECLARE_int64(memory_limit_hard_bytes);
 DECLARE_bool(ysql_enable_auto_analyze);
 DECLARE_bool(ysql_enable_auto_analyze_service);
 DECLARE_bool(ysql_enable_table_mutation_counter);
+DECLARE_string(ysql_pg_conf_csv);
+DECLARE_string(ysql_yb_enable_cbo);
 
 namespace yb {
 
@@ -125,40 +131,106 @@ void AdjustMemoryLimitsIfNeeded(bool is_master) {
   AdjustMemoryLimits(values);
 }
 
-// If auto analyze service is used based on old deprecated flags,
-// adjust new flags' values based on the deprecated flags to make auto analyze
-// service behaves consistently.
-// When using deprecated flags, only setting both FLAGS_ysql_enable_auto_analyze
-// and FLAGS_ysql_enable_table_mutation_counter makes auto analyze service runable,
-// so ysql_enable_auto_analyze is set in the following precedence order
-//    1. Any explicit user set value for ysql_enable_auto_analyze.
-//    2. If not set explicitly,
-//          2.1 If ysql_enable_auto_analyze_service and ysql_enable_table_mutation_counter are set
-//              explicitly. Use their values to set value for ysql_enable_auto_analyze.
-//              ysql_enable_auto_analyze is true only if both ysql_enable_auto_analyze_service and
-//              ysql_enable_table_mutation_counter are true.
-//          2.2 If not both of ysql_enable_auto_analyze_service and
-//              ysql_enable_table_mutation_counter are set explicity,
-//              use ysql_enable_auto_analyze default value.
+// Return the lower case value of a GUC set in a CSV string (FLAGS_ysql_pg_conf_csv),
+// accounting for the following
+// 1. GUC keys and values are case insensitive
+// 2. A GUC can be repeated multiple times, the final value is what matters
+//    (consistent with WritePostgresConfig)
+std::optional<std::string> FindGUCValue(
+    const std::string& guc_key, const std::vector<std::string>& user_configs) {
+  std::optional<std::string> result;
+  for (const auto& config : user_configs) {
+    const auto lower_case_config = yb::ToLowerCase(config);
+    if (lower_case_config.find(guc_key) == std::string::npos) {
+      continue;
+    }
+    const auto pos = lower_case_config.find('=');
+    if (pos != std::string::npos) {
+      std::string config_key = boost::trim_copy(lower_case_config.substr(0, pos));
+      std::string config_value = boost::trim_copy(lower_case_config.substr(pos + 1));
+      if (config_key == guc_key) {
+        result = config_value;
+      }
+    }
+  }
+  return result;
+}
+
+// Auto analyze flag can have different defaults based on other flags like
+// older (deprecated) auto analyze flags or the cbo flags. In any case, an explict
+// user set flag value should be respected as-is.
 void AdjustAutoAnalyzeFlagIfNeeded() {
-  // Check if the flag is explictly set.
-  if (!google::GetCommandLineFlagInfoOrDie("ysql_enable_auto_analyze").is_default)
+  // If the auto analyze flag is explictly set, do not change it.
+  if (!google::GetCommandLineFlagInfoOrDie("ysql_enable_auto_analyze").is_default) {
+    LOG(INFO) << "Flag: ysql_enable_auto_analyze has been set to " << FLAGS_ysql_enable_auto_analyze
+              << " by explicit config in command line";
     return;
-  bool old_value_of_new_flag = FLAGS_ysql_enable_auto_analyze;
-  // check if the old flags are enabled.
-  bool auto_analyze_enabled_using_old_flags
-    = FLAGS_ysql_enable_auto_analyze_service && FLAGS_ysql_enable_table_mutation_counter;
+  }
+
+  // When we change ysql_yb_enable_cbo to default on, we need to update the logic here, so adding
+  // a DCHECK to catch this situation.
+  DCHECK(
+      !(google::GetCommandLineFlagInfoOrDie("ysql_yb_enable_cbo").is_default &&
+        FLAGS_ysql_yb_enable_cbo == "on"));
+
+  // If ysql_yb_enable_cbo was explicitly set by user, then ysql_enable_auto_analyze flag is set to
+  // true whenever it is on. The reasoning is that CBO in legacy mode
+  // can have more unpredictable behavior when ANALYZE is run, so we don't want to run ANALYZE
+  // automatically.
+  bool cbo_flag_on = false;
+  std::string cbo_reason = "";
+  const std::vector<std::string> cbo_on_values = {"on", "true", "1", "yes"};
+  if (!google::GetCommandLineFlagInfoOrDie("ysql_yb_enable_cbo").is_default) {
+    if (std::find(
+            cbo_on_values.begin(), cbo_on_values.end(),
+            yb::ToLowerCase(FLAGS_ysql_yb_enable_cbo)) != cbo_on_values.end()) {
+      cbo_flag_on = true;
+      cbo_reason = "ysql_yb_enable_cbo flag was explicitly set by user to '" +
+                   FLAGS_ysql_yb_enable_cbo + "'";
+    }
+  } else {
+    // If ysql_pg_conf_csv contains yb_enable_cbo=on, then ysql_enable_auto_analyze is set to true.
+    // Note that ysql_pg_conf_csv has lower precedence than user-set ysql_yb_enable_cbo.
+    std::vector<std::string> user_configs;
+    std::optional<std::string> cbo_guc_value;
+    if (!FLAGS_ysql_pg_conf_csv.empty() &&
+        ReadCSVValues(FLAGS_ysql_pg_conf_csv, &user_configs).ok() &&
+        (cbo_guc_value = FindGUCValue("yb_enable_cbo", user_configs)).has_value() &&
+        std::find(cbo_on_values.begin(), cbo_on_values.end(), cbo_guc_value.value()) !=
+            cbo_on_values.end()) {
+      cbo_flag_on = true;
+      cbo_reason = "yb_enable_cbo was set to " + cbo_guc_value.value() + " in ysql_pg_conf_csv";
+    }
+  }
+
+  if (cbo_flag_on) {
+    google::SetCommandLineOptionWithMode(
+      "ysql_enable_auto_analyze", "true", google::SET_FLAG_IF_DEFAULT);
+    LOG(INFO) << "Flag: ysql_yb_enable_auto_analyze was auto-set to "
+              << FLAGS_ysql_enable_auto_analyze << " because " << cbo_reason;
+    return;
+  }
+
+  // If ysql_enable_auto_analyze_service and ysql_enable_table_mutation_counter are set
+  // explicitly, use their values to set value for ysql_enable_auto_analyze.
+  // ysql_enable_auto_analyze is true only if both ysql_enable_auto_analyze_service and
+  // ysql_enable_table_mutation_counter are true.
+  const bool old_value_of_new_flag = FLAGS_ysql_enable_auto_analyze;
+  const bool auto_analyze_enabled_using_old_flags =
+      FLAGS_ysql_enable_auto_analyze_service && FLAGS_ysql_enable_table_mutation_counter;
   if (auto_analyze_enabled_using_old_flags) {
-    // set new flag default.
     google::SetCommandLineOptionWithMode("ysql_enable_auto_analyze", "true",
                                          google::SET_FLAG_IF_DEFAULT);
 
-    LOG(INFO) << "Flag: ysql_enable_auto_analyze was auto-set to "
-              << FLAGS_ysql_enable_auto_analyze
+    LOG(INFO) << "Flag: ysql_enable_auto_analyze was auto-set to " << FLAGS_ysql_enable_auto_analyze
               << " from its default value of " << old_value_of_new_flag
-              << ". Flags: ysql_enable_auto_analyze_service and "
-              << "ysql_enable_table_mutation_counter are deprecated.";
+              << "because it was enabled by the following deprecated flags"
+              << " - ysql_enable_auto_analyze_service and ysql_enable_table_mutation_counter.";
+    return;
   }
+
+  LOG(INFO) << "Not changing ysql_enable_auto_analyze from its default value "
+            << FLAGS_ysql_enable_auto_analyze;
 }
 
 }  // anonymous namespace

src/yb/tserver/tserver_flags.cc

@@ -27,6 +27,7 @@ DEFINE_UNKNOWN_uint64(tserver_master_replication_factor, 0,
     "name and port through tserver_master_addrs for masters auto-discovery when running on "
     "Kubernetes.");
 
-DEFINE_RUNTIME_bool(ysql_enable_auto_analyze, true,
+DEFINE_RUNTIME_bool(
+    ysql_enable_auto_analyze, false,
     "Enable Auto Analyze to automatically trigger ANALYZE for updating table statistics of tables "
     "which have changed more than a configurable threshold.");

src/yb/util/pg_util.cc

@@ -14,7 +14,10 @@
 #include "yb/util/pg_util.h"
 
 #include <algorithm>
+
+#include <regex>
 #include <string>
+#include <boost/algorithm/string/replace.hpp>
 
 #include "yb/util/logging.h"
 
@@ -111,4 +114,38 @@ std::string PgDeriveSocketLockFile(const HostPort& host_port) {
       PgDeriveSocketDir(host_port), Format(".s.PGSQL.$0.lock", host_port.port()));
 }
 
+Status ReadCSVValues(const std::string& csv, std::vector<std::string>* lines) {
+  // Function reads CSV string in the following format:
+  // - fields are divided with comma (,)
+  // - fields with comma (,) or double-quote (") are quoted with double-quote (")
+  // - pair of double-quote ("") in quoted field represents single double-quote (")
+  //
+  // Examples:
+  // 1,"two, 2","three ""3""", four , -> ['1', 'two, 2', 'three "3"', ' four ', '']
+  // 1,"two                           -> Malformed CSV (quoted field 'two' is not closed)
+  // 1, "two"                         -> Malformed CSV (quoted field 'two' has leading spaces)
+  // 1,two "2"                        -> Malformed CSV (field with " must be quoted)
+  // 1,"tw"o"                         -> Malformed CSV (no separator after quoted field 'tw')
+
+  const std::regex exp(R"(^(?:([^,"]+)|(?:"((?:[^"]|(?:""))*)\"))(?:(?:,)|(?:$)))");
+  auto i = csv.begin();
+  const auto end = csv.end();
+  std::smatch match;
+  while (i != end && std::regex_search(i, end, match, exp)) {
+    // Replace pair of double-quote ("") with single double-quote (") in quoted field.
+    if (match[2].length() > 0) {
+      lines->emplace_back(match[2].first, match[2].second);
+      boost::algorithm::replace_all(lines->back(), "\"\"", "\"");
+    } else {
+      lines->emplace_back(match[1].first, match[1].second);
+    }
+    i += match.length();
+  }
+  SCHECK(i == end, InvalidArgument, Format("Malformed CSV '$0'", csv));
+  if (!csv.empty() && csv.back() == ',') {
+    lines->emplace_back();
+  }
+  return Status::OK();
+}
+
 } // namespace yb