Re: Walsender doesn't process options passed in the startup packet

Hi,

I found walsender didn't process options (e.g., application_name
in primary_conninfo) passed in the startup packet. The cause is
that walsender doesn't execute process_postgres_switches() and
SetConfigOption() in InitPostgres().

The attached patch fixes this bug.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On 10/09/10 13:17, Fujii Masao wrote:
> Hi,
>
> I found walsender didn't process options (e.g., application_name
> in primary_conninfo) passed in the startup packet. The cause is
> that walsender doesn't execute process_postgres_switches() and
> SetConfigOption() in InitPostgres().
>
> The attached patch fixes this bug.

Hmm, should walsender call InitializeClientEncoding too? It affects
error messages. And what about per-user settings in pg_db_role_setting?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, Sep 10, 2010 at 7:54 PM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> I found walsender didn't process options (e.g., application_name
>> in primary_conninfo) passed in the startup packet. The cause is
>> that walsender doesn't execute process_postgres_switches() and
>> SetConfigOption() in InitPostgres().
>>
>> The attached patch fixes this bug.
>
> Hmm, should walsender call InitializeClientEncoding too? It affects error
> messages. And what about per-user settings in pg_db_role_setting?

Yes, we should do both. SetDatabasePath() and RelationCacheInitializePhase3()
need to be called before accessing to pg_db_role_setting. And we should apply
PostAuthDelay. I attached the updated patch.

BTW, this issue seems to derive from the following commit.
http://archives.postgresql.org/pgsql-committers/2010-04/msg00175.php

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Fri, Sep 10, 2010 at 9:56 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> BTW, this issue seems to derive from the following commit.
> http://archives.postgresql.org/pgsql-committers/2010-04/msg00175.php

> it was previously using an untenable assumption that template1 would
> always be available to connect to

The above commit message shows that the updated patch is untenable too
since it assumes the existence of the template1.

Is there way to access to pg_db_role_setting without connecting to the
specific database? If not, it's difficult for walsender to process the
per-user settings since it's tied to no database.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> On Fri, Sep 10, 2010 at 9:56 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> BTW, this issue seems to derive from the following commit.
>> http://archives.postgresql.org/pgsql-committers/2010-04/msg00175.php

>> it was previously using an untenable assumption that template1 would
>> always be available to connect to

> The above commit message shows that the updated patch is untenable too
> since it assumes the existence of the template1.

> Is there way to access to pg_db_role_setting without connecting to the
> specific database? If not, it's difficult for walsender to process the
> per-user settings since it's tied to no database.

Huh? walsender has no business applying any per-user or per-database
settings.

regards, tom lane

On Mon, Sep 13, 2010 at 12:32 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Huh?  walsender has no business applying any per-user or per-database
> settings.

Okay. I got rid of the access to pg_db_role_setting from the patch.
I attached the updated version, which makes walsender process the
options passed in the startup packet, apply PostAuthDelay and initialize
client encoding.

OTOH, I can believe that some people would create a dedicated role for
replication and configure the parameters for replication on the role.
What about leaving the capability to apply per-role settings as a TODO
item?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On 13/09/10 08:10, Fujii Masao wrote:
> On Mon, Sep 13, 2010 at 12:32 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Huh? walsender has no business applying any per-user or per-database
>> settings.
>
> Okay. I got rid of the access to pg_db_role_setting from the patch.
> I attached the updated version, which makes walsender process the
> options passed in the startup packet, apply PostAuthDelay and initialize
> client encoding.
>
> OTOH, I can believe that some people would create a dedicated role for
> replication and configure the parameters for replication on the role.

Right, per-database settings clearly make no sense, but per-role
settings do. There isn't very many settings that make sense for
walsender, but client_encoding is one example. I agree it's not terribly
useful, but would be nice for the sake of completeness.

> What about leaving the capability to apply per-role settings as a TODO
> item?

Yeah, seems best at this point.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On 13/09/10 08:10, Fujii Masao wrote:
> Okay. I got rid of the access to pg_db_role_setting from the patch.
> I attached the updated version, which makes walsender process the
> options passed in the startup packet, apply PostAuthDelay and initialize
> client encoding.

Thanks, committed.

I moved the check for "MyProcPort == NULL" case to the callers of
process_startup_packet(), it seems more logical to me.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com