public inbox for [email protected]  
help / color / mirror / Atom feed
From: Tatsuo Ishii <[email protected]>
To: [email protected]
Cc: [email protected]
Subject: Re: Proposal: recent access based routing for primary-replica setups
Date: Mon, 25 Aug 2025 11:18:25 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CACeKOO000N8gUodkXt_1V=XQYx_utxXdyOxkzV7GYSPpdHnTfg@mail.gmail.com>
References: <CACeKOO3KF9ZdhkvgM+znSaj6dkTyMR77xCebQk9iuTfVavkYbA@mail.gmail.com>
	<[email protected]>
	<CACeKOO000N8gUodkXt_1V=XQYx_utxXdyOxkzV7GYSPpdHnTfg@mail.gmail.com>

Hi Nadav,

Thank you for the patch!

I have one question. How do you provide a password (sr_check_password)
while executing replication_delay_source_cmd as sr_check_user? In my
understanding replication_delay_source_cmd is executed through su
command in your patch. In this case su command tries to read the
password from terminal. I don't see such a code in the patch.

BTW, I start to think that executing replication_delay_source_cmd as
sr_check_user might not be a good idea. sr_check_user is a database
user, not OS user. In PostgreSQL they are not necessarily the
same. Also doing su in pgpool process needs to be very carefully to
avoid vulnerability. Probably we just execute it as pgpool OS user?

Lastly when I apply the patches using git apply, there are some
trailing space errors.

$ git apply ~/external-lag-feature-implementation.patch 
/home/t-ishii/external-lag-feature-implementation.patch:314: trailing whitespace.
			
/home/t-ishii/external-lag-feature-implementation.patch:317: trailing whitespace.
			
/home/t-ishii/external-lag-feature-implementation.patch:318: trailing whitespace.
			cmd_len = strlen(escaped_cmd) + 
/home/t-ishii/external-lag-feature-implementation.patch:320: trailing whitespace.
			
/home/t-ishii/external-lag-feature-implementation.patch:322: trailing whitespace.
			snprintf(full_command, cmd_len, "su - %s -c '%s'", 
warning: squelched 4 whitespace errors
warning: 9 lines add whitespace errors.

$ git apply ~/external-lag-feature-tests.patch 
/home/t-ishii/external-lag-feature-tests.patch:87: trailing whitespace.
- test_parsing.sh: Unit test for parsing logic  
/home/t-ishii/external-lag-feature-tests.patch:440: trailing whitespace.
# Test 2: Float values  
warning: 2 lines add whitespace errors.

Also I have some compilation errors after patching the source
code. See attached compilation log.

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

Making all in src
make[1]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src'
Making all in parser
make[2]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/parser'
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/parser'
Making all in libs
make[2]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs'
Making all in pcp
make[3]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs/pcp'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs/pcp'
make[3]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs'
make[3]: Nothing to be done for 'all-am'.
make[3]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs'
make[2]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs'
Making all in watchdog
make[2]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/watchdog'
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/watchdog'
Making all in .
make[2]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src'
gcc -DHAVE_CONFIG_H -DDEFAULT_CONFIGDIR=\"/usr/local/etc\" -DPGSQL_BIN_DIR=\"/usr/local/pgsql/bin\" -I. -I../src/include  -D_GNU_SOURCE -I /usr/local/pgsql/include   -g -O2 -Wall -Wmissing-prototypes -Wmissing-declarations -Wno-format-truncation -Wno-stringop-truncation -fno-strict-aliasing -c -o streaming_replication/pool_worker_child.o streaming_replication/pool_worker_child.c
streaming_replication/pool_worker_child.c: In function 'do_worker_child':
streaming_replication/pool_worker_child.c:266:5: warning: this 'else' clause does not guard... [-Wmisleading-indentation]
  266 |     else
      |     ^~~~
streaming_replication/pool_worker_child.c:270:6: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'else'
  270 |      node_status = verify_backend_node_status(slots);
      |      ^~~~~~~~~~~
In file included from streaming_replication/pool_worker_child.c:56:
streaming_replication/pool_worker_child.c: In function 'check_replication_time_lag_with_cmd':
../src/include/utils/elog.h:397:25: warning: unused variable 'save_context_stack' [-Wunused-variable]
  397 |   ErrorContextCallback *save_context_stack = error_context_stack; \
      |                         ^~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:786:2: note: in expansion of macro 'PG_TRY'
  786 |  PG_TRY();
      |  ^~~~~~
../src/include/utils/elog.h:396:15: warning: unused variable 'save_exception_stack' [-Wunused-variable]
  396 |   sigjmp_buf *save_exception_stack = PG_exception_stack; \
      |               ^~~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:786:2: note: in expansion of macro 'PG_TRY'
  786 |  PG_TRY();
      |  ^~~~~~
../src/include/utils/elog.h:405:3: error: expected 'while' before 'else'
  405 |   else \
      |   ^~~~
streaming_replication/pool_worker_child.c:961:2: note: in expansion of macro 'PG_CATCH'
  961 |  PG_CATCH();
      |  ^~~~~~~~
../src/include/utils/elog.h:412:24: error: 'save_exception_stack' undeclared (first use in this function); did you mean 'PG_exception_stack'?
  412 |   PG_exception_stack = save_exception_stack; \
      |                        ^~~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:980:2: note: in expansion of macro 'PG_END_TRY'
  980 |  PG_END_TRY();
      |  ^~~~~~~~~~
../src/include/utils/elog.h:412:24: note: each undeclared identifier is reported only once for each function it appears in
  412 |   PG_exception_stack = save_exception_stack; \
      |                        ^~~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:980:2: note: in expansion of macro 'PG_END_TRY'
  980 |  PG_END_TRY();
      |  ^~~~~~~~~~
../src/include/utils/elog.h:413:25: error: 'save_context_stack' undeclared (first use in this function); did you mean 'error_context_stack'?
  413 |   error_context_stack = save_context_stack; \
      |                         ^~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:980:2: note: in expansion of macro 'PG_END_TRY'
  980 |  PG_END_TRY();
      |  ^~~~~~~~~~
streaming_replication/pool_worker_child.c:743:9: warning: variable 'cmd_allocated' set but not used [-Wunused-but-set-variable]
  743 |  bool   cmd_allocated = false;
      |         ^~~~~~~~~~~~~
In file included from streaming_replication/pool_worker_child.c:56:
streaming_replication/pool_worker_child.c: At top level:
../src/include/utils/elog.h:414:4: error: expected identifier or '(' before 'while'
  414 |  } while (0)
      |    ^~~~~
streaming_replication/pool_worker_child.c:980:2: note: in expansion of macro 'PG_END_TRY'
  980 |  PG_END_TRY();
      |  ^~~~~~~~~~
streaming_replication/pool_worker_child.c:983:2: error: expected identifier or '(' before 'if'
  983 |  if (line)
      |  ^~
streaming_replication/pool_worker_child.c:985:2: error: expected identifier or '(' before 'if'
  985 |  if (escaped_cmd)
      |  ^~
streaming_replication/pool_worker_child.c:987:2: error: expected identifier or '(' before 'if'
  987 |  if (cmd_allocated && command)
      |  ^~
streaming_replication/pool_worker_child.c:990:2: warning: data definition has no type or storage class
  990 |  error_context_stack = callback.previous;
      |  ^~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:990:2: warning: type defaults to 'int' in declaration of 'error_context_stack' [-Wimplicit-int]
streaming_replication/pool_worker_child.c:990:2: error: conflicting types for 'error_context_stack'
In file included from streaming_replication/pool_worker_child.c:56:
../src/include/utils/elog.h:360:42: note: previous declaration of 'error_context_stack' was here
  360 | extern PGDLLIMPORT ErrorContextCallback *error_context_stack;
      |                                          ^~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:990:24: error: 'callback' undeclared here (not in a function); did you mean 'calloc'?
  990 |  error_context_stack = callback.previous;
      |                        ^~~~~~~~
      |                        calloc
streaming_replication/pool_worker_child.c:991:1: error: expected identifier or '(' before '}' token
  991 | }
      | ^
In file included from streaming_replication/pool_worker_child.c:56:
streaming_replication/pool_worker_child.c: In function 'get_query_result':
../src/include/utils/elog.h:397:46: warning: initialization of 'ErrorContextCallback *' {aka 'struct ErrorContextCallback *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
  397 |   ErrorContextCallback *save_context_stack = error_context_stack; \
      |                                              ^~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:1099:2: note: in expansion of macro 'PG_TRY'
 1099 |  PG_TRY();
      |  ^~~~~~
../src/include/utils/elog.h:408:24: warning: assignment to 'int' from 'ErrorContextCallback *' {aka 'struct ErrorContextCallback *'} makes integer from pointer without a cast [-Wint-conversion]
  408 |    error_context_stack = save_context_stack
      |                        ^
streaming_replication/pool_worker_child.c:1103:2: note: in expansion of macro 'PG_CATCH'
 1103 |  PG_CATCH();
      |  ^~~~~~~~
../src/include/utils/elog.h:413:23: warning: assignment to 'int' from 'ErrorContextCallback *' {aka 'struct ErrorContextCallback *'} makes integer from pointer without a cast [-Wint-conversion]
  413 |   error_context_stack = save_context_stack; \
      |                       ^
streaming_replication/pool_worker_child.c:1112:2: note: in expansion of macro 'PG_END_TRY'
 1112 |  PG_END_TRY();
      |  ^~~~~~~~~~
make[2]: *** [Makefile:883: streaming_replication/pool_worker_child.o] Error 1
make[2]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src'
make[1]: *** [Makefile:949: all-recursive] Error 1
make[1]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src'
make: *** [Makefile:413: all-recursive] Error 1


Attachments:

  [text/plain] compile.log (8.0K, 2-compile.log)
  download | inline:
Making all in src
make[1]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src'
Making all in parser
make[2]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/parser'
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/parser'
Making all in libs
make[2]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs'
Making all in pcp
make[3]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs/pcp'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs/pcp'
make[3]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs'
make[3]: Nothing to be done for 'all-am'.
make[3]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs'
make[2]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/libs'
Making all in watchdog
make[2]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/watchdog'
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src/watchdog'
Making all in .
make[2]: Entering directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src'
gcc -DHAVE_CONFIG_H -DDEFAULT_CONFIGDIR=\"/usr/local/etc\" -DPGSQL_BIN_DIR=\"/usr/local/pgsql/bin\" -I. -I../src/include  -D_GNU_SOURCE -I /usr/local/pgsql/include   -g -O2 -Wall -Wmissing-prototypes -Wmissing-declarations -Wno-format-truncation -Wno-stringop-truncation -fno-strict-aliasing -c -o streaming_replication/pool_worker_child.o streaming_replication/pool_worker_child.c
streaming_replication/pool_worker_child.c: In function 'do_worker_child':
streaming_replication/pool_worker_child.c:266:5: warning: this 'else' clause does not guard... [-Wmisleading-indentation]
  266 |     else
      |     ^~~~
streaming_replication/pool_worker_child.c:270:6: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'else'
  270 |      node_status = verify_backend_node_status(slots);
      |      ^~~~~~~~~~~
In file included from streaming_replication/pool_worker_child.c:56:
streaming_replication/pool_worker_child.c: In function 'check_replication_time_lag_with_cmd':
../src/include/utils/elog.h:397:25: warning: unused variable 'save_context_stack' [-Wunused-variable]
  397 |   ErrorContextCallback *save_context_stack = error_context_stack; \
      |                         ^~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:786:2: note: in expansion of macro 'PG_TRY'
  786 |  PG_TRY();
      |  ^~~~~~
../src/include/utils/elog.h:396:15: warning: unused variable 'save_exception_stack' [-Wunused-variable]
  396 |   sigjmp_buf *save_exception_stack = PG_exception_stack; \
      |               ^~~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:786:2: note: in expansion of macro 'PG_TRY'
  786 |  PG_TRY();
      |  ^~~~~~
../src/include/utils/elog.h:405:3: error: expected 'while' before 'else'
  405 |   else \
      |   ^~~~
streaming_replication/pool_worker_child.c:961:2: note: in expansion of macro 'PG_CATCH'
  961 |  PG_CATCH();
      |  ^~~~~~~~
../src/include/utils/elog.h:412:24: error: 'save_exception_stack' undeclared (first use in this function); did you mean 'PG_exception_stack'?
  412 |   PG_exception_stack = save_exception_stack; \
      |                        ^~~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:980:2: note: in expansion of macro 'PG_END_TRY'
  980 |  PG_END_TRY();
      |  ^~~~~~~~~~
../src/include/utils/elog.h:412:24: note: each undeclared identifier is reported only once for each function it appears in
  412 |   PG_exception_stack = save_exception_stack; \
      |                        ^~~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:980:2: note: in expansion of macro 'PG_END_TRY'
  980 |  PG_END_TRY();
      |  ^~~~~~~~~~
../src/include/utils/elog.h:413:25: error: 'save_context_stack' undeclared (first use in this function); did you mean 'error_context_stack'?
  413 |   error_context_stack = save_context_stack; \
      |                         ^~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:980:2: note: in expansion of macro 'PG_END_TRY'
  980 |  PG_END_TRY();
      |  ^~~~~~~~~~
streaming_replication/pool_worker_child.c:743:9: warning: variable 'cmd_allocated' set but not used [-Wunused-but-set-variable]
  743 |  bool   cmd_allocated = false;
      |         ^~~~~~~~~~~~~
In file included from streaming_replication/pool_worker_child.c:56:
streaming_replication/pool_worker_child.c: At top level:
../src/include/utils/elog.h:414:4: error: expected identifier or '(' before 'while'
  414 |  } while (0)
      |    ^~~~~
streaming_replication/pool_worker_child.c:980:2: note: in expansion of macro 'PG_END_TRY'
  980 |  PG_END_TRY();
      |  ^~~~~~~~~~
streaming_replication/pool_worker_child.c:983:2: error: expected identifier or '(' before 'if'
  983 |  if (line)
      |  ^~
streaming_replication/pool_worker_child.c:985:2: error: expected identifier or '(' before 'if'
  985 |  if (escaped_cmd)
      |  ^~
streaming_replication/pool_worker_child.c:987:2: error: expected identifier or '(' before 'if'
  987 |  if (cmd_allocated && command)
      |  ^~
streaming_replication/pool_worker_child.c:990:2: warning: data definition has no type or storage class
  990 |  error_context_stack = callback.previous;
      |  ^~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:990:2: warning: type defaults to 'int' in declaration of 'error_context_stack' [-Wimplicit-int]
streaming_replication/pool_worker_child.c:990:2: error: conflicting types for 'error_context_stack'
In file included from streaming_replication/pool_worker_child.c:56:
../src/include/utils/elog.h:360:42: note: previous declaration of 'error_context_stack' was here
  360 | extern PGDLLIMPORT ErrorContextCallback *error_context_stack;
      |                                          ^~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:990:24: error: 'callback' undeclared here (not in a function); did you mean 'calloc'?
  990 |  error_context_stack = callback.previous;
      |                        ^~~~~~~~
      |                        calloc
streaming_replication/pool_worker_child.c:991:1: error: expected identifier or '(' before '}' token
  991 | }
      | ^
In file included from streaming_replication/pool_worker_child.c:56:
streaming_replication/pool_worker_child.c: In function 'get_query_result':
../src/include/utils/elog.h:397:46: warning: initialization of 'ErrorContextCallback *' {aka 'struct ErrorContextCallback *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
  397 |   ErrorContextCallback *save_context_stack = error_context_stack; \
      |                                              ^~~~~~~~~~~~~~~~~~~
streaming_replication/pool_worker_child.c:1099:2: note: in expansion of macro 'PG_TRY'
 1099 |  PG_TRY();
      |  ^~~~~~
../src/include/utils/elog.h:408:24: warning: assignment to 'int' from 'ErrorContextCallback *' {aka 'struct ErrorContextCallback *'} makes integer from pointer without a cast [-Wint-conversion]
  408 |    error_context_stack = save_context_stack
      |                        ^
streaming_replication/pool_worker_child.c:1103:2: note: in expansion of macro 'PG_CATCH'
 1103 |  PG_CATCH();
      |  ^~~~~~~~
../src/include/utils/elog.h:413:23: warning: assignment to 'int' from 'ErrorContextCallback *' {aka 'struct ErrorContextCallback *'} makes integer from pointer without a cast [-Wint-conversion]
  413 |   error_context_stack = save_context_stack; \
      |                       ^
streaming_replication/pool_worker_child.c:1112:2: note: in expansion of macro 'PG_END_TRY'
 1112 |  PG_END_TRY();
      |  ^~~~~~~~~~
make[2]: *** [Makefile:883: streaming_replication/pool_worker_child.o] Error 1
make[2]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src'
make[1]: *** [Makefile:949: all-recursive] Error 1
make[1]: Leaving directory '/home/t-ishii/work/Pgpool-II/current/pgpool2/src'
make: *** [Makefile:413: all-recursive] Error 1

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Proposal: recent access based routing for primary-replica setups
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox