Re: [HACKERS] Reviewing freeze map code

2016-07-23 Thread Amit Kapila
On Mon, Jul 18, 2016 at 2:03 PM, Andres Freund wrote: > On 2016-07-18 10:02:52 +0530, Amit Kapila wrote: >> > >> >> Consider the below scenario. >> >> Vacuum >> a. acquires a cleanup lock for page - 10 >> b. busy in checking visibility of tuples

Re: [HACKERS] freeze map open item

2016-07-22 Thread Amit Kapila
being fixed. I recommend we open > a new open issue for that, or at least fix it. > +1. I think, sometime needs to be spend to confirm if that theoretical risk can practically cause any problem. I am planning to spend some time on it in next week or so and will report my

Re: [HACKERS] PoC: Make it possible to disallow WHERE-less UPDATE and DELETE

2016-07-21 Thread Amit Kapila
artificially add where clause which seems slightly inconvenient, but may be such cases are less. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] [BUG] pg_basebackup from disconnected standby fails

2016-07-20 Thread Amit Kapila
- https://www.postgresql.org/message-id/CAB7nPqTv5gmKQcNDoFGTGqoqXz2xLz4RRw247oqOJzZTVy6-7Q%40mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] [BUG] pg_basebackup from disconnected standby fails

2016-07-20 Thread Amit Kapila
On Wed, Jul 20, 2016 at 5:12 AM, Michael Paquier wrote: > On Tue, Jul 19, 2016 at 9:08 PM, Amit Kapila wrote: >> On Tue, Jul 19, 2016 at 10:43 AM, Michael Paquier >> wrote: >>> On Sat, Jul 16, 2016 at 9:20 PM, Amit Kapila >>> wrote: >>>> On

Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-19 Thread Amit Kapila
On Wed, Jul 20, 2016 at 7:57 AM, Andres Freund wrote: > > > On July 19, 2016 7:14:42 PM PDT, Amit Kapila wrote: >>On Wed, Jul 20, 2016 at 5:02 AM, Andres Freund >>wrote: >>> On 2016-07-19 18:09:59 -0500, Kevin Grittner wrote: >>>> As far as I can se

Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-19 Thread Amit Kapila
pTupleSatisfiesMVCC and >HeapTupleSatisfiesToast? > I also think so. However, it is not clear what is the best place to initialize toast snapshot. One idea could be to do it in GetSnapshotData() after capturing the required information for the valid value of old_snapshot_threshold.

Re: [HACKERS] [BUG] pg_basebackup from disconnected standby fails

2016-07-19 Thread Amit Kapila
On Tue, Jul 19, 2016 at 10:43 AM, Michael Paquier wrote: > On Sat, Jul 16, 2016 at 9:20 PM, Amit Kapila wrote: >> On Wed, Jul 13, 2016 at 8:56 AM, Michael Paquier >> wrote: >>> Another way that just popped into my mind is to add dedicated fields >>> to XLogCtl

Re: [HACKERS] Reviewing freeze map code

2016-07-17 Thread Amit Kapila
On Mon, Jul 18, 2016 at 9:13 AM, Andres Freund wrote: > On 2016-07-18 09:07:19 +0530, Amit Kapila wrote: >> + /* >> + * Before locking the buffer, pin the visibility map page if it may be >> + * necessary. >> + */ >> >> + if (PageIsAllVisible(BufferGe

Re: [HACKERS] Reviewing freeze map code

2016-07-17 Thread Amit Kapila
On Mon, Jul 18, 2016 at 8:18 AM, Andres Freund wrote: > On 2016-07-16 10:45:26 -0700, Andres Freund wrote: >> >> >> On July 16, 2016 8:49:06 AM PDT, Tom Lane wrote: >> >Amit Kapila writes: >> >> On Sat, Jul 16, 2016 at 7:02 AM, Andres Freund >> &

Re: [HACKERS] [BUG] pg_basebackup from disconnected standby fails

2016-07-16 Thread Amit Kapila
On Wed, Jul 13, 2016 at 8:56 AM, Michael Paquier wrote: > On Tue, Jul 12, 2016 at 12:22 PM, Amit Kapila wrote: >> I think updating minRecoveryPoint unconditionally can change it's >> purpose in some cases. Refer below comments in code: >> >> * minRecoveryPoint i

Re: [HACKERS] Reviewing freeze map code

2016-07-16 Thread Amit Kapila
; likely in the former case) to have been updated. > > I think we have two choices how to deal with that: First, we can add a > new flags variable to xl_heap_lock similar to > xl_heap_insert/update/... and bump page magic, > +1 for going in this way. This will keep us consistent with

Re: [HACKERS] Unable to test parallel aggregate/joins in Postgres beta 2

2016-07-14 Thread Amit Kapila
art from the mentioned ones on why pg > would not choose a parallel plan ? > You can try by setting parallel_setup_cost=0 and parallel_tuple_cost=0, though changing that way is not advisable. If that doesn't work for you, share the exact test for which you are expecting parallel plan t

Re: [HACKERS] Reviewing freeze map code

2016-07-14 Thread Amit Kapila
t; VISIBILITYMAP_ALL_FROZEN); >> + } >> + > > FWIW, I don't think it's worth introducing visibilitymap_clear_extended. > As this is a 9.6 only patch, i think it's better to change > visibilitymap_clear's API. > > Unless somebody prote

Re: [HACKERS] Hash Indexes

2016-07-14 Thread Amit Kapila
On Wed, Jun 22, 2016 at 8:48 PM, Robert Haas wrote: > On Wed, Jun 22, 2016 at 5:14 AM, Amit Kapila wrote: >> We can do it in the way as you are suggesting, but there is another thing >> which we need to consider here. As of now, the patch tries to finish the >> split

Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-13 Thread Amit Kapila
course of fixing this issue? > > I took some time trying to see what you have in mind, and I'm > really not "getting it". > Isn't it possible if we initialize lsn and whenTaken in SnapshotToast when old_snapshot_threshold > 0 and add a check for HeapTu

Re: [HACKERS] pg_basebackup wish list

2016-07-13 Thread Amit Kapila
ty is to enhance -P option as -P sec, such that it will display progress after ever 'sec' seconds. Something like we have for pgbench. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] Simplifying the interface of UpdateMinRecoveryPoint

2016-07-13 Thread Amit Kapila
then your patch does the right thing by removing it. Personally, I feel overloading the parameter for multiple purposes makes code less maintainable, so retaining as it is in HEAD has some merits. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mai

Re: [HACKERS] [BUG] pg_basebackup from disconnected standby fails

2016-07-11 Thread Amit Kapila
, when the database state is DB_IN_ARCHIVE_RECOVERY and updating minRecoveryPoint unconditionally doesn't look in sync with that as well. I think your and Kyotaro-san's point that minRecoveryPoint should be updated to support back-ups on standby has merits, but I think doing it unconditionally mig

Re: [HACKERS] [BUGS] BUG #14230: Wrong timeline returned by pg_stop_backup on a standby

2016-07-11 Thread Amit Kapila
ups is at the origin of this defect. > > > I agree this looks correct. > > But isn't this also a pre-existing bug in 9.5? Or did we change something > else that suddenly made it visible? > I think the bug is pre-existing, but it becomes visible to user now by new API. -

Re: [HACKERS] INSERT .. SET syntax

2016-07-09 Thread Amit Kapila
ocumentation part of the patch. I noticed that you have used transformUpdateTargetList() to generate expression list for this case, but that function raises some internal errors which indicates that error is from Update. I think that might be misleading to users, if they ever got raised.

Re: [HACKERS] [BUGS] BUG #14230: Wrong timeline returned by pg_stop_backup on a standby

2016-07-08 Thread Amit Kapila
cPtr() to get the timeline ID when in > recovery, but what we really want to know here is the timeline of the > last REDO pointer, which is starttli, and that's more consistent with > the fact that we use startpoint when writing the backup_label file. In > short, +1 for this

Re: [HACKERS] Reviewing freeze map code

2016-07-08 Thread Amit Kapila
isible(BufferGetPage(buffer)) && + VM_ALL_FROZEN(relation, block, &vmbuffer)) + { + visibilitymap_clear_extended(relation, block, vmbuffer, + VISIBILITYMAP_ALL_FROZEN); + } + + if (RelationNeedsWAL(relation)) + { .. + XLogBeginInsert(); + XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);

Re: [HACKERS] Showing parallel status in \df+

2016-07-08 Thread Amit Kapila
adding this piece of information? >>> >> >> Seems like a good idea to me. It's going to be useful in debugging > > Okay. Here we go. I named the column for the parallel information > "Parallelism". > Another option could be to name it as Parallel Mode.

Re: [HACKERS] copyParamList

2016-07-07 Thread Amit Kapila
gt;paramMask to make sure that even if it gets used in future, the code works as expected. Alternatively, one might think of adding an Assert there, but that doesn't seem to be future-proof. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com set-parammask-

Re: [HACKERS] EXPLAIN ANALYZE for parallel query doesn't report the SortMethod information.

2016-07-07 Thread Amit Kapila
On Thu, Jul 7, 2016 at 7:37 PM, Tom Lane wrote: > Amit Kapila writes: >> On Thu, Jul 7, 2016 at 1:23 PM, Fujii Masao wrote: >>> I found $SUBJECT while trying to test parallel queries. Is this a bug? > > Presumably the instrumentation data needed for that is not get

Re: [HACKERS] EXPLAIN ANALYZE for parallel query doesn't report the SortMethod information.

2016-07-07 Thread Amit Kapila
0 > loops=1) > I think this can never happen for force_parallel_mode TO off, because we don't generate a gather on top of sort node. The reason why we are able to push Sort below gather, because it is marked as parallel_safe (create_sort_path). I think we should not mark it as

Re: [HACKERS] Documentation fixes for pg_visibility

2016-07-06 Thread Amit Kapila
possibly use that here too, once we have it. > makes sense. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] Reviewing freeze map code

2016-07-06 Thread Amit Kapila
x10 >> +#define XLHL_ALL_VISIBLE_CLEARED 0x20 > > Hm. We can't easily do that in the back-patched version; because a > standby won't know to check for the flag . That's kinda ok, since we > don't yet need to clear all-visible yet at that point of > heap_upda

Re: [HACKERS] Hash Indexes

2016-07-06 Thread Amit Kapila
On Wed, Jun 22, 2016 at 8:44 PM, Robert Haas wrote: > On Wed, Jun 22, 2016 at 5:10 AM, Amit Kapila wrote: >>> > Insertion will happen by scanning the appropriate bucket and needs to >>> > retain pin on primary bucket to ensure that concurrent split doesn't >

Re: [HACKERS] to_date_valid()

2016-07-04 Thread Amit Kapila
the additional function calls make to_date much costlier than its current implementation? I don't know if there is a better way, but I think it is worth to consider, if we can find a cheaper way to detect validity of date. Note - Your patch is small (~13KB) enough that it doesn't need to

Re: [HACKERS] Re: GiST optimizing memmoves in gistplacetopage for fixed-size updates [PoC]

2016-07-04 Thread Amit Kapila
n change the organization of page which might lead to failure in replay of consecutive WAL records. + /*if we have just one tuple to update we replace it on-place on page*/ In comments, we always leave a space both in the beginning and at the end of a comment. See comments in nearby code. -- With R

Re: [HACKERS] Reviewing freeze map code

2016-07-04 Thread Amit Kapila
On Mon, Jul 4, 2016 at 2:31 PM, Masahiko Sawada wrote: > On Sat, Jul 2, 2016 at 12:17 PM, Amit Kapila wrote: >> On Sat, Jul 2, 2016 at 12:53 AM, Andres Freund wrote: >>> On 2016-07-01 15:18:39 -0400, Robert Haas wrote: >>> >>>> Should we just cl

Re: [HACKERS] Re: [ADMIN] problems using pg_start_backup/pg_stop_backup and pg_basebackup at same time

2016-07-01 Thread Amit Kapila
) to finish which is that corresponding .ready file should be generated. Can it be possible, that due to some reason the .ready file is not generated corresponding to stoppoint determined by pg_stop_backup()? Is it possible to verify if the required .ready files are present? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] fixing subplan/subquery confusion

2016-07-01 Thread Amit Kapila
a way to build an Append plan where some of the child + * plans are forced to run in the parent and others can run in any + * process, but for now there's no point in expending cycles building + * childrel paths we can't use.) + */ + if (!rel->consider_parallel) + childrel->consider_parallel = fals

Re: [HACKERS] Reviewing freeze map code

2016-07-01 Thread Amit Kapila
On Fri, Jul 1, 2016 at 7:52 PM, Masahiko Sawada wrote: > On Fri, Jul 1, 2016 at 11:12 AM, Amit Kapila wrote: >> >> Why do you think IndexOnlyScan will return wrong result? If the >> server crash in the way as you described, the transaction that has >> made m

Re: [HACKERS] Reviewing freeze map code

2016-07-01 Thread Amit Kapila
ck_tuple, which entirely > preserves all-visible (but shouldn't preserve all-frozen), ISTM we > better find something that doesn't invalidate all-visible. > Sounds logical, considering that we have a way to set all-frozen and vacuum does that as well. So probably either we need to ha

Re: [HACKERS] Documentation fixes for pg_visibility

2016-07-01 Thread Amit Kapila
On Fri, Jul 1, 2016 at 9:30 PM, Robert Haas wrote: > On Fri, Jul 1, 2016 at 10:09 AM, Amit Kapila wrote: >>>> Ah, I see. So your suggestion is to do this job in lazy_scan_heap() >>>> when scanning each block, and then to issue a WARNING and clear the >>>>

Re: [HACKERS] Documentation fixes for pg_visibility

2016-07-01 Thread Amit Kapila
n't > add more visibility map checks for pages where we already know the VM > bit can't possibly be set. > > Other opinions on the concept or the patch? > +1 on the idea. + PageClearAllVisible(page); + MarkBufferDirty(buf); What is the need to clear the Page level bit, if it is already cleared, doesn't '!all_frozen' indicate that? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] [sqlsmith] crashes in RestoreSnapshot on hot standby

2016-06-30 Thread Amit Kapila
On Fri, Jul 1, 2016 at 9:38 AM, Thomas Munro wrote: > On Fri, Jul 1, 2016 at 3:25 PM, Amit Kapila wrote: >> On Fri, Jul 1, 2016 at 8:48 AM, Thomas Munro >> wrote: >>> If serialized_snapshot->xcnt == 0, then snapshot->xip never gets >>> in

Re: [HACKERS] [sqlsmith] crashes in RestoreSnapshot on hot standby

2016-06-30 Thread Amit Kapila
ode that way will be more consistent. In case of recovery, I think serialized_snapshot->xcnt will always be zero as we fill everything in subxip array (refer below code in GetSnapshotData). GetSnapshotData() { /* * We're in hot standby, so get XIDs from KnownAssignedXids. .. .. } -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Reviewing freeze map code

2016-06-30 Thread Amit Kapila
On Thu, Jun 30, 2016 at 8:10 PM, Masahiko Sawada wrote: > On Thu, Jun 30, 2016 at 3:12 PM, Amit Kapila wrote: >> On Thu, Jun 30, 2016 at 9:13 AM, Andres Freund wrote: >>> On 2016-06-30 08:59:16 +0530, Amit Kapila wrote: >>>> On Wed, Jun 29, 2016 at 10:30 PM, Andres

Re: [HACKERS] fixing subplan/subquery confusion

2016-06-30 Thread Amit Kapila
On Tue, Jun 28, 2016 at 5:22 PM, Amit Kapila wrote: > On Tue, Jun 28, 2016 at 8:25 AM, Tom Lane wrote: > >> In the appendrel case, I tend to agree that the easiest solution is to >> scan all the children of the appendrel and just mark the whole thing as >> not consider

Re: [HACKERS] Reviewing freeze map code

2016-06-29 Thread Amit Kapila
On Thu, Jun 30, 2016 at 9:13 AM, Andres Freund wrote: > On 2016-06-30 08:59:16 +0530, Amit Kapila wrote: >> On Wed, Jun 29, 2016 at 10:30 PM, Andres Freund wrote: >> > On 2016-06-29 19:04:31 +0530, Amit Kapila wrote: >> >> There is nothing in this record which

Re: [HACKERS] Reviewing freeze map code

2016-06-29 Thread Amit Kapila
On Wed, Jun 29, 2016 at 10:30 PM, Andres Freund wrote: > On 2016-06-29 19:04:31 +0530, Amit Kapila wrote: >> There is nothing in this record which recorded the information about >> visibility clear flag. > > I think we can actually defer the clearing to the lock release?

Re: [HACKERS] Rename max_parallel_degree?

2016-06-29 Thread Amit Kapila
On Wed, Jun 29, 2016 at 11:54 AM, Julien Rouhaud wrote: > On 29/06/2016 06:29, Amit Kapila wrote: >> On Wed, Jun 29, 2016 at 2:57 AM, Julien Rouhaud >> wrote: >>> >>> Thanks a lot for the help! >>> >>> PFA v6 which should fix all the

Re: [HACKERS] Reviewing freeze map code

2016-06-29 Thread Amit Kapila
On Wed, Jun 29, 2016 at 11:14 AM, Masahiko Sawada wrote: > On Fri, Jun 24, 2016 at 11:04 AM, Amit Kapila wrote: >> On Fri, Jun 24, 2016 at 4:33 AM, Andres Freund wrote: >>> On 2016-06-23 18:59:57 -0400, Alvaro Herrera wrote: >>>> Andres Freund wrote: >

Re: [HACKERS] Rename max_parallel_degree?

2016-06-28 Thread Amit Kapila
n I am not sure if it is good idea, because it can cause some confusion to the user about usage of both the parameters together. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes t

Re: [HACKERS] Rename max_parallel_degree?

2016-06-28 Thread Amit Kapila
ding the description of max_parallel_worker, user can set its value more than max_wroker_processes which we don't want. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] fixing subplan/subquery confusion

2016-06-28 Thread Amit Kapila
On Tue, Jun 28, 2016 at 8:25 AM, Tom Lane wrote: > Amit Kapila writes: >> I had couple of questions [1] related to that patch. See if you find >> those as relevant? > > I do not think those cases are directly relevant: you're talking about > appendrels not single,

Re: [HACKERS] Rename max_parallel_degree?

2016-06-27 Thread Amit Kapila
On Mon, Jun 27, 2016 at 10:35 PM, Julien Rouhaud wrote: > On 27/06/2016 07:18, Amit Kapila wrote: >> On Mon, Jun 27, 2016 at 10:21 AM, Amit Kapila >> wrote: >>> >>> I think as the parallel_terminate_count is only modified by postmaster >>> and read by

Re: [HACKERS] fixing subplan/subquery confusion

2016-06-27 Thread Amit Kapila
great need to commit it myself, if that's what you >> meant. > > OK, I'll set aside some time for further review and either commit it > or send an update by COB Thursday US time. > I had couple of questions [1] related to that patch. See if you find those as relevant?

Re: [HACKERS] Rename max_parallel_degree?

2016-06-26 Thread Amit Kapila
On Mon, Jun 27, 2016 at 10:21 AM, Amit Kapila wrote: > On Sun, Jun 26, 2016 at 3:57 PM, Julien Rouhaud > wrote: >> On 26/06/2016 08:37, Amit Kapila wrote: >>> >>> @@ -370,6 +379,8 @@ ForgetBackgroundWorker(slist_mutable_iter *cur) >>> Assert(rw-&g

Re: [HACKERS] Rename max_parallel_degree?

2016-06-26 Thread Amit Kapila
On Sun, Jun 26, 2016 at 3:57 PM, Julien Rouhaud wrote: > On 26/06/2016 08:37, Amit Kapila wrote: >> >> @@ -370,6 +379,8 @@ ForgetBackgroundWorker(slist_mutable_iter *cur) >> Assert(rw->rw_shmem_slot < >> max_worker_processes); >> slot = &B

Re: [HACKERS] Rename max_parallel_degree?

2016-06-25 Thread Amit Kapila
On Sat, Jun 25, 2016 at 2:27 PM, Julien Rouhaud wrote: > On 25/06/2016 09:33, Amit Kapila wrote: >> On Wed, Jun 15, 2016 at 11:43 PM, Julien Rouhaud >> wrote: >>> >>> Attached v4 implements the design you suggested, I hope everything's ok. >>> &

Re: [HACKERS] Rename max_parallel_degree?

2016-06-25 Thread Amit Kapila
above in documentation to indicate that "pool of processes established by guc-max-parallel-workers". -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] Hash Indexes

2016-06-24 Thread Amit Kapila
On Fri, Jun 24, 2016 at 2:38 PM, Mithun Cy wrote: > On Thu, Jun 16, 2016 at 12:58 PM, Amit Kapila > wrote: > > I have a question regarding code changes in _hash_first. > > +/* > + * Conditionally get the lock on primary bucket page for search > while >

Re: [HACKERS] Hash Indexes

2016-06-23 Thread Amit Kapila
On Thu, Jun 23, 2016 at 10:56 PM, Robert Haas wrote: > On Wed, Jun 22, 2016 at 10:13 PM, Amit Kapila wrote: >>> A scan that has seen the flag won't look at the >>> tuple in any case. >> >> Why so? Assume that scan started on new bucket where >> spl

Re: [HACKERS] Reviewing freeze map code

2016-06-23 Thread Amit Kapila
when the >> new tuple goes to another page, though. > > I think it has to happen in both cases unfortunately. We could try to > add some optimizations (e.g. only release lock & WAL log if the target > page, via fsm, is before the current one), but I don't really want t

Re: [HACKERS] Hash Indexes

2016-06-22 Thread Amit Kapila
On Wed, Jun 22, 2016 at 8:48 PM, Robert Haas wrote: > On Wed, Jun 22, 2016 at 5:14 AM, Amit Kapila wrote: >> We can do it in the way as you are suggesting, but there is another thing >> which we need to consider here. As of now, the patch tries to finish the >> split

Re: [HACKERS] Hash Indexes

2016-06-22 Thread Amit Kapila
On Wed, Jun 22, 2016 at 8:44 PM, Robert Haas wrote: > On Wed, Jun 22, 2016 at 5:10 AM, Amit Kapila wrote: >>> >>> I think this is basically correct, although I don't find it to be as >>> clear as I think it could be. It seems very clear that any operation >

Re: [HACKERS] Hash Indexes

2016-06-22 Thread Amit Kapila
On Tue, Jun 21, 2016 at 9:33 PM, Robert Haas wrote: > > On Thu, Jun 16, 2016 at 3:28 AM, Amit Kapila wrote: > >> Incomplete splits can be completed either by vacuum or insert as both > >> needs exclusive lock on bucket. If vacuum finds split-in-progress flag on a

Re: [HACKERS] Hash Indexes

2016-06-22 Thread Amit Kapila
On Tue, Jun 21, 2016 at 9:26 PM, Robert Haas wrote: > > On Tue, May 10, 2016 at 8:09 AM, Amit Kapila wrote: > > > Once the split operation has set the split-in-progress flag, it will begin scanning bucket (N+1)/2. Every time it finds a tuple that properly belongs in bucket N+1,

Re: [HACKERS] Reviewing freeze map code

2016-06-20 Thread Amit Kapila
On Tue, Jun 21, 2016 at 9:21 AM, Andres Freund wrote: > > On 2016-06-21 08:59:13 +0530, Amit Kapila wrote: > > Can we consider to use some strategy to avoid deadlocks without releasing > > the lock on old page? Consider if we could have a mechanism such that > > Relation

Re: [HACKERS] Reviewing freeze map code

2016-06-20 Thread Amit Kapila
On Tue, Jun 21, 2016 at 9:16 AM, Thomas Munro wrote: > > On Tue, Jun 21, 2016 at 3:29 PM, Amit Kapila wrote: > > Some others ways could be: > > > > Before releasing the lock on buffer containing old tuple, clear the VM and > > visibility info from page and WAL log

Re: [HACKERS] Reviewing freeze map code

2016-06-20 Thread Amit Kapila
On Tue, Jun 21, 2016 at 9:08 AM, Thomas Munro wrote: > > On Tue, Jun 21, 2016 at 3:29 PM, Amit Kapila wrote: > > On Tue, Jun 21, 2016 at 1:03 AM, Andres Freund wrote: > >> Well, I think generally nobody seriously looked at actually refactoring > >> heap_update()

Re: [HACKERS] parallel.c is not marked as test covered

2016-06-20 Thread Amit Kapila
w what this mode should be called, I'm pretty sure > > that neither "single_copy" nor "pipeline" are useful descriptions. > > Maybe we should make this an enum rather than a boolean, since there > seem to be more than two useful behaviors. > How about calling it as control_parallel/control_parallelism or override_parallel/override_parallelism? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Reviewing freeze map code

2016-06-20 Thread Amit Kapila
4c46133f81e188653a127f939eb33c4a ) and set the same in old tuple header before releasing lock on buffer and teach tqual.c to honor the flag. I think tqual.c should consider HEAP_XMAX_UNLOGGED as an indication of aborted transaction unless it is currently in-progress. Also, I think we need to clear this flag before WAL logging in heap_update. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] parallel.c is not marked as test covered

2016-06-20 Thread Amit Kapila
On Thu, Jun 16, 2016 at 8:20 AM, Robert Haas wrote: > > On Wed, Jun 15, 2016 at 10:48 PM, Amit Kapila wrote: > > exec_stmt_execsql() is used to execute SQL statements insider plpgsql which > > includes dml statements as well, so probably you wanted to play safe by not >

Re: [HACKERS] Actuall row count of Parallel Seq Scan in EXPLAIN ANALYZE .

2016-06-20 Thread Amit Kapila
On Mon, Jun 20, 2016 at 12:47 PM, Amit Langote < langote_amit...@lab.ntt.co.jp> wrote: > > On 2016/06/20 15:42, Amit Kapila wrote: > > > >> > >> After spent time to investigate this behaviour, ISTM that the problem > >> is nloops of Parallel Seq Scan.

Re: [HACKERS] Actuall row count of Parallel Seq Scan in EXPLAIN ANALYZE .

2016-06-20 Thread Amit Kapila
xplain Analyze section. [1] - https://www.postgresql.org/docs/9.6/static/using-explain.html With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Actuall row count of Parallel Seq Scan in EXPLAIN ANALYZE .

2016-06-19 Thread Amit Kapila
ted to 3. > nloops here indicates, that it is done for 2 workers and a master backend. > So its "actual rows" is calculated 333(1000 / 3) at explain.c:L1223. > Thats how it should be considered. You might want to compare the behaviour with other cases where value of nloops is used. > Is it a bug? > I don't think so. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] parallel.c is not marked as test covered

2016-06-18 Thread Amit Kapila
ther the > code executes in the leader or a worker? > Before doing that test, we set force_parallel_mode=1, so it should always execute in worker which will ensure a stable output. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-17 Thread Amit Kapila
On Fri, Jun 17, 2016 at 8:18 AM, Amit Kapila wrote: > > On Fri, Jun 17, 2016 at 3:20 AM, Robert Haas wrote: > > > > > Something like what you have there might work if you use > > create_projection_path instead of apply_projection_to_path. > > >

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-16 Thread Amit Kapila
On Fri, Jun 17, 2016 at 11:39 AM, Andreas Joseph Krogh wrote: > På torsdag 16. juni 2016 kl. 20:19:44, skrev Tom Lane : > > Amit Kapila writes: > > On Mon, Jun 13, 2016 at 10:36 PM, Tom Lane wrote: > >> min_parallel_relation_size, or min_parallelizable_relation_size

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-16 Thread Amit Kapila
On Fri, Jun 17, 2016 at 3:20 AM, Robert Haas wrote: > > On Thu, Jun 16, 2016 at 12:16 PM, Robert Haas wrote: > > On Thu, Jun 16, 2016 at 8:00 AM, Amit Kapila wrote: > >>> 1. The case originally reported by Thomas Munro still fails. To fix > >>> that, we pr

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-16 Thread Amit Kapila
On Thu, Jun 16, 2016 at 11:49 PM, Tom Lane wrote: > > Amit Kapila writes: > > On Mon, Jun 13, 2016 at 10:36 PM, Tom Lane wrote: > >> min_parallel_relation_size, or min_parallelizable_relation_size, or > >> something like that? > > > You are right that

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-16 Thread Amit Kapila
On Thu, Jun 16, 2016 at 9:26 AM, Robert Haas wrote: > > On Tue, Jun 14, 2016 at 12:18 PM, Tom Lane wrote: > > Amit Kapila writes: > >> On Mon, Jun 13, 2016 at 9:37 PM, Tom Lane wrote: > >>> ... I got a core dump in the window.sql test: > >>> whic

Re: [HACKERS] Hash Indexes

2016-06-16 Thread Amit Kapila
On Tue, May 10, 2016 at 5:39 PM, Amit Kapila wrote: > > > Incomplete Splits > -- > Incomplete splits can be completed either by vacuum or insert as both > needs exclusive lock on bucket. If vacuum finds split-in-progress flag on > a bucket then it wi

Re: [HACKERS] parallel.c is not marked as test covered

2016-06-15 Thread Amit Kapila
On Thu, Jun 16, 2016 at 12:46 AM, Robert Haas wrote: > > On Wed, Jun 15, 2016 at 5:23 AM, Amit Kapila wrote: > >> > Considering above analysis is correct, we have below options: > >> > a. Modify the test such that it actually generates an error and to hide >

Re: [HACKERS] Reviewing freeze map code

2016-06-15 Thread Amit Kapila
On Wed, Jun 15, 2016 at 7:13 PM, Robert Haas wrote: > > On Wed, Jun 15, 2016 at 9:43 AM, Amit Kapila wrote: > > On Wed, Jun 15, 2016 at 6:26 PM, Robert Haas wrote: > >> On Wed, Jun 15, 2016 at 2:41 AM, Thomas Munro > >> wrote: > >> > I spent some tim

Re: [HACKERS] Reviewing freeze map code

2016-06-15 Thread Amit Kapila
n the heap page to go do some more work that > might even ERROR out. > Can't we clear the all-visible flag before releasing the lock? We can use logic of already_marked as it is currently used in code to clear it just once. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-15 Thread Amit Kapila
On Tue, Jun 14, 2016 at 4:48 PM, Amit Kapila wrote: > On Mon, Jun 13, 2016 at 8:11 PM, Tom Lane wrote: > > > I do > > not share your confidence that using apply_projection_to_path within > > create_grouping_paths is free of such a hazard. > > > > Fair enoug

Re: [HACKERS] parallel.c is not marked as test covered

2016-06-15 Thread Amit Kapila
On Wed, Jun 15, 2016 at 12:07 PM, Noah Misch wrote: > > On Wed, Jun 15, 2016 at 11:50:33AM +0530, Amit Kapila wrote: > > In short, this test doesn't serve it's purpose which is to generate an > > error from worker. > > That's bad. Thanks for fig

Re: [HACKERS] parallel.c is not marked as test covered

2016-06-15 Thread Amit Kapila
On Wed, Jun 15, 2016 at 12:07 PM, Noah Misch wrote: > > On Wed, Jun 15, 2016 at 11:50:33AM +0530, Amit Kapila wrote: > > do $$begin > > Perform stringu1::int2 from tenk1 where unique1 = 1; > > end$$; > > > > ERROR: invalid input syntax for integer: "B

Re: [HACKERS] parallel.c is not marked as test covered

2016-06-14 Thread Amit Kapila
actually generates an error and to hide the context, we can use force_parallel_mode = regress; c. Remove this test and then think of a better way to exercise error path in worker. Thoughts? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-14 Thread Amit Kapila
th guc.c. I am not sure if it is advisable to use PG_INT32_MAX in guc.c as other similar parameters use INT_MAX. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com min_parallel_relation_size_v1.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] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-14 Thread Amit Kapila
ly, I am wondering about the same problem as we discussed here [1]. [1] - https://www.postgresql.org/message-id/CAA4eK1Jz5tG2D2PCNYqYob2cb4dKowKYwVJ7OmP5vwyRyCMx4g%40mail.gmail.com With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-14 Thread Amit Kapila
On Mon, Jun 13, 2016 at 8:11 PM, Tom Lane wrote: > > Amit Kapila writes: > > On Mon, Jun 13, 2016 at 7:51 PM, Tom Lane wrote: > >> I think the real question here is why the code removed by 04ae11f62 > >> was wrong. It was unsafe to use apply_projection_to_p

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-13 Thread Amit Kapila
On Mon, Jun 13, 2016 at 9:37 PM, Tom Lane wrote: > > I wrote: > > Amit Kapila writes: > >> It is slightly tricky to write a reproducible parallel-query test, but > >> point taken and I think we should try to have a test unless such a test is > >> r

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-13 Thread Amit Kapila
On Mon, Jun 13, 2016 at 7:51 PM, Tom Lane wrote: > > Robert Haas writes: > > On Mon, Jun 13, 2016 at 3:18 AM, Amit Kapila wrote: > >> In create_grouping_paths(), we are building partial_grouping_path and same > >> is used for gather path and other gro

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-13 Thread Amit Kapila
On Mon, Jun 13, 2016 at 7:17 PM, Robert Haas wrote: > > On Mon, Jun 13, 2016 at 3:18 AM, Amit Kapila wrote: > > In create_grouping_paths(), we are building partial_grouping_path and same > > is used for gather path and other grouping paths (for partial paths). > > Howe

Re: [HACKERS] ERROR: ORDER/GROUP BY expression not found in targetlist

2016-06-13 Thread Amit Kapila
grouping paths (for partial paths). However, we don't use it for partial path list and sort path due to which path target for Sort path is different from what we have expected. Is there a problem in applying partial_grouping_path for partial pathlist? Attached patch just does that and I don

Re: [HACKERS] Reviewing freeze map code

2016-06-12 Thread Amit Kapila
till > be marked all-frozen, which is bad. > To clarify, are you talking about a case where insertion has aborted? Won't in such a case all_visible flag be set to false due to return value from HeapTupleSatisfiesVacuum() and if so, later code shouldn't mark it as all_frozen? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] [COMMITTERS] pgsql: Don't generate parallel paths for rels with parallel-restricted

2016-06-11 Thread Amit Kapila
On Sat, Jun 11, 2016 at 2:07 PM, Andreas Seltenreich wrote: > > Amit Kapila writes: > > > I have moved it to CLOSE_WAIT state because we have derived our > > queries to reproduce the problem based on original report[1]. If next > > run of sqlsmith doesn't show any

Re: [HACKERS] [COMMITTERS] pgsql: Don't generate parallel paths for rels with parallel-restricted

2016-06-10 Thread Amit Kapila
On Fri, Jun 10, 2016 at 8:20 PM, Tom Lane wrote: > > Amit Kapila writes: > > On Thu, Jun 9, 2016 at 11:38 PM, Tom Lane wrote: > >> Robert Haas writes: > >>> Could you answer my question about whether adjust_appendrel_attrs() > >>> might transla

Re: [HACKERS] Reviewing freeze map code

2016-06-10 Thread Amit Kapila
On Fri, Jun 10, 2016 at 12:09 PM, Andres Freund wrote: > > On 2016-06-10 11:58:26 +0530, Amit Kapila wrote: > > > > While looking at code in this area, I observed that during replay of > > records (heap_xlog_delete), we first clear the vm, then update the page. > >

Re: [HACKERS] [COMMITTERS] pgsql: Don't generate parallel paths for rels with parallel-restricted

2016-06-10 Thread Amit Kapila
N ALL it seems parent rels rtekind is RTE_SUBQUERY and we consider such rels as parallel unsafe (see set_rel_consider_parallel()). So it doesn't matter even if child rels target list contains any parallel unsafe expression, as we are not going to create parallel paths for such relations. Wit

Re: [HACKERS] Reviewing freeze map code

2016-06-09 Thread Amit Kapila
_scan_heap(), then I think we should also consider applying snapshot old threshold limit to oldestxmin. We currently do that in vacuum_set_xid_limits() for Vacuum. Is there a reason for not considering it for visibility check function? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Reviewing freeze map code

2016-06-09 Thread Amit Kapila
On Fri, Jun 10, 2016 at 8:27 AM, Andres Freund wrote: > > > On June 9, 2016 7:46:06 PM PDT, Amit Kapila > wrote: > >On Fri, Jun 10, 2016 at 8:08 AM, Andres Freund > >wrote: > > > >> On 2016-06-09 19:33:52 -0700, Andres Freund wrote: > >

<    6   7   8   9   10   11   12   13   14   15   >