Skip to content

Commit

Permalink
[Backport 2.6][#8580, #9489] ysql: Inherit default PGSQL proxy bind a…
Browse files Browse the repository at this point in the history
…ddress from rpc bind address

Summary:
#8580: Index backfill would fail if the `pgsql_proxy_bind_address` flag was unset due to an inconsistency between the default Postgres listen address and the default flag value read by the tablet server.
#9489: Tablet server UI buttons for YSQL failed if the `pgsql_proxy_bind_address` flag was unset due to problematic treatment of empty string values and links would be set without host.

Both issues are solved once the `pgsql_proxy_bind_address` receives a reasonable default value. Previously, when the gflag was unset, the default empty value would result in an empty host: "". In the former issue, the `PgProcessConf` instead had a separate default host of `0.0.0.0` in the case the flag was empty, while the tablet server treated the empty host as-is, leading to a discrepancy in index backfill. Similarly, when `pgsql_proxy_bind_address` defaulted to the empty host, the host for `GetDynamicUrlTile` was empty (with the appropriate port, in this case, 13000), but an empty host is notably not a wildcard address, causing URLs to be generated to `http://:13000`.

This diff inherits a default value from the value of `rpc_bind_addresses` for `pgsql_proxy_bind_address`, as was done previously for `cql` and `redis` bind addresses. This new default flag value for `pgsql_proxy_bind_address` is then utilized both by the tablet server and `PgProcessConf` as the flag is defined both before the tablet server is initialized on `Init()` and before the Postgres process is initialized with `CreateValidateAndRunInitDb(...)`, solving the first issue. Additionally, the default host for `rpc_bind_addresses` is `0.0.0.0`, matching the Postgres value even if the `rpc_bind_addresses` gflag is unset. The new default flag value for `pgsql_proxy_bind_address` is also used in the `GetDynamicUrlTile` function, now with a valid host.

Original Commit: 3f1e218
Original Diff: https://phabricator.dev.yugabyte.com/D13136

Test Plan:
Jenkins: rebase: 2.6

For both of the below testing methods, remove the lines which set `--pgsql_proxy_bind_address` in `get_tserver_only_flags(...)` from the `yb-ctl` script to illustrate functionality in the lack of an explicit setting.

**#8580**
1. Run `./bin/yb-ctl start --tserver_flags "vmodule=tablet=1` to start up local cluster with tserver logging extended
2. Run `./bin/ysqlsh --command 'create table t (i int)'; ./bin/ysqlsh --command 'create index on t (i)'` and observe result:
```
CREATE TABLE
CREATE INDEX
```
after change and error:
```
ERROR:  Aborted: backfill connection to DB failed: could not connect to server: No such file or directory
        Is the server running locally and accepting connections on Unix domain socket "/tmp/.yb.0/.s.PGSQL.5433"?
```
before diff

**#9489**
1. Run `./bin/yb-ctl` start to start up local cluster
2. Open `http://127.0.0.1:9000/`, navigate to `Utilities`, and observe that `YSQL Live Ops` and `YSQL All Ops` route to valid hosts (`http://127.0.0.1:13000/(rpcz,statements)`) after change, vs invalid host (`http://:13000/(rpcz,statements)`) before change.

**Exhibiting expected functionality even when `rpc_bind_addresses` is left unset (defaults to `0.0.0.0`)**
1. Start up master with the `rpc_bind_addresses` flag unset (which is set by default in `yb-ctl`):
```
./build/latest/bin/yb-master --fs_data_dirs $PATH_TO_YUGABYTE_DATA/node-1/disk-1 --webserver_interface 127.0.0.1 --v 0 --version_file_json_path=$PATH_TO_YUGABYTE/build/debug-clang-dynamic-ninja --replication_factor=1 --yb_num_shards_per_tserver 2 --ysql_num_shards_per_tserver=2 --default_memory_limit_to_ram_ratio=0.35 --master_addresses 127.0.0.1:7100 --enable_ysql=true
```
2. Start up the tserver without the `rpc_bind_addresses` flag set:
```
./build/latest/bin/yb-tserver --fs_data_dirs $PATH_TO_YUGABYTE_DATA/node-1/disk-1 --webserver_interface 127.0.0.1 --v 0 --version_file_json_path= $PATH_TO_YUGABYTE/build/debug-clang-dynamic-ninja --tserver_master_addrs=127.0.0.1:7100 --yb_num_shards_per_tserver=2 --redis_proxy_bind_address=127.0.0.1:6379 --cql_proxy_bind_address=127.0.0.1:9042 --local_ip_for_outbound_sockets=127.0.0.1 --use_cassandra_authentication=false --ysql_num_shards_per_tserver=2 --default_memory_limit_to_ram_ratio=0.65 --enable_ysql=true --vmodule=tablet=1
```
3. Run `./bin/ysqlsh --command 'create table t (i int)'; ./bin/ysqlsh --command 'create index on t (i)'` and observe result:
```
CREATE TABLE
CREATE INDEX
```
4. Open `http://127.0.0.1:9000/`, navigate to `Utilities`, and observe that `YSQL Live Ops` and `YSQL All Ops` route to valid hosts (`http://127.0.0.1:13000/(rpcz,statements)`).

Reviewers: sanketh, jason

Reviewed By: jason

Subscribers: yql, sanketh, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D13184
  • Loading branch information
Deepayan Patra committed Sep 27, 2021
1 parent 36f284c commit e2cf724
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
11 changes: 10 additions & 1 deletion src/yb/integration-tests/tablet_server-itest.cc
Expand Up @@ -34,7 +34,9 @@ TEST_F(TabletServerITest, TestProxyAddrs) {
for (int i = 0; i < FLAGS_num_tablet_servers; i++) {
auto tserver = cluster_->tablet_server(i);
auto rpc_host = GetHost(ASSERT_RESULT(tserver->GetFlag("rpc_bind_addresses")));
for (auto flag : {"cql_proxy_bind_address", "redis_proxy_bind_address"}) {
for (auto flag : {"cql_proxy_bind_address",
"redis_proxy_bind_address",
"pgsql_proxy_bind_address"}) {
auto host = GetHost(ASSERT_RESULT(tserver->GetFlag(flag)));
ASSERT_EQ(host, rpc_host);
}
Expand All @@ -47,6 +49,9 @@ TEST_F(TabletServerITest, TestProxyAddrsNonDefault) {
std::unique_ptr<FileLock> redis_port_file_lock;
auto redis_port = GetFreePort(&redis_port_file_lock);
ts_flags.push_back("--redis_proxy_bind_address=127.0.0.2${index}:" + std::to_string(redis_port));
std::unique_ptr<FileLock> pgsql_port_file_lock;
auto pgsql_port = GetFreePort(&pgsql_port_file_lock);
ts_flags.push_back("--pgsql_proxy_bind_address=127.0.0.3${index}:" + std::to_string(pgsql_port));

CreateCluster("ts-itest-cluster-nd", ts_flags, master_flags);

Expand All @@ -61,6 +66,10 @@ TEST_F(TabletServerITest, TestProxyAddrsNonDefault) {
res = GetHost(ASSERT_RESULT(tserver->GetFlag("redis_proxy_bind_address")));
ASSERT_EQ(res, Format("127.0.0.2$0", i));
ASSERT_NE(res, rpc_host);

res = GetHost(ASSERT_RESULT(tserver->GetFlag("pgsql_proxy_bind_address")));
ASSERT_EQ(res, Format("127.0.0.3$0", i));
ASSERT_NE(res, rpc_host);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/yb/tserver/tablet_server_main.cc
Expand Up @@ -137,6 +137,7 @@ void SetProxyAddresses() {
LOG(INFO) << "Using parsed rpc = " << FLAGS_rpc_bind_addresses;
SetProxyAddress(&FLAGS_redis_proxy_bind_address, "YEDIS", RedisServer::kDefaultPort);
SetProxyAddress(&FLAGS_cql_proxy_bind_address, "YCQL", CQLServer::kDefaultPort);
SetProxyAddress(&FLAGS_pgsql_proxy_bind_address, "YSQL", PgProcessConf::kDefaultPort);
}

int TabletServerMain(int argc, char** argv) {
Expand Down

0 comments on commit e2cf724

Please sign in to comment.