Обсуждение: Fixed "Slony" related issues

Hi

On Mon, Aug 5, 2013 at 1:21 PM, Akshay Joshi
<akshay.joshi@enterprisedb.com> wrote:
> Hi Dave
>
> While testing pgAdmin with "Slony" I have found couple of following issues
>
> Found crash (segmentation fault) when create a new schema using slony
> replication cluster. Crash is reproducible when user opens the "New
> Schema..." dialog and select an slony cluster from "Use Slony" combo box and
> click "OK".
> DDL Statements like ( "CREATE SCHEMA" , "ALTER TABLE" ..)  didn't work if
> user select an slony cluster from "Use Slony" combo box and click "OK".
>
> I have analyze and fixed both the issues. Below is my analysis and solution
>
> Analysis for 1: - We have used wxComboBox for "Use Slony" option in the
> dlgProperty dialog. To add an item into the combo box we have used "Append"
> method of wxControlWithItems (wxComboBox is derived from
> wxControlWithItems).
>
> According to the documentation of "wxControlWithItems" it is mention that in
> the same control all items must have client data of the same type (typed or
> untyped), if any. This type is determined by the first call to Append (the
> version with client data pointer) or SetClientData. Attached is the
> screenshot.
>
> In pgAdmin we have appended the wxEmptyString without any client data first
> in the "Use Slony" combo box and then the slony cluster with client data. So
> when the first append call gets executed, type of the client data is
> determined as "Void *"
>
> Solution for 1:- There are two ways to solve this issue.
>
> Create a dummy object of client data(replClientData in our case) and passed
> it when we append the wxEmptyString".
> Typecast the "replClientData" to (void *) while appending the slony cluster.
>
> I have fixed it using the second option.

OK.

> Analysis for 2:- After fixing the above crash when we try to create the new
> schema or add any new column to the existing table it neither applied to the
> Master nor on Slave. As per the pgAdmin code if we select a slony cluster
> then following two statements runs:
>
> SELECT _slonycluster.ddlscript_prepare(1, 0);
> SELECT _slonycluster.ddlscript_complete(1, 'ALTER TABLE "Test" ADD COLUMN
> isSlony boolean; ', 0);
>
> Where "p_only_on_node" parameter is set to 0. It is hard coded. Then I have
> googled and found some example where this parameter is set to -1.
>
> I have analyze the code of "Slony 2.1.3 (slonik.c file) " and found that it
> executes the following statement in sequence
> SELECT _slonycluster.ddlscript_prepare(1, -1);
> ALTER TABLE "Test" ADD COLUMN isSlony boolean;
> SELECT _slonycluster.ddlscript_complete(1, 'ALTER TABLE "Test" ADD COLUMN
> isSlony boolean; ', -1);
>
> Comparing it with pgAdmin code we execute only two
> statements(ddlscript_prepare and ddlscript_complete) which is being used for
> replication only, but there is no logic to execute the statement on Master
> itself. May be my understanding here is wrong, please correct me.
>
>
> Solution for 2:- To solve the above issue, First I have set the value of
> "p_only_on_node" to -1 instead of 0 and then added the SQL which is going to
> be executed on Master.

Did you check that the modified version of the SQL is appropriate for
all versions of Slony? We currently support back to 1.2 iirc.

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Thanks - patch applied.

On Tue, Aug 6, 2013 at 10:54 AM, Akshay Joshi
<akshay.joshi@enterprisedb.com> wrote:
>
>
>
> On Mon, Aug 5, 2013 at 6:52 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Mon, Aug 5, 2013 at 1:21 PM, Akshay Joshi
>> <akshay.joshi@enterprisedb.com> wrote:
>> > Hi Dave
>> >
>> > While testing pgAdmin with "Slony" I have found couple of following
>> > issues
>> >
>> > Found crash (segmentation fault) when create a new schema using slony
>> > replication cluster. Crash is reproducible when user opens the "New
>> > Schema..." dialog and select an slony cluster from "Use Slony" combo box
>> > and
>> > click "OK".
>> > DDL Statements like ( "CREATE SCHEMA" , "ALTER TABLE" ..)  didn't work
>> > if
>> > user select an slony cluster from "Use Slony" combo box and click "OK".
>> >
>> > I have analyze and fixed both the issues. Below is my analysis and
>> > solution
>> >
>> > Analysis for 1: - We have used wxComboBox for "Use Slony" option in the
>> > dlgProperty dialog. To add an item into the combo box we have used
>> > "Append"
>> > method of wxControlWithItems (wxComboBox is derived from
>> > wxControlWithItems).
>> >
>> > According to the documentation of "wxControlWithItems" it is mention
>> > that in
>> > the same control all items must have client data of the same type (typed
>> > or
>> > untyped), if any. This type is determined by the first call to Append
>> > (the
>> > version with client data pointer) or SetClientData. Attached is the
>> > screenshot.
>> >
>> > In pgAdmin we have appended the wxEmptyString without any client data
>> > first
>> > in the "Use Slony" combo box and then the slony cluster with client
>> > data. So
>> > when the first append call gets executed, type of the client data is
>> > determined as "Void *"
>> >
>> > Solution for 1:- There are two ways to solve this issue.
>> >
>> > Create a dummy object of client data(replClientData in our case) and
>> > passed
>> > it when we append the wxEmptyString".
>> > Typecast the "replClientData" to (void *) while appending the slony
>> > cluster.
>> >
>> > I have fixed it using the second option.
>>
>> OK.
>>
>> > Analysis for 2:- After fixing the above crash when we try to create the
>> > new
>> > schema or add any new column to the existing table it neither applied to
>> > the
>> > Master nor on Slave. As per the pgAdmin code if we select a slony
>> > cluster
>> > then following two statements runs:
>> >
>> > SELECT _slonycluster.ddlscript_prepare(1, 0);
>> > SELECT _slonycluster.ddlscript_complete(1, 'ALTER TABLE "Test" ADD
>> > COLUMN
>> > isSlony boolean; ', 0);
>> >
>> > Where "p_only_on_node" parameter is set to 0. It is hard coded. Then I
>> > have
>> > googled and found some example where this parameter is set to -1.
>> >
>> > I have analyze the code of "Slony 2.1.3 (slonik.c file) " and found that
>> > it
>> > executes the following statement in sequence
>> > SELECT _slonycluster.ddlscript_prepare(1, -1);
>> > ALTER TABLE "Test" ADD COLUMN isSlony boolean;
>> > SELECT _slonycluster.ddlscript_complete(1, 'ALTER TABLE "Test" ADD
>> > COLUMN
>> > isSlony boolean; ', -1);
>> >
>> > Comparing it with pgAdmin code we execute only two
>> > statements(ddlscript_prepare and ddlscript_complete) which is being used
>> > for
>> > replication only, but there is no logic to execute the statement on
>> > Master
>> > itself. May be my understanding here is wrong, please correct me.
>> >
>> >
>> > Solution for 2:- To solve the above issue, First I have set the value of
>> > "p_only_on_node" to -1 instead of 0 and then added the SQL which is
>> > going to
>> > be executed on Master.
>>
>> Did you check that the modified version of the SQL is appropriate for
>> all versions of Slony? We currently support back to 1.2 iirc.
>
>
>    I have tested my logic with Slony v2.1.3 and v2.0.7. For v1.2 I have
> taken the source code and compare the slonik.c file with the v2.1.3 and it
> seems to be working on that as well.
>>
>>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>
>
>
> --
> Akshay Joshi
> Senior Software Engineer
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
> Phone: +91 20-3058-9522
> Mobile: +91 976-788-8246



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company