public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tatsuo Ishii <[email protected]>
To: [email protected]
Cc: [email protected]
Subject: Re: Proposal: recent access based routing for primary-replica setups
Date: Sat, 01 Nov 2025 15:36:14 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CACeKOO2W09UBbE5erXQeCDRBSbW8gK=9AsMVddRbX0dWaXKtig@mail.gmail.com>
<[email protected]>
>> Hi,
>>
>> I'm back at work - wdyt of this version?
>
> Thanks for the patch! I will look into it weekend.
Here are review comments.
1. git apply complains trailing whitespace and new blank line.
$ git apply ~/0001-external-replication-delay-implementation.patch
/home/t-ishii/0001-external-replication-delay-implementation.patch:225: trailing whitespace.
/home/t-ishii/0001-external-replication-delay-implementation.patch:232: trailing whitespace.
/home/t-ishii/0001-external-replication-delay-implementation.patch:246: trailing whitespace.
warning: 3 lines add whitespace errors.
$ git apply ~/0002-external-replication-delay-tests-and-docs.patch
/home/t-ishii/0002-external-replication-delay-tests-and-docs.patch:639: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
2. You can use psprintf() instead of palloc() + snprintf() to make the code simpler.
+ if (!bi || bi->backend_hostname[0] == '\0' || bi->backend_port <= 0)
+ {
+ /* Fallback if hostname or port is not set */
+ out = palloc(32);
+ snprintf(out, 32, "unknown_node_%d", node_id);
+ return out;
+ }
3. Ditto as above.
+ /* Use hostname:port format */
+ hlen = strlen(bi->backend_hostname);
+ /* max port chars ~5, plus colon and NUL */
+ out_len = hlen + 1 + 5 + 1;
+ out = palloc(out_len);
+ snprintf(out, out_len, "%s:%d", bi->backend_hostname, bi->backend_port);
+ return out;
4. There are a few compiler warnings.
streaming_replication/pool_worker_child.c: In function ‘do_worker_child’:
streaming_replication/pool_worker_child.c:269:33: warning: this ‘else’ clause does not guard... [-Wmisleading-indentation]
269 | else
| ^~~~
streaming_replication/pool_worker_child.c:273:41: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘else’
273 | node_status = verify_backend_node_status(slots);
| ^~~~~~~~~~~
5. 041.external_replication_delay failed.
timeout: failed to run command './test.sh': Permission denied
After running "chmod 755" to fix the issue, still the test fails. From
src/test/regression/log/041.external_replication_delay:
./test.sh: line 45: syntax error near unexpected token `('
./test.sh: line 45: `echo === Test0: External command receives replica identifiers only (primary omitted) ==='
Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
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