Re: [HACKERS] Reviewing freeze map code

[Noah Misch]

On Fri, May 06, 2016 at 04:42:48PM -0400, Robert Haas wrote:
> On Thu, May 5, 2016 at 2:20 PM, Andres Freund  wrote:
> > On 2016-05-02 14:48:18 -0700, Andres Freund wrote:
> > +   charnew_vmbuf[BLCKSZ];
> > +   char   *new_cur = new_vmbuf;
> > +   boolempty = true;
> > +   boolold_lastpart;
> > +
> > +   /* Copy page header in advance */
> > +   memcpy(new_vmbuf, , 
> > SizeOfPageHeaderData);
> >
> > Shouldn't we zero out new_vmbuf? Afaics we're not necessarily zeroing it
> > with old_lastpart && !empty, right?
> 
> Oh, dear.  That seems like a possible data corruption bug.  Maybe we'd
> better fix that right away (although I don't actually have time before
> the wrap).

[This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com


-- 
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] Performance degradation in commit ac1d794

[Noah Misch]

On Fri, Dec 25, 2015 at 08:08:15PM +0300, Васильев Дмитрий wrote:
> I suddenly found commit ac1d794 gives up to 3 times performance degradation.
> 
> I tried to run pgbench -s 1000 -j 48 -c 48 -S -M prepared on 70 CPU-core
> machine:
> commit ac1d794 gives me 363,474 tps
> and previous commit a05dc4d gives me 956,146
> and master( 3d0c50f ) with revert ac1d794 gives me 969,265

[This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com


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

[HACKERS] Re: pg9.6 segfault using simple query (related to use fk for join estimates)

[Noah Misch]

On Fri, May 06, 2016 at 03:06:01PM -0400, Robert Haas wrote:
> On Thu, May 5, 2016 at 10:48 AM, David Rowley
>  wrote:
> > On 5 May 2016 at 16:04, David Rowley  wrote:
> >> I've started making some improvements to this, but need to talk to
> >> Tomas. It's currently in the middle of his night, but will try to
> >> catch him in his morning to discuss this with him.
> >
> > Ok, so I spoke to Tomas about this briefly, and he's asked me to send
> > in this patch. He didn't get time to look over it due to some other
> > commitments he has today.
> >
> > I do personally feel that if the attached is not good enough, or not
> > very close to good enough then probably the best course of action is
> > to revert the whole thing.
> 
> Tom, what do you think about this patch?  Is it good enough, or should
> we revert the whole thing?

[This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Simon,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com


-- 
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] Rename max_parallel_degree?

[Noah Misch]

On Fri, May 06, 2016 at 02:52:30PM -0400, Robert Haas wrote:
> OK, my reading of this thread is that there is a consensus is to
> redefine max_parallel_degree=1 as "no parallelism" and
> max_parallel_degree>1 as "parallelism using a leader plus N-1
> workers", and along with that, to keep the names unchanged.  However,
> I don't think I can get that done before beta1, at least not without a
> serious risk of breaking stuff.  I can look at this post-beta1.

[This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com


-- 
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] parallel.c is not marked as test covered

[Noah Misch]

On Sun, May 15, 2016 at 12:53:13PM +, Clément Prévost wrote:
> On Mon, May 9, 2016 at 4:50 PM Andres Freund  wrote:
> > I think it's a good idea to run a force-parallel run on some buildfarm
> > members. But I'm rather convinced that the core tests run by all animals
> > need some minimal coverage of parallel queries. Both because otherwise
> > it'll be hard to get some coverage of unusual platforms, and because
> > it's imo something rather relevant to test during development.
> >
> Good point.
> 
> After some experiments, I found out that, for my setup (9b7bfc3a88ef7b), a
> parallel seq scan is used given both parallel_setup_cost
> and parallel_tuple_cost are set to 0 and given that the table is at least 3
> times as large as the biggest test table tenk1.
> 
> The attached patch is a regression test using this method that is
> reasonably small and fast to run. I also hid the workers count from the
> explain output when costs are disabled as suggested by Tom Lane and Robert
> Haas on this same thread (
> http://www.postgresql.org/message-id/CA+TgmobBQS4ss3+CwoZOKgbsBqKfRndwc=hlialaep5axqc...@mail.gmail.com
> ).
> 
> Testing under these conditions does not test the planner job at all but at
> least some parallel code can be run on the build farm and the test suite
> gets 2643 more lines and 188 more function covered.
> 
> I don't know however if this test will be reliable on other platforms, some
> more feedback is needed here.

[This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com


-- 
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] Perf Benchmarking and regression.

[Noah Misch]

On Thu, May 12, 2016 at 10:49:06AM -0400, Robert Haas wrote:
> On Thu, May 12, 2016 at 8:39 AM, Ashutosh Sharma  
> wrote:
> > Please find the test results for the following set of combinations taken at
> > 128 client counts:
> >
> > 1) Unpatched master, default *_flush_after :  TPS = 10925.882396
> >
> > 2) Unpatched master, *_flush_after=0 :  TPS = 18613.343529
> >
> > 3) That line removed with #if 0, default *_flush_after :  TPS = 9856.809278
> >
> > 4) That line removed with #if 0, *_flush_after=0 :  TPS = 18158.648023
> 
> I'm getting increasingly unhappy about the checkpoint flush control.
> I saw major regressions on my parallel COPY test, too:
> 
> http://www.postgresql.org/message-id/ca+tgmoyouqf9cgcpgygngzqhcy-gcckryaqqtdu8kfe4n6h...@mail.gmail.com
> 
> That was a completely different machine (POWER7 instead of Intel,
> lousy disks instead of good ones) and a completely different workload.
> Considering these results, I think there's now plenty of evidence to
> suggest that this feature is going to be horrible for a large number
> of users.  A 45% regression on pgbench is horrible.  (Nobody wants to
> take even a 1% hit for snapshot too old, right?)  Sure, it might not
> be that way for every user on every Linux system, and I'm sure it
> performed well on the systems where Andres benchmarked it, or he
> wouldn't have committed it.  But our goal can't be to run well only on
> the newest hardware with the least-buggy kernel...

[This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Andres,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com


-- 
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] "Allow usage of huge maintenance_work_mem for GIN build" patch

[Noah Misch]

On Thu, May 12, 2016 at 11:33:24AM -0400, Robert Haas wrote:
> On Fri, May 6, 2016 at 7:58 PM, Peter Geoghegan  wrote:
> > I noticed that commit 30bb26b5 ("Allow usage of huge
> > maintenance_work_mem for GIN build") made the following modification:
> >
> > --- a/src/include/access/gin_private.h
> > +++ b/src/include/access/gin_private.h
> > @@ -903,7 +903,7 @@ typedef struct GinEntryAccumulator
> >  typedef struct
> >  {
> > GinState   *ginstate;
> > -   longallocatedMemory;
> > +   SizeallocatedMemory;
> > GinEntryAccumulator *entryallocator;
> > uint32  eas_used;
> > RBTree *tree;
> >
> > Are you sure this is safe, Teodor? I don't have time to study the
> > patch in detail, but offhand I think that it might have been better to
> > make allocatedMemory of type int64, just like the tuplesort.c memory
> > accounting variables are post-MaxAllocHuge. It's not obvious to me
> > that this variable isn't allowed to occasionally become negative, just
> > like in tuplesort.c. It looks like that *might* be true -- ginbulk.c
> > may let allocatedMemory go negative for a period, which would now be
> > broken.
> >
> > If you did make this exact error, you would not be the first. If it
> > isn't actually broken, perhaps you should still make this change,
> > simply on general principle. I'd like to hear other opinions on that,
> > though.
> 
> I've added this to the open items list.

[This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Teodor,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com


-- 
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] Statement timeout

[Tatsuo Ishii]

> Really?  It should get reported at some execution of CHECK_FOR_INTERRUPTS
> after we pass the 2-second mark in execute(portal1).  If that's not what
> you're observing, maybe you've found a code path that's missing some
> CHECK_FOR_INTERRUPTS call(s).

Oops. Previous example was not appropriate. Here are revised
examples. (in any case, the time consumed at parse and bind are small,
and I omit the CHECK_FOR_INTERRUPTS after these commands)

[example 1]

statement_timeout = 3s

parse(statement1) -- time 0. set statement timout timer
bind(statement1, portal1)
execute(portal1) -- took 2 seconds
CHECK_FOR_INTERRUPTS -- 2 seconds passed since time 0. statement timeout does 
not fire
parse(statement2)
bind(statement2, portal2)
execute(portal2) -- took 2 seconds
CHECK_FOR_INTERRUPTS -- 4 seconds passed since time 0. the statement timeout 
fires!

Another example.

[example 2]

statement_timeout = 3s

parse(statement1) -- time 0. set statement timout timer
bind(statement1, portal1)
execute(portal1) -- took 1 second
CHECK_FOR_INTERRUPTS -- 1 second passed since time 0. statement timeout does 
not fire
parse(statement2)
bind(statement2, portal2)
execute(portal2) -- took 1 second
CHECK_FOR_INTERRUPTS -- 2 seconds passed since time 0. the statement timeout 
fires!
[client is idle for 2 seconds]
sync
CHECK_FOR_INTERRUPTS -- 4 seconds passed since time 0. the statement timeout 
fires!

I think current code is good in detecting statement timeout if each
command execution time actually exceeds the specified timeout. However
it could report false statement timeout in some cases like above.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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] Does people favor to have matrix data type?

[Kouhei Kaigai]

> -Original Message-
> From: Joe Conway [mailto:m...@joeconway.com]
> Sent: Sunday, May 29, 2016 1:40 AM
> To: Kaigai Kouhei(海外 浩平); Jim Nasby; Ants Aasma; Simon Riggs
> Cc: pgsql-hackers@postgresql.org
> Subject: Re: [HACKERS] Does people favor to have matrix data type?
> 
> On 05/28/2016 07:12 AM, Kouhei Kaigai wrote:
> > Sparse matrix! It is a disadvantaged area for the current array format.
> >
> > I have two ideas. HPC folks often split a large matrix into multiple
> > grid. A grid is typically up to 1024x1024 matrix, for example.
> > If a grid is consists of all zero elements, it is obvious we don't need
> > to have individual elements on the grid.
> > One other idea is compression. If most of matrix is zero, it is an ideal
> > data for compression, and it is easy to reconstruct only when calculation.
> >
> >> Related to this, Tom has mentioned in the past that perhaps we should
> >> support abstract use of the [] construct. Currently point finds a way to
> >> make use of [], but I think that's actually coded into the grammar.
> >>
> > Yep, if we consider 2D-array is matrix, no special enhancement is needed
> > to use []. However, I'm inclined to have own data structure for matrix
> > to present the sparse matrix.
> 
> +1 I'm sure this would be useful for PL/R as well.
> 
> Joe
>
It is pretty good idea to combine PL/R and PL/CUDA (what I'm now working)
for advanced analytics. We will be able to off-load heavy computing portion
to GPU, then also utilize various R functions inside database.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 

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

[HACKERS] Misdesigned command/status APIs for parallel dump/restore

[Tom Lane]

pg_dump's APIs for parallel dump/restore include an
archive-format-specific MasterStartParallelItem function, which is
evidently intended to allow format-specific data to be sent to the
worker process during startup of a parallel sub-job.  However, no
such data can be sent in practice, because parsing of those commands
is hardwired in parallel.c's WaitForCommands() function; nor do the
APIs for the format-specific WorkerJobDump/WorkerJobRestore functions
allow any format-specific data to be passed to them.  So it's unsurprising
that the two existing instantiations of MasterStartParallelItem
are effectively duplicates.

I am pretty strongly tempted to get rid of MasterStartParallelItem
altogether and just hard-code what it does in DispatchJobForTocEntry.
That'd also allow getting rid of the ugly usage of static buffers there.

Alternatively, we could do what some of the comments suggest and make the
format-specific WorkerJobDump/WorkerJobRestore functions responsible for
parsing the command strings.  But that seems like it would just be adding
more duplicative code in support of flexibility that there's no use for.
The arguments to DispatchJobForTocEntry are just a TocEntry and a
T_Action, and that seems unlikely to change.

The situation for MasterEndParallelItem isn't a whole lot better.
At least the status strings are both created and parsed by format-
specific functions --- but those functions are completely duplicative,
performing no useful format-specific work, and there's no good reason
to think that we'd need format-specific work in future.  So I'm tempted
to get rid of the MasterEndParallelItem API as well.  Instead, have
WorkerJobDump/WorkerJobRestore just return integer status codes, and
perform all construction and parsing of the status messages in parallel.c.

It's possible to imagine that future archive formats might have use for,
say, passing an amount-of-data-written number from a worker back up to
the master.  But you could also implement that by relying on the
filesystem (that is, a master or another worker could check file size
to see how much had been written).  So I'm pretty skeptical that we
need extra flexibility here.

A different line of thought would be to fix the worker-command-parsing
situation by allowing the parsing to happen in format-specific code,
but avoid duplicative coding by letting archive formats share a common
implementation function if they had no need for any custom data.

Thoughts?

regards, tom lane


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

[HACKERS] pgdump/parallel.c: "aborting" flag is dead code

[Tom Lane]

parallel.c has an "aborting" flag that's entirely useless, as it's
set only during archive_close_connection(), and tested only in places
that are not reached after that.  I've confirmed this both by code
reading and by testing.  It appears from some of the comments that
there was once an intent to shut down workers by sending an explicit
TERMINATE message, rather than just closing the pipe, and in that
case maybe there would have been some value in this flag.  As is,
though, it just sows confusion, so I propose removing it as attached.

regards, tom lane

diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index 56a74cb..e9e8698 100644
*** a/src/bin/pg_dump/parallel.c
--- b/src/bin/pg_dump/parallel.c
*** static int	piperead(int s, char *buf, in
*** 95,105 
  
  #else			/* !WIN32 */
  
! /*
!  * Variables for handling signals.  aborting is only ever used in the master,
!  * the workers just need wantAbort.
!  */
! static bool aborting = false;
  static volatile sig_atomic_t wantAbort = 0;
  
  /* Non-Windows implementation of pipe access */
--- 95,101 
  
  #else			/* !WIN32 */
  
! /* Signal handler flag */
  static volatile sig_atomic_t wantAbort = 0;
  
  /* Non-Windows implementation of pipe access */
*** archive_close_connection(int code, void 
*** 301,314 
  			if (si->AHX)
  DisconnectDatabase(si->AHX);
  
- #ifndef WIN32
- 
- 			/*
- 			 * Setting aborting to true shuts off error/warning messages that
- 			 * are no longer useful once we start killing workers.
- 			 */
- 			aborting = true;
- #endif
  			ShutdownWorkersHard(si->pstate);
  		}
  		else
--- 297,302 
*** select_loop(int maxFd, fd_set *workerset
*** 1178,1188 
  		/*
  		 * If we Ctrl-C the master process, it's likely that we interrupt
  		 * select() here. The signal handler will set wantAbort == true and
! 		 * the shutdown journey starts from here. Note that we'll come back
! 		 * here later when we tell all workers to terminate and read their
! 		 * responses. But then we have aborting set to true.
  		 */
! 		if (wantAbort && !aborting)
  			exit_horribly(modulename, "terminated by user\n");
  
  		if (i < 0 && errno == EINTR)
--- 1166,1174 
  		/*
  		 * If we Ctrl-C the master process, it's likely that we interrupt
  		 * select() here. The signal handler will set wantAbort == true and
! 		 * the shutdown journey starts from here.
  		 */
! 		if (wantAbort)
  			exit_horribly(modulename, "terminated by user\n");
  
  		if (i < 0 && errno == EINTR)
*** sendMessageToWorker(ParallelState *pstat
*** 1279,1295 
  
  	if (pipewrite(pstate->parallelSlot[worker].pipeWrite, str, len) != len)
  	{
! 		/*
! 		 * If we're already aborting anyway, don't care if we succeed or not.
! 		 * The child might have gone already.  (XXX but if we're aborting
! 		 * already, why are we here at all?)
! 		 */
! #ifndef WIN32
! 		if (!aborting)
! #endif
! 			exit_horribly(modulename,
! 		"could not write to the communication channel: %s\n",
! 		  strerror(errno));
  	}
  }
  
--- 1265,1273 
  
  	if (pipewrite(pstate->parallelSlot[worker].pipeWrite, str, len) != len)
  	{
! 		exit_horribly(modulename,
! 	  "could not write to the communication channel: %s\n",
! 	  strerror(errno));
  	}
  }
  

-- 
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] Does people favor to have matrix data type?

[Joe Conway]

On 05/28/2016 07:12 AM, Kouhei Kaigai wrote:
> Sparse matrix! It is a disadvantaged area for the current array format.
> 
> I have two ideas. HPC folks often split a large matrix into multiple
> grid. A grid is typically up to 1024x1024 matrix, for example.
> If a grid is consists of all zero elements, it is obvious we don't need
> to have individual elements on the grid.
> One other idea is compression. If most of matrix is zero, it is an ideal
> data for compression, and it is easy to reconstruct only when calculation.
> 
>> Related to this, Tom has mentioned in the past that perhaps we should
>> support abstract use of the [] construct. Currently point finds a way to
>> make use of [], but I think that's actually coded into the grammar.
>>
> Yep, if we consider 2D-array is matrix, no special enhancement is needed
> to use []. However, I'm inclined to have own data structure for matrix
> to present the sparse matrix.

+1 I'm sure this would be useful for PL/R as well.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Statement timeout

[Tom Lane]

Tatsuo Ishii  writes:
> When extended query protocol message is used, statement timeout is not
> checked until a sync message is received (more precisely, statement
> timeout timer is canceled while processing the sync message, and
> actual checking timeout is done in CHECK_FOR_INTERRUPTS). Example:

> parse(statement1)
> bind(statement1, portal1)
> execute(portal1)
> parse(statement2)
> bind(statement2, portal2)
> execute(portal2)
> sync

> Suppose statement_timeout = 2s. If execute(portal1) takes 3 seconds,
> and execute(portal2) takes 1 second, the statement timeout is reported
> at the time when the sync message is processed which is right after
> execute(portal2).

Really?  It should get reported at some execution of CHECK_FOR_INTERRUPTS
after we pass the 2-second mark in execute(portal1).  If that's not what
you're observing, maybe you've found a code path that's missing some
CHECK_FOR_INTERRUPTS call(s).

regards, tom lane


-- 
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] Parallel pg_dump's error reporting doesn't work worth squat

[Tom Lane]

Amit Kapila  writes:
> On Sat, May 28, 2016 at 5:06 AM, Tom Lane  wrote:
>> It looks like the vacuumdb.c version of this code actually is tied
>> into an interrupt handler, but whoever copied it for parallel.c just
>> ripped out the CancelRequested checks, making the looping behavior
>> pretty useless.

> It seems to me that CancelRequested checks were introduced in vacuumdb.c as
> part of commit a1792320 and select_loop for parallel.c version exists from
> commit 9e257a18 which got committed earlier.

Huh, interesting.  I wonder how parallel.c's select_loop got to be like
that then?  The git history offers no clue: it is essentially the same as
HEAD as far back as the initial commit of parallel.c.  It certainly looks
like someone intended to introduce a cancel check and never did, or had
one and took it out without simplifying the rest of the logic.

Anyway, AFAICS the time limit on select() wait is completely useless in
the code as it stands; but we'll likely want to add a cancel check there,
so ripping it out wouldn't be a good plan.  I'll change the added comment.

regards, tom lane


-- 
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] Does people favor to have matrix data type?

[Kouhei Kaigai]

> On 5/25/16 7:46 AM, Kouhei Kaigai wrote:
> > My only concern is that domain type is not allowed to define type cast.
> > If we could add type cast on domain, we can define type transformation from
> > other array type to matrix.
> 
> I've actually wished for that in the past, as well as casting to
> compound types. Having those would make it easier to mock up a real data
> type for experimentation.
> 
> I strongly encourage you to talk to the MADlib community about
> first-class matrix support. They currently emulate matrices via arrays.
>
A MADLib folks contacted me in the past; when I initially made PG-Strom
several years before, however, no communication with them after that.

https://madlib.incubator.apache.org/docs/v1.8/group__grp__arraysmatrix.html
According to the documentation, their approach is different from what
I'd like to implement. They use a table as if a matrix. Because of its
data format, we have to adjust position of the data element to fit
requirement by high performance matrix libraries (like cuBLAS).

> I don't know offhand if they support NULLs in their regular matrices.
> They also support a sparsematrix "type" that is actually implemented as
> a table that contains coordinates and a value for each value in the
> matrix. Having that as a type might also be interesting (if you're
> sparse enough, that will be cheaper than the current NULL bitmap
> implementation).
>
Sparse matrix! It is a disadvantaged area for the current array format.

I have two ideas. HPC folks often split a large matrix into multiple
grid. A grid is typically up to 1024x1024 matrix, for example.
If a grid is consists of all zero elements, it is obvious we don't need
to have individual elements on the grid.
One other idea is compression. If most of matrix is zero, it is an ideal
data for compression, and it is easy to reconstruct only when calculation.

> Related to this, Tom has mentioned in the past that perhaps we should
> support abstract use of the [] construct. Currently point finds a way to
> make use of [], but I think that's actually coded into the grammar.
>
Yep, if we consider 2D-array is matrix, no special enhancement is needed
to use []. However, I'm inclined to have own data structure for matrix
to present the sparse matrix.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 

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

[HACKERS] Statement timeout

[Tatsuo Ishii]

When extended query protocol message is used, statement timeout is not
checked until a sync message is received (more precisely, statement
timeout timer is canceled while processing the sync message, and
actual checking timeout is done in CHECK_FOR_INTERRUPTS). Example:

parse(statement1)
bind(statement1, portal1)
execute(portal1)
parse(statement2)
bind(statement2, portal2)
execute(portal2)
sync

Suppose statement_timeout = 2s. If execute(portal1) takes 3 seconds,
and execute(portal2) takes 1 second, the statement timeout is reported
at the time when the sync message is processed which is right after
execute(portal2). This may lead to certain confusions to users:

1) If log_min_error_statement is ERROR or below, the cause of statement
   timeout is reported as statement2, rather than statement1.

2) If log_duration is set, the execution time for execute(portal1) is
   3 seconds, and execute(portal2) is 1 second which looks
   inconsistent with #1.

3) If the sync message arrives long time after execute(portal2) (say,
   3 seconds), statement timeout will raised even if execute(portal1)
   and execute(portal2) take less than 2 seconds.

Is there any room to enhance this? For example calling
disable_timeout(STATEMENT_TIMEOUT, false) in exec_execute_message.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


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