Thread: [Postgres-xc-developers] pgxc_monitor patch

Hi,

Enclosed is a path of pgxc_monitor to check if gtm, gtm_proxy,
coordinator or datanode is running.    I first thought to add
dedicated protocol
to gtm and gtm_proxy for this, but found PQconnectGTM() does many
shakehands and it's sufficient to check if gtm/gtm_proxy is running.

So there's no modification to the core.

Sorry the patch to filelist.sgmlin includes fix to restrict
pgxc_clean, pgxc_ddl and pgxc_monitor only to XC.

Regards;
----------
Koichi Suzuki

Hi Suzuki-San,


> Enclosed is a path of pgxc_monitor to check if gtm, gtm_proxy,
> coordinator or datanode is running.

AFAICS, gtm_proxy  and datanode is not handled still?

> I first thought to add
> dedicated protocol
> to gtm and gtm_proxy for this, but found PQconnectGTM() does many
> shakehands and it's sufficient to check if gtm/gtm_proxy is running.
>
> So there's no modification to the core.
>

That's nice. Can't we do this same for the GTM proxy too? Just do a
PQconnectGTM()?

Datanodes will need some more thinking. We can always send in a
"select 1" to them too just to see if they are up and about. But that
will unnecessarily consume local xids.

OTOH, we can contact it via a coordinator. So in that case we can
extend the code you added for the coordinator to additionally take in
a node name. And if it's provided it contacts the datanode otherwise
it runs a "select 1" on itself or something like that.

Regards,
Nikhils

> Sorry the patch to filelist.sgmlin includes fix to restrict
> pgxc_clean, pgxc_ddl and pgxc_monitor only to XC.
>
> Regards;
> ----------
> Koichi Suzuki
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> Postgres-xc-developers mailing list
> Pos...@li...
> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers
>



-- 
StormDB - http://www.stormdb.com
The Database Cloud

2012/7/12 Nikhil Sontakke <ni...@st...>:
> Hi Suzuki-San,
>
>
>> Enclosed is a path of pgxc_monitor to check if gtm, gtm_proxy,
>> coordinator or datanode is running.
>
> AFAICS, gtm_proxy  and datanode is not handled still?

Yes, they're handled.   To be honest, pgxc_monitor does not
distinguish gtm and gtm_proxy, coordinator and datanode because we're
using the same protocol.

>
>> I first thought to add
>> dedicated protocol
>> to gtm and gtm_proxy for this, but found PQconnectGTM() does many
>> shakehands and it's sufficient to check if gtm/gtm_proxy is running.
>>
>> So there's no modification to the core.
>>
>
> That's nice. Can't we do this same for the GTM proxy too? Just do a
> PQconnectGTM()?

Yes.

>
> Datanodes will need some more thinking. We can always send in a
> "select 1" to them too just to see if they are up and about. But that
> will unnecessarily consume local xids.

In terms of local xid, because 'select 1' is read-only transaction,
datanode will not consume local xids.

>
> OTOH, we can contact it via a coordinator. So in that case we can
> extend the code you added for the coordinator to additionally take in
> a node name. And if it's provided it contacts the datanode otherwise
> it runs a "select 1" on itself or something like that.

This means that we need to know what coordinator is alive.   It may
not fit to automatic failover system.    XC can continue cluster
operation even though some of the coordinators crashed.   In this
situation, we need a means to detect if specific datanode is alive
without coordinator's help.

> Regards,
> Nikhils
>
>> Sorry the patch to filelist.sgmlin includes fix to restrict
>> pgxc_clean, pgxc_ddl and pgxc_monitor only to XC.
>>
>> Regards;
>> ----------
>> Koichi Suzuki
>>
>> ------------------------------------------------------------------------------
>> Live Security Virtual Conference
>> Exclusive live event will cover all the ways today's security and
>> threat landscape has changed and how IT managers can respond. Discussions
>> will include endpoint security, mobile security and the latest in malware
>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> _______________________________________________
>> Postgres-xc-developers mailing list
>> Pos...@li...
>> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers
>>
>
>
>
> --
> StormDB - http://www.stormdb.com
> The Database Cloud

On Thu, Jul 12, 2012 at 12:32 AM, Koichi Suzuki
<koi...@gm...> wrote:
> 2012/7/12 Nikhil Sontakke <ni...@st...>:
>> Hi Suzuki-San,
>>
>>
>>> Enclosed is a path of pgxc_monitor to check if gtm, gtm_proxy,
>>> coordinator or datanode is running.
>>
>> AFAICS, gtm_proxy  and datanode is not handled still?
>
> Yes, they're handled.   To be honest, pgxc_monitor does not
> distinguish gtm and gtm_proxy, coordinator and datanode because we're
> using the same protocol.
>

PFA, patch to fix the following:

* fixed "--help"

* "gtm_proxy", "coordinator" and "datanode" was not handled when
passed in to -Z.

* Added "-U" for username and "-d" for database name support. I know
you have mentioned that we should use .pgpass, but providing them on
the CLI is more convenient. And this utility is for maintenance
activities anyways, so no harm in providing -U, -d support on the cli
IMO.

* Some minor typos and arguments renaming

Regards,
Nikhils
-- 
StormDB - http://www.stormdb.com
The Database Cloud

Hi,

Thanks for the fix.   Will be committed after reviewing more closely.

Regards;
----------
Koichi Suzuki


2012/8/9 Nikhil Sontakke <ni...@st...>:
> On Thu, Jul 12, 2012 at 12:32 AM, Koichi Suzuki
> <koi...@gm...> wrote:
>> 2012/7/12 Nikhil Sontakke <ni...@st...>:
>>> Hi Suzuki-San,
>>>
>>>
>>>> Enclosed is a path of pgxc_monitor to check if gtm, gtm_proxy,
>>>> coordinator or datanode is running.
>>>
>>> AFAICS, gtm_proxy  and datanode is not handled still?
>>
>> Yes, they're handled.   To be honest, pgxc_monitor does not
>> distinguish gtm and gtm_proxy, coordinator and datanode because we're
>> using the same protocol.
>>
>
> PFA, patch to fix the following:
>
> * fixed "--help"
>
> * "gtm_proxy", "coordinator" and "datanode" was not handled when
> passed in to -Z.
>
> * Added "-U" for username and "-d" for database name support. I know
> you have mentioned that we should use .pgpass, but providing them on
> the CLI is more convenient. And this utility is for maintenance
> activities anyways, so no harm in providing -U, -d support on the cli
> IMO.
>
> * Some minor typos and arguments renaming
>
> Regards,
> Nikhils
> --
> StormDB - http://www.stormdb.com
> The Database Cloud

Nikhil, please also update related sgml documentation.
Thanks,

On Thu, Aug 9, 2012 at 6:16 PM, Nikhil Sontakke <ni...@st...> wrote:

> On Thu, Jul 12, 2012 at 12:32 AM, Koichi Suzuki
> <koi...@gm...> wrote:
> > 2012/7/12 Nikhil Sontakke <ni...@st...>:
> >> Hi Suzuki-San,
> >>
> >>
> >>> Enclosed is a path of pgxc_monitor to check if gtm, gtm_proxy,
> >>> coordinator or datanode is running.
> >>
> >> AFAICS, gtm_proxy  and datanode is not handled still?
> >
> > Yes, they're handled.   To be honest, pgxc_monitor does not
> > distinguish gtm and gtm_proxy, coordinator and datanode because we're
> > using the same protocol.
> >
>
> PFA, patch to fix the following:
>
> * fixed "--help"
>
> * "gtm_proxy", "coordinator" and "datanode" was not handled when
> passed in to -Z.
>
> * Added "-U" for username and "-d" for database name support. I know
> you have mentioned that we should use .pgpass, but providing them on
> the CLI is more convenient. And this utility is for maintenance
> activities anyways, so no harm in providing -U, -d support on the cli
> IMO.
>
> * Some minor typos and arguments renaming
>
> Regards,
> Nikhils
> --
> StormDB - http://www.stormdb.com
> The Database Cloud
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> Postgres-xc-developers mailing list
> Pos...@li...
> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers
>
>


-- 
Michael Paquier
http://michael.otacoo.com

OK, I am having a look at that, and here are some comments.
1) The sgml documentation is not updated with the new options
(doc-xc/src/sgml/pgxcmonitor.sgmlin)
2) The code inside do_node_ping is getting *more* dirty...
Why not simply completing the psql command line based on each option
supplied successively?
Something like:
char command[SIZE] = "psql -c \"select 1 a\"";
if (host)
    psql += " -h $host"
if (port)
    psql += " -p $port"
if (user)
    psql += " -U $user"
if (database)
    psql += "database"
else
    psql += "postgres"

Such a cascading structure will save a lot of efforts for code readability.
Of course, you need to complete the command string with a sprintf or
similar.
This java-like code is just to show the idea ;).
3) Thanks for the addition of the "help" call, it should be a separate
patch though. This bug is not related to your patch.
4) Not completely related to your patch, but you add more. The first letter
of Coordinator and Datanode is upper case. This is a name format to define
components more or less respected in the code.

I am going to fix the help bug and the grammar in the code and directly
commit that. I also found another bug when only port is specified...

However, I tested your feature with user and database and it works as
expected. Could you fix your patch based on my comments 1) and 2)?
Thanks.

On Fri, Aug 10, 2012 at 1:03 PM, Michael Paquier
<mic...@gm...>wrote:

> Nikhil, please also update related sgml documentation.
> Thanks,
>
> On Thu, Aug 9, 2012 at 6:16 PM, Nikhil Sontakke <ni...@st...>wrote:
>
>> On Thu, Jul 12, 2012 at 12:32 AM, Koichi Suzuki
>> <koi...@gm...> wrote:
>> > 2012/7/12 Nikhil Sontakke <ni...@st...>:
>> >> Hi Suzuki-San,
>> >>
>> >>
>> >>> Enclosed is a path of pgxc_monitor to check if gtm, gtm_proxy,
>> >>> coordinator or datanode is running.
>> >>
>> >> AFAICS, gtm_proxy  and datanode is not handled still?
>> >
>> > Yes, they're handled.   To be honest, pgxc_monitor does not
>> > distinguish gtm and gtm_proxy, coordinator and datanode because we're
>> > using the same protocol.
>> >
>>
>> PFA, patch to fix the following:
>>
>> * fixed "--help"
>>
>> * "gtm_proxy", "coordinator" and "datanode" was not handled when
>> passed in to -Z.
>>
>> * Added "-U" for username and "-d" for database name support. I know
>> you have mentioned that we should use .pgpass, but providing them on
>> the CLI is more convenient. And this utility is for maintenance
>> activities anyways, so no harm in providing -U, -d support on the cli
>> IMO.
>>
>> * Some minor typos and arguments renaming
>>
>> Regards,
>> Nikhils
>> --
>> StormDB - http://www.stormdb.com
>> The Database Cloud
>>
>>
>> ------------------------------------------------------------------------------
>> Live Security Virtual Conference
>> Exclusive live event will cover all the ways today's security and
>> threat landscape has changed and how IT managers can respond. Discussions
>> will include endpoint security, mobile security and the latest in malware
>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> _______________________________________________
>> Postgres-xc-developers mailing list
>> Pos...@li...
>> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers
>>
>>
>
>
> --
> Michael Paquier
> http://michael.otacoo.com
>



-- 
Michael Paquier
http://michael.otacoo.com

Hi Nikhil,

Do you have a patch for that?
I think that I might finish the implementation myself, it should not take
that much time.
Thanks,

On Tue, Aug 14, 2012 at 2:38 PM, Michael Paquier
<mic...@gm...>wrote:

> OK, I am having a look at that, and here are some comments.
> 1) The sgml documentation is not updated with the new options
> (doc-xc/src/sgml/pgxcmonitor.sgmlin)
> 2) The code inside do_node_ping is getting *more* dirty...
> Why not simply completing the psql command line based on each option
> supplied successively?
> Something like:
> char command[SIZE] = "psql -c \"select 1 a\"";
> if (host)
>     psql += " -h $host"
> if (port)
>     psql += " -p $port"
> if (user)
>     psql += " -U $user"
> if (database)
>     psql += "database"
> else
>     psql += "postgres"
>
> Such a cascading structure will save a lot of efforts for code
> readability. Of course, you need to complete the command string with a
> sprintf or similar.
> This java-like code is just to show the idea ;).
> 3) Thanks for the addition of the "help" call, it should be a separate
> patch though. This bug is not related to your patch.
> 4) Not completely related to your patch, but you add more. The first
> letter of Coordinator and Datanode is upper case. This is a name format to
> define components more or less respected in the code.
>
> I am going to fix the help bug and the grammar in the code and directly
> commit that. I also found another bug when only port is specified...
>
> However, I tested your feature with user and database and it works as
> expected. Could you fix your patch based on my comments 1) and 2)?
> Thanks.
>
>
> On Fri, Aug 10, 2012 at 1:03 PM, Michael Paquier <
> mic...@gm...> wrote:
>
>> Nikhil, please also update related sgml documentation.
>> Thanks,
>>
>> On Thu, Aug 9, 2012 at 6:16 PM, Nikhil Sontakke <ni...@st...>wrote:
>>
>>> On Thu, Jul 12, 2012 at 12:32 AM, Koichi Suzuki
>>> <koi...@gm...> wrote:
>>> > 2012/7/12 Nikhil Sontakke <ni...@st...>:
>>> >> Hi Suzuki-San,
>>> >>
>>> >>
>>> >>> Enclosed is a path of pgxc_monitor to check if gtm, gtm_proxy,
>>> >>> coordinator or datanode is running.
>>> >>
>>> >> AFAICS, gtm_proxy  and datanode is not handled still?
>>> >
>>> > Yes, they're handled.   To be honest, pgxc_monitor does not
>>> > distinguish gtm and gtm_proxy, coordinator and datanode because we're
>>> > using the same protocol.
>>> >
>>>
>>> PFA, patch to fix the following:
>>>
>>> * fixed "--help"
>>>
>>> * "gtm_proxy", "coordinator" and "datanode" was not handled when
>>> passed in to -Z.
>>>
>>> * Added "-U" for username and "-d" for database name support. I know
>>> you have mentioned that we should use .pgpass, but providing them on
>>> the CLI is more convenient. And this utility is for maintenance
>>> activities anyways, so no harm in providing -U, -d support on the cli
>>> IMO.
>>>
>>> * Some minor typos and arguments renaming
>>>
>>> Regards,
>>> Nikhils
>>> --
>>> StormDB - http://www.stormdb.com
>>> The Database Cloud
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Live Security Virtual Conference
>>> Exclusive live event will cover all the ways today's security and
>>> threat landscape has changed and how IT managers can respond. Discussions
>>> will include endpoint security, mobile security and the latest in malware
>>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>>> _______________________________________________
>>> Postgres-xc-developers mailing list
>>> Pos...@li...
>>> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers
>>>
>>>
>>
>>
>> --
>> Michael Paquier
>> http://michael.otacoo.com
>>
>
>
>
> --
> Michael Paquier
> http://michael.otacoo.com
>



-- 
Michael Paquier
http://michael.otacoo.com

Apologies for the delay Michael,

Here is the modified patch as per comments 1 and 2. I don't think we
need a version against 1.0 stable do we?

Regards,
Nikhils

On Mon, Aug 20, 2012 at 5:37 PM, Michael Paquier
<mic...@gm...> wrote:
> Hi Nikhil,
>
> Do you have a patch for that?
> I think that I might finish the implementation myself, it should not take
> that much time.
> Thanks,
>
>
> On Tue, Aug 14, 2012 at 2:38 PM, Michael Paquier <mic...@gm...>
> wrote:
>>
>> OK, I am having a look at that, and here are some comments.
>> 1) The sgml documentation is not updated with the new options
>> (doc-xc/src/sgml/pgxcmonitor.sgmlin)
>> 2) The code inside do_node_ping is getting *more* dirty...
>> Why not simply completing the psql command line based on each option
>> supplied successively?
>> Something like:
>> char command[SIZE] = "psql -c \"select 1 a\"";
>> if (host)
>>     psql += " -h $host"
>> if (port)
>>     psql += " -p $port"
>> if (user)
>>     psql += " -U $user"
>> if (database)
>>     psql += "database"
>> else
>>     psql += "postgres"
>>
>> Such a cascading structure will save a lot of efforts for code
>> readability. Of course, you need to complete the command string with a
>> sprintf or similar.
>> This java-like code is just to show the idea ;).
>> 3) Thanks for the addition of the "help" call, it should be a separate
>> patch though. This bug is not related to your patch.
>> 4) Not completely related to your patch, but you add more. The first
>> letter of Coordinator and Datanode is upper case. This is a name format to
>> define components more or less respected in the code.
>>
>> I am going to fix the help bug and the grammar in the code and directly
>> commit that. I also found another bug when only port is specified...
>>
>> However, I tested your feature with user and database and it works as
>> expected. Could you fix your patch based on my comments 1) and 2)?
>> Thanks.
>>
>>
>> On Fri, Aug 10, 2012 at 1:03 PM, Michael Paquier
>> <mic...@gm...> wrote:
>>>
>>> Nikhil, please also update related sgml documentation.
>>> Thanks,
>>>
>>> On Thu, Aug 9, 2012 at 6:16 PM, Nikhil Sontakke <ni...@st...>
>>> wrote:
>>>>
>>>> On Thu, Jul 12, 2012 at 12:32 AM, Koichi Suzuki
>>>> <koi...@gm...> wrote:
>>>> > 2012/7/12 Nikhil Sontakke <ni...@st...>:
>>>> >> Hi Suzuki-San,
>>>> >>
>>>> >>
>>>> >>> Enclosed is a path of pgxc_monitor to check if gtm, gtm_proxy,
>>>> >>> coordinator or datanode is running.
>>>> >>
>>>> >> AFAICS, gtm_proxy  and datanode is not handled still?
>>>> >
>>>> > Yes, they're handled.   To be honest, pgxc_monitor does not
>>>> > distinguish gtm and gtm_proxy, coordinator and datanode because we're
>>>> > using the same protocol.
>>>> >
>>>>
>>>> PFA, patch to fix the following:
>>>>
>>>> * fixed "--help"
>>>>
>>>> * "gtm_proxy", "coordinator" and "datanode" was not handled when
>>>> passed in to -Z.
>>>>
>>>> * Added "-U" for username and "-d" for database name support. I know
>>>> you have mentioned that we should use .pgpass, but providing them on
>>>> the CLI is more convenient. And this utility is for maintenance
>>>> activities anyways, so no harm in providing -U, -d support on the cli
>>>> IMO.
>>>>
>>>> * Some minor typos and arguments renaming
>>>>
>>>> Regards,
>>>> Nikhils
>>>> --
>>>> StormDB - http://www.stormdb.com
>>>> The Database Cloud
>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Live Security Virtual Conference
>>>> Exclusive live event will cover all the ways today's security and
>>>> threat landscape has changed and how IT managers can respond.
>>>> Discussions
>>>> will include endpoint security, mobile security and the latest in
>>>> malware
>>>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>>>> _______________________________________________
>>>> Postgres-xc-developers mailing list
>>>> Pos...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers
>>>>
>>>
>>>
>>>
>>> --
>>> Michael Paquier
>>> http://michael.otacoo.com
>>
>>
>>
>>
>> --
>> Michael Paquier
>> http://michael.otacoo.com
>
>
>
>
> --
> Michael Paquier
> http://michael.otacoo.com



-- 
StormDB - http://www.stormdb.com
The Database Cloud