[#31805] DocDB: Fix 60s libpq connect block in CheckYsqlLaggingCatalogVersions

src/yb/tserver/tablet_server.cc

@@ -118,6 +118,7 @@
 #include "yb/util/status_format.h"
 #include "yb/util/status_log.h"
 #include "yb/util/string_util.h"
+#include "yb/util/tsan_util.h"
 
 #include "yb/yql/pggate/util/ybc_util.h"
 #include "yb/yql/pgwrapper/libpq_utils.h"
@@ -938,6 +939,12 @@ void TabletServer::Shutdown() {
   }
   LOG(INFO) << "TabletServer shutting down...";
 
+  // Stop the periodic lagging-catalog-versions check from re-scheduling itself. An
+  // already-running check aborts quickly, because shutting_down_ is set: the callback checks it
+  // before connecting, and the libpq connect retry loop observes it through the should_stop
+  // predicate of CreateInternalPGConn.
+  check_lagging_catalog_versions_task_.StartShutdown();
+
   // Best effort to give up our ysql lease.
   // todo(zdrudi): there's lifetime issues trying to access the pg_supervisor here through
   // callbacks. Probably due to the way the MiniCluster sets up the PgSupervisor.
@@ -998,6 +1005,11 @@ void TabletServer::Shutdown() {
   // otherwise their orphaned RPCs keep their connections open and wedge reactor shutdown.
   AbortInFlightRelcacheInitConnections();
 
+  // Wait out any in-flight lagging-catalog-versions check and abort the pending one. This must
+  // happen while the messenger io_threads that run the check are still alive; the base class
+  // shutdowns below join them.
+  check_lagging_catalog_versions_task_.CompleteShutdown();
+
   DbServerBase::Shutdown();
   RpcAndWebServerBase::Shutdown();
   tablet_manager_->CompleteShutdown();
@@ -2139,7 +2151,13 @@ void TabletServer::DoGarbageCollectionOfInvalidationMessages(
 }
 
 Status TabletServer::CheckYsqlLaggingCatalogVersions() {
-  auto deadline = CoarseMonoClock::Now() + default_client_timeout();
+  // This is a periodic best-effort check against the local postmaster: use a short connect
+  // budget instead of the general client timeout. When the postmaster is unreachable (e.g. it
+  // is already stopped while the process shuts down), the full 60s budget would pin a
+  // messenger io_thread in the connect retry loop for the whole minute per invocation
+  // (#31805). A missed cycle merely delays invalidation message garbage collection until the
+  // next interval.
+  auto deadline = CoarseMonoClock::Now() + 2s * kTimeMultiplier;
   auto pg_conn = VERIFY_RESULT(
       CreateInternalPGConn("template1", kDefaultInternalPgUser, false, deadline));
   const std::string query = "SELECT datid, local_catalog_version FROM "
@@ -2571,12 +2589,17 @@ TserverXClusterContextIf& TabletServer::GetXClusterContext() const {
 
 void TabletServer::ScheduleCheckLaggingCatalogVersions() {
   LOG(INFO) << __func__;
-  messenger()->scheduler().Schedule(
+  check_lagging_catalog_versions_task_.Bind(&messenger()->scheduler());
+  check_lagging_catalog_versions_task_.Schedule(
     [this](const Status& status) {
       if (!status.ok()) {
         LOG(INFO) << status;
         return;
       }
+      if (shutting_down_) {
+        LOG(INFO) << "Skipping the lagging catalog versions check: shutting down";
+        return;
+      }
       auto s = CheckYsqlLaggingCatalogVersions();
       if (!s.ok()) {
         LOG(WARNING) << "Could not get the set of lagging catalog versions: " << s;

src/yb/tserver/tablet_server.h

@@ -60,6 +60,7 @@
 #include "yb/gutil/macros.h"
 
 #include "yb/rpc/rpc_fwd.h"
+#include "yb/rpc/scheduler.h"
 
 #include "yb/master/master_fwd.h"
 #include "yb/master/master_heartbeat.pb.h"
@@ -724,6 +725,11 @@ class TabletServer : public DbServerBase, public TabletServerIf {
   std::optional<ConnectivityPoller> connectivity_poller_;
 
   std::unique_ptr<docdb::ObjectLockSharedStateManager> object_lock_shared_state_manager_;
+
+  // Tracks the periodic CheckYsqlLaggingCatalogVersions task so that Shutdown() can stop the
+  // re-schedule loop and wait out an in-flight check before the messenger is shut down.
+  rpc::ScheduledTaskTracker check_lagging_catalog_versions_task_;
+
   OneTimeBool shutting_down_;
 
   // Per-database list of pending relcache-init callbacks. The first caller for a database creates

src/yb/yql/pgwrapper/pg_catalog_version-test.cc

@@ -15,7 +15,9 @@
 #include "yb/tserver/tserver_service.proxy.h"
 #include "yb/tserver/tserver_shared_mem.h"
 #include "yb/util/env_util.h"
+#include "yb/util/monotime.h"
 #include "yb/util/path_util.h"
+#include "yb/util/pg_util.h"
 #include "yb/util/scope_exit.h"
 #include "yb/util/status_log.h"
 #include "yb/util/string_util.h"
@@ -2939,6 +2941,47 @@ TEST_F(PgCatalogVersionTest, InvalMessageMinimalRetention) {
       std::make_pair(false, true) /* at_least */);
 }
 
+// Regression test for https://github.com/yugabyte/yugabyte-db/issues/31805.
+// The periodic lagging-catalog-versions check opens a libpq connection to the
+// local postmaster. When the postmaster is unreachable (stopped, or its unix
+// socket directory removed), the connect used to retry until the general 60s
+// client deadline, pinning a messenger io_thread for the full 60s per
+// invocation, and the task re-scheduled itself unconditionally even while the
+// tablet server was shutting down. Verify that the check gives up quickly and
+// that tablet server shutdown is not delayed by the check.
+TEST_F(PgCatalogVersionTest, CheckLaggingCatalogVersionsPostmasterDown) {
+  RestartClusterWithInvalMessageEnabled(
+      { "--check_lagging_catalog_versions_interval_secs=1" });
+  auto* ts = cluster_->tablet_server(0);
+  // Ensure the postmaster on ts is up and accepting connections.
+  pg_ts = ts;
+  ASSERT_RESULT(Connect());
+
+  // Arm the log watcher before making the postmaster unreachable: LogWaiter is
+  // edge-triggered and only sees log lines emitted after it is armed.
+  LogWaiter log_waiter(ts, "Could not get the set of lagging catalog versions");
+
+  // Remove the postmaster's unix socket directory. Connection attempts fail
+  // immediately with "No such file or directory", exactly like they do after
+  // the postmaster has shut down.
+  const auto socket_dir =
+      PgDeriveSocketDir(HostPort(ts->bind_host(), ts->pgsql_rpc_port()));
+  LOG(INFO) << "Removing postmaster socket directory " << socket_dir;
+  ASSERT_OK(Env::Default()->DeleteRecursively(socket_dir));
+
+  // The check runs every second and must fail fast. Before the fix the first
+  // failure was logged only after the full 60s connect deadline expired.
+  ASSERT_OK(log_waiter.WaitFor(MonoDelta::FromSeconds(20 * kTimeMultiplier)));
+
+  // Graceful shutdown must not be blocked by an in-flight or re-scheduled
+  // check either.
+  const auto shutdown_start = MonoTime::Now();
+  ts->Shutdown(SafeShutdown::kTrue);
+  const auto shutdown_duration = MonoTime::Now() - shutdown_start;
+  LOG(INFO) << "Tablet server shutdown took " << shutdown_duration;
+  ASSERT_LT(shutdown_duration, MonoDelta::FromSeconds(30 * kTimeMultiplier));
+}
+
 // https://github.com/yugabyte/yugabyte-db/issues/27822
 TEST_F(PgCatalogVersionTest, InvalMessageWaitOnVersionGap) {
   // Disable auto analyze in this test because it introduce flakiness of metrics.