Re: [HACKERS] ECPG: WHENEVER statement with DO CONTINUE action

[Vinayak Pokale]

On Aug 25, 2017 10:45 PM, "Michael Meskes" <mes...@postgresql.org> wrote:
>
> > The v3 patch looks good to me. I've changed this patch status to Ready
> > for Committer.
>
> Thank you all, committed.

Thank you very much.

Regards,
Vinayak Pokale

Re: [HACKERS] ECPG: WHENEVER statement with DO CONTINUE action

[Vinayak Pokale]

Thank you for your reply

On Jun 9, 2017 5:39 PM, "Michael Meskes" <mes...@postgresql.org> wrote:
>
> Hi,
>
> > To develop the ECPG application more efficiently and improve
> > portability,
> > I would like to suggest one minor improvement "WHENEVER condition DO
> > CONTINUE" support in ECPG.
> > Oracle Pro*C supports WHENEVER statement with DO CONTINUE action.[1]
> >
> > EXEC SQL WHENEVER SQLERROR CONTINUE;
> > is not same as
> > EXEC SQL WHENEVER SQLERROR DO CONTINUE;
> >
> > The CONTINUE action instructs the client application to proceed to
> > the next statement whereas DO CONTINUE action instructs the client
> > application to emit a C continue statement and the flow of control
> > return to the beginning of the enclosing loop.
>
> This did actual escape me. Thanks for bringing it to our attention and
> fixing this missing functionality.
>
> > I have tried to implement it. Please check the attached patch.
> > Please give me feedback.
> > ...
>
> Could you please add a "DO CONTINUE" case to one of the test cases? Or
> add a new one? We would need a test case IMO.
>
Yes I will add test case and send updated patch.

Regards,
Vinayak Pokale

Re: [HACKERS] [PATCH] New command to monitor progression of long running queries

[Vinayak Pokale]

On Mon, Apr 17, 2017 at 9:09 PM, Remi Colinet 
wrote:

> Hello,
>
> I've implemented a new command named PROGRESS to monitor progression of
> long running SQL queries in a backend process.
>
> Thank you for the patch.

I am testing your patch but after applying your patch other regression test
failed.

$ make installcheck
13 of 178 tests failed.

Regards,
Vinayak

Re: [HACKERS] Transactions involving multiple postgres foreign servers

[Vinayak Pokale]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

I have tested the latest patch and it looks good to me,
so I marked it "Ready for committer".
Anyway, it would be great if anyone could also have a look at the patches and 
send comments.

The new status of this patch is: Ready for Committer

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Transactions involving multiple postgres foreign servers

[Vinayak Pokale]

Hi All,

Ashutosh proposed the feature 2PC for FDW for achieving atomic commits
across multiple foreign servers.
If a transaction make changes to more than two foreign servers the current
implementation in postgres_fdw doesn't make sure that either all of them
commit or all of them rollback their changes.

We (Masahiko Sawada and me) reopen this thread and trying to contribute in
it.

2PC for FDW

The patch provides support for atomic commit for transactions involving
foreign servers. when the transaction makes changes to foreign servers,
either all the changes to all the foreign servers commit or rollback.

The new patch 2PC for FDW include the following things:
1. The patch 0001 introduces a generic feature. All kinds of FDW that
support 2PC such as oracle_fdw, mysql_fdw, postgres_fdw etc. can involve in
the transaction.

Currently we can push some conditions down to shard nodes, especially in
9.6 the directly modify feature has
been introduced. But such a transaction modifying data on shard node is not
executed surely.
Using 0002 patch, that modify is executed with 2PC. It means that we almost
can provide sharding solution using
multiple PostgreSQL server (one parent node and several shared node).

For multi master, we definitely need transaction manager but transaction
manager probably can use this 2PC for FDW feature to manage distributed
transaction.

2. 0002 patch makes postgres_fdw possible to use 2PC.

0002 patch makes postgres_fdw to use below APIs. These APIs are generic
features which can be used by all kinds of FDWs.

a. Execute PREAPRE TRANSACTION and COMMIT/ABORT PREAPRED instead of
COMMIT/ABORT on foreign server which supports 2PC.
b. Manage information of foreign prepared transactions resolver

Masahiko Sawada will post the patch.

Suggestions and comments are helpful to implement this feature.

Regards,

Vinayak Pokale

On Mon, Feb 1, 2016 at 11:14 PM, Alvaro Herrera <alvhe...@2ndquadrant.com>
wrote:

> Alvaro Herrera wrote:
> > Ashutosh Bapat wrote:
> >
> > > Here's updated patch. I didn't use version numbers in file names in my
> > > previous patches. I am starting from this onwards.
> >
> > Um, I tried this patch and it doesn't apply at all.  There's a large
> > number of conflicts.  Please update it and resubmit to the next
> > commitfest.
>
> Also, please run "git show --check" of "git diff origin/master --check"
> and fix the whitespace problems that it shows.  It's an easy thing but
> there's a lot of red squares in my screen.
>
> --
> Álvaro Herrerahttp://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Vinayak Pokale]

Hello,

On Fri, Feb 26, 2016 at 6:19 PM, Amit Langote  wrote:

>
> Hi Vinayak,
>
> Thanks for updating the patch! A quick comment:
>
> On 2016/02/26 17:28, poku...@pm.nttdata.co.jp wrote:
> >> CREATE VIEW pg_stat_vacuum_progress AS
> >>   SELECT S.s[1] as pid,
> >>  S.s[2] as relid,
> >>  CASE S.s[3]
> >>WHEN 1 THEN 'Scanning Heap'
> >>WHEN 2 THEN 'Vacuuming Index and Heap'
> >>ELSE 'Unknown phase'
> >>  END,
> >>
> >>   FROM pg_stat_get_command_progress(PROGRESS_COMMAND_VACUUM) as S;
> >>
> >> # The name of the function could be other than *_command_progress.
> > The name of function is updated as pg_stat_get_progress_info() and also
> updated the function.
> > Updated the pg_stat_vacuum_progress view as suggested.
>
> So, pg_stat_get_progress_info() now accepts a parameter to distinguish
> different commands.  I see the following in its definition:
>
> +   /*  Report values for only those backends which are
> running VACUUM
> command */
> +   if (cmdtype == COMMAND_LAZY_VACUUM)
> +   {
> +   /*Progress can only be viewed by role member.*/
> +   if (has_privs_of_role(GetUserId(),
> beentry->st_userid))
> +   {
> +   values[2] =
> UInt32GetDatum(beentry->st_progress_param[0]);
> +   values[3] =
> UInt32GetDatum(beentry->st_progress_param[1]);
> +   values[4] =
> UInt32GetDatum(beentry->st_progress_param[2]);
> +   values[5] =
> UInt32GetDatum(beentry->st_progress_param[3]);
> +   values[6] =
> UInt32GetDatum(beentry->st_progress_param[4]);
> +   values[7] =
> UInt32GetDatum(beentry->st_progress_param[5]);
> +   if (beentry->st_progress_param[1] != 0)
> +   values[8] =
> Float8GetDatum(beentry->st_progress_param[2] * 100 /
> beentry->st_progress_param[1]);
> +   else
> +   nulls[8] = true;
> +   }
> +   else
> +   {
> +   nulls[2] = true;
> +   nulls[3] = true;
> +   nulls[4] = true;
> +   nulls[5] = true;
> +   nulls[6] = true;
> +   nulls[7] = true;
> +   nulls[8] = true;
> +   }
> +   }
>
> How about doing this in a separate function which takes the command id as
> parameter and returns an array of values and the number of values (per
> command id). pg_stat_get_progress_info() then creates values[] and nulls[]
> arrays from that and returns that as result set.  It will be a cleaner
> separation of activities, perhaps.
>
> +1

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Vinayak Pokale]

Hi,

Please find attached updated patch with an updated interface.

On Jan 26, 2016 11:22 AM, "Vinayak Pokale" <vinpok...@gmail.com> wrote:
>
> Hi Amit,
>
> Thank you for reviewing the patch.
>
> On Jan 26, 2016 9:51 AM, "Amit Langote" <langote_amit...@lab.ntt.co.jp>
wrote:
> >
> >
> > Hi Vinayak,
> >
> > On 2016/01/25 20:58, Vinayak Pokale wrote:
> > > Hi,
> > >
> > > Please find attached updated patch with an updated interface.
> > >
> >
> > Thanks for updating the patch.
> >
> > > I added the below interface to update the
> > > scanned_heap_pages,scanned_index_pages and index_scan_count only.
> > > void pgstat_report_progress_scanned_pages(int num_of_int, uint32
> > > *progress_scanned_pages_param)
> >
> > I think it's still the same interface with the names changed. IIRC, what
> > was suggested was to provide a way to not have to pass the entire array
> > for the update of a single member of it. Just pass the index of the
> > updated member and its new value. Maybe, something like:
> >
> > void pgstat_progress_update_counter(int index, uint32 newval);
> >
> > The above function would presumably update the value of
> > beentry.st_progress_counter[index] or something like that.

Following interface functions are added:

/*
 * index: in the array of uint32 counters in the beentry
 * counter: new value of the (index)th counter
 */
void
pgstat_report_progress_update_counter(int index, uint32 counter)

/*
called to updatet the VACUUM progress phase.
msg: new value of (index)th message
*/
void
pgstat_report_progress_update_message(int index, char
msg[N_PROGRESS_PARAM][PROGRESS_MESSAGE_LENGTH])

Regards,
Vinayak


Vacuum_progress_checker_v10.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Vinayak Pokale]

Hi,

Please find attached updated patch with an updated interface.

I added the below interface to update the
scanned_heap_pages,scanned_index_pages and index_scan_count only.
void pgstat_report_progress_scanned_pages(int num_of_int, uint32
*progress_scanned_pages_param)

Other interface functions which are called at the beginning:
void pgstat_report_progress_set_command_target(Oid relid)

Regards,
Vinayak

On Wed, Jan 13, 2016 at 3:16 PM, Amit Langote <langote_amit...@lab.ntt.co.jp
> wrote:

> On 2016/01/12 11:28, Vinayak Pokale wrote:
> > On Jan 12, 2016 11:22 AM, "Amit Langote" <langote_amit...@lab.ntt.co.jp>
> > wrote:
> >>
> >> On 2016/01/12 10:30, Amit Langote wrote:
> >>> I'm slightly concerned that the latest patch doesn't incorporate any
> >>> revisions to the original pgstat interface per Robert's comments in
> [1].
> >>
> >> I meant to say "originally proposed pgstat interface on this thread".
> >
> > Yes.
> > Robert's comments related to pgstat interface needs to be address.
> > I will update it.
>
> So, I updated the patch status to "Waiting on Author".
>
> Thanks,
> Amit
>
>
>


Vacuum_progress_checker_v9.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Vinayak Pokale]

Hi Amit,

Thank you for reviewing the patch.
On Jan 26, 2016 9:51 AM, "Amit Langote" <langote_amit...@lab.ntt.co.jp>
wrote:
>
>
> Hi Vinayak,
>
> On 2016/01/25 20:58, Vinayak Pokale wrote:
> > Hi,
> >
> > Please find attached updated patch with an updated interface.
> >
>
> Thanks for updating the patch.
>
> > I added the below interface to update the
> > scanned_heap_pages,scanned_index_pages and index_scan_count only.
> > void pgstat_report_progress_scanned_pages(int num_of_int, uint32
> > *progress_scanned_pages_param)
>
> I think it's still the same interface with the names changed. IIRC, what
> was suggested was to provide a way to not have to pass the entire array
> for the update of a single member of it. Just pass the index of the
> updated member and its new value. Maybe, something like:
>
> void pgstat_progress_update_counter(int index, uint32 newval);
>
> The above function would presumably update the value of
> beentry.st_progress_counter[index] or something like that.

Understood. I will update the patch.

Regards,
Vinayak

[HACKERS] Typo in sequence.c

[Vinayak Pokale]

Hi,

I found a typo in sequence.c
Please check the attached patch.

Regards,
Vinayak


typo-sequence-c.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Vinayak Pokale]

On Jan 12, 2016 11:22 AM, "Amit Langote" <langote_amit...@lab.ntt.co.jp>
wrote:
>
> On 2016/01/12 10:30, Amit Langote wrote:
> > I'm slightly concerned that the latest patch doesn't incorporate any
> > revisions to the original pgstat interface per Robert's comments in [1].
>
> I meant to say "originally proposed pgstat interface on this thread".

Yes.
Robert's comments related to pgstat interface needs to be address.
I will update it.

Regards,
Vinayak Pokale

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Vinayak Pokale]

Hi Sudhir,

On Jan 7, 2016 3:02 AM, "Sudhir Lonkar-2" <sudhir.lon...@nttdata.com> wrote:
>
> Hello,
> >Please find attached patch addressing following comments
> I have tested this patch.
> It is showing empty (null) in phase column of pg_stat_vacuum_progress,
when
> I switched to a unprivileged user.
> In the previous patch, it is showing  in phase
> column.
Yes. I will update the patch.
Regards,
Vinayak Pokale

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Vinayak Pokale]

Hi,
Please find attached patch addressing following comments.

>The relation OID should be reported and not its name. In case of a
>relation rename that would not be cool for tracking, and most users
>are surely going to join with other system tables using it.
The relation OID is reported instead of relation name.
The following interface function is called at the beginning to report the
relation OID once.
void pgstat_report_command_target(Oid relid)

>Regarding pg_stat_get_vacuum_progress(): I think a backend can simply be
>skipped if (!has_privs_of_role(GetUserId(), beentry->st_userid)) (cannot
>put that in plain English, :))
Updated in the attached patch.

In the previous patch, ACTIVITY_IS_VACUUM is set unnecessarily for
VACOPT_FULL and they are not covered by lazy_scan_heap().
I have updated it in attached patch and also renamed ACTIVITY_IS_VACUUM to
COMMAND_LAZY_VACUUM.

Added documentation for view.
Some more comments need to be addressed.

Regards,

Vinayak Pokale

On Sat, Dec 12, 2015 at 2:07 AM, Robert Haas <robertmh...@gmail.com> wrote:

> On Fri, Dec 11, 2015 at 1:25 AM, Michael Paquier
> <michael.paqu...@gmail.com> wrote:
> >> For another thing, there are definitely going to be
> >> some people that want the detailed information - and I can practically
> >> guarantee that if we don't make it available, at least one person will
> >> write a tool that tries to reverse-engineer the detailed progress
> >> information from whatever we do report.
> >
> > OK, so this justifies the fact of having detailed information, but
> > does it justify the fact of having precise and accurate data? ISTM
> > that having detailed information and precise information are two
> > different things. The level of details is defined depending on how
> > verbose we want the information to be, and the list you are giving
> > would fulfill this requirement nicely for VACUUM. The level of
> > precision/accuracy at which this information is provided though
> > depends at which frequency we want to send this information. For
> > long-running VACUUM it does not seem necessary to update the fields of
> > the progress tracker each time a counter needs to be incremented.
> > CLUSTER has been mentioned as well as a potential target for the
> > progress facility, but it seems that it enters as well in the category
> > of things that would need to be reported on a slower frequency pace
> > than "each-time-a-counter-is-incremented".
> >
> > My impression is just based on the needs of VACUUM and CLUSTER.
> > Perhaps I am lacking imagination regarding the potential use cases of
> > the progress facility though in cases where we'd want to provide
> > extremely precise progress information :)
> > It just seems to me that this is not a requirement for VACUUM or
> > CLUSTER. That's all.
>
> It's not a hard requirement, but it should be quite easy to do without
> adding any significant overhead.  All you need to do is something
> like:
>
> foo->changecount++;
> pg_write_barrier();
> foo->count_of_blocks++;
> pg_write_barrier();
> foo->changecount++;
>
> I suspect that's plenty cheap enough to do for every block.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


Vacuum_progress_checker_v8.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers