Skip to content

Commit

Permalink
[#25477] YSQL: disable check_pghost_envvar
Browse files Browse the repository at this point in the history
Summary:
`check_pghost_envvar` is used by Postgres to validate that the pg server we are connecting to for upgrade to is a local server. It checks for conditions:
```
(strcmp(value, "localhost") != 0 && strcmp(value, "127.0.0.1") != 0 && strcmp(value, "::1") != 0 && !is_unixsock_path(value)))
```

However, in Yugabyte, the servers don't necessarily match this layout. `test_upgrade.sh` creates a server with IP 127.0.0.200, which causes `check_pghost_envvar` to fail with
```
+ build/latest/postgres/bin/pg_upgrade --old-datadir /tmp/pg15_cluster_data/node-1/disk-1/pg_data --old-host 127.0.0.200 --old-port 5433 --username yugabyte --check

libpq environment variable PGHOST has a non-local server value: 127.0.0.200
```

Rather than updating this check to work for every possible scenario in Yugabyte, it is simpler to just disable the check for Yugabyte, since the new cluster creation is owned by Yugabyte anyways.
Jira: DB-14727

Test Plan:
Jenkins
```
pg15_tests/test_upgrade.sh
```

Reviewers: hsunder, fizaa

Reviewed By: hsunder

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D41041
  • Loading branch information
timothy-e committed Jan 14, 2025
1 parent 6218feb commit 8dfc9a5
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
15 changes: 15 additions & 0 deletions src/postgres/src/bin/pg_upgrade/option.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ static void check_required_directory(char **dirpath,
const char *envVarName, bool useCwd,
const char *cmdLineOption, const char *description,
bool missingOk);

static void yb_check_sockdir(ClusterInfo *cluster);

#define FIX_DEFAULT_READ_ONLY "-c default_transaction_read_only=false"


Expand Down Expand Up @@ -274,6 +277,9 @@ parseCommandLine(int argc, char *argv[])
check_required_directory(&user_opts.socketdir, "PGSOCKETDIR", true,
"-s", _("sockets will be created"), false);

if (is_yugabyte_enabled() && !user_opts.check)
yb_check_sockdir(&new_cluster);

#ifdef WIN32

/*
Expand Down Expand Up @@ -541,3 +547,12 @@ get_sock_dir(ClusterInfo *cluster, bool live_check)
cluster->sockdir = NULL;
#endif
}

static void
yb_check_sockdir(ClusterInfo *cluster)
{
Assert(is_yugabyte_enabled());

if (!cluster->sockdir)
pg_fatal("socket directory must be specified\n");
}
10 changes: 6 additions & 4 deletions src/postgres/src/bin/pg_upgrade/pg_upgrade.c
Original file line number Diff line number Diff line change
Expand Up @@ -361,10 +361,12 @@ setup(char *argv0, bool *live_check)
* make sure the user has a clean environment, otherwise, we may confuse
* libpq when we connect to one (or both) of the servers.
*/
#ifdef YB_TODO
/* Investigate/implement this check */
check_pghost_envvar();
#endif
if (!is_yugabyte_enabled())
/*
* YB: New cluster creation is controlled by Yugabyte, but the IP
* addresses don't match the expected values of this check.
*/
check_pghost_envvar();

/*
* In case the user hasn't specified the directory for the new binaries
Expand Down

0 comments on commit 8dfc9a5

Please sign in to comment.