On Thu, Aug 4, 2016 at 3:24 AM, Andres Freund wrote:
> Hi,
>
> On 2016-08-02 10:55:18 -0400, Noah Misch wrote:
>> [Action required within 72 hours. This is a generic notification.]
>>
>> The above-described topic is currently a PostgreSQL 9.6 open item. Andres,
>> since you committed the patch b
Hi,
On 2016-08-02 10:55:18 -0400, Noah Misch wrote:
> [Action required within 72 hours. 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.
Well ki
On Tue, Aug 02, 2016 at 02:10:29PM +0530, Amit Kapila wrote:
> On Tue, Aug 2, 2016 at 11:19 AM, Noah Misch wrote:
> > On Sat, Jul 23, 2016 at 01:25:55PM +0530, Amit Kapila wrote:
> >> On Mon, Jul 18, 2016 at 2:03 PM, Andres Freund wrote:
> >> > On 2016-07-18 10:02:52 +0530, Amit Kapila wrote:
> >
On Tue, Aug 2, 2016 at 11:19 AM, Noah Misch wrote:
> On Sat, Jul 23, 2016 at 01:25:55PM +0530, Amit Kapila wrote:
>> 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
On Sat, Jul 23, 2016 at 01:25:55PM +0530, Amit Kapila wrote:
> 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 visibi
On Wed, Jul 27, 2016 at 3:24 AM, Robert Haas wrote:
> On Sat, Jul 23, 2016 at 3:55 AM, Amit Kapila wrote:
>> Attached patch fixes the problem for me. Note, I have not tried to
>> reproduce the problem for heap_lock_updated_tuple_rec(), but I think
>> if you are convinced with above cases, then w
On Sat, Jul 23, 2016 at 3:55 AM, Amit Kapila wrote:
> Attached patch fixes the problem for me. Note, I have not tried to
> reproduce the problem for heap_lock_updated_tuple_rec(), but I think
> if you are convinced with above cases, then we should have a similar
> check in it as well.
I don't th
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
>> --assume, here it takes some time and in the mean
On Sat, Jul 16, 2016 at 10:08 AM, Andres Freund wrote:
> On 2016-07-14 20:53:07 -0700, Andres Freund wrote:
>> On 2016-07-13 23:06:07 -0700, Andres Freund wrote:
>> > won't enter the branch, because HEAP_XMAX_LOCK_ONLY won't be set. Which
>> > will leave t_ctid and HEAP_HOT_UPDATED set differentl
On 2016-07-18 01:33:10 -0700, Andres Freund wrote:
> On 2016-07-18 10:02:52 +0530, Amit Kapila wrote:
> > 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
On 2016-07-18 10:02:52 +0530, Amit Kapila wrote:
> 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 (PageIsA
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(BufferGetPage(*buffer)))
>> + visibilitymap_pin(relatio
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(BufferGetPage(*buffer)))
> + visibilitymap_pin(relation, block, &vmbuffer);
> +
> LockBuffer(*buffer, BUFFER_LOCK_E
On 2016-07-17 23:34:01 -0400, Robert Haas wrote:
> Thanks very much for working on this. Random suggestions after a quick look:
>
> + * Before locking the buffer, pin the visibility map page if it may be
> + * necessary.
>
> s/necessary/needed/
>
> More substantively, what happens if th
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
>> >wrote:
>> >>> I think we have two choices how to
On Sun, Jul 17, 2016 at 10:48 PM, Andres Freund wrote:
> Took till today. Attached is a rather heavily revised version of
> Sawada-san's patch. Most notably the recovery routines take care to
> reset the vm in all cases, we don't perform visibilitymap_get_status
> from inside a critical section an
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
> >wrote:
> >>> I think we have two choices how to deal with that: First, we can add
> >a
> >>> new flags variabl
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
>wrote:
>>> 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 p
Amit Kapila writes:
> On Sat, Jul 16, 2016 at 7:02 AM, Andres Freund wrote:
>> 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
On Sat, Jul 16, 2016 at 7:02 AM, Andres Freund wrote:
> On 2016-07-13 23:06:07 -0700, Andres Freund wrote:
>> > + /* Clear only the all-frozen bit on visibility map if needed */
>> > + if (PageIsAllVisible(BufferGetPage(buffer)) &&
>> > + VM_ALL_FROZEN(relatio
On 2016-07-13 23:06:07 -0700, Andres Freund wrote:
> > + /* Clear only the all-frozen bit on visibility map if needed */
> > + if (PageIsAllVisible(BufferGetPage(buffer)) &&
> > + VM_ALL_FROZEN(relation, block, &vmbuffer))
> > + {
> > +
On 2016-07-14 20:53:07 -0700, Andres Freund wrote:
> On 2016-07-13 23:06:07 -0700, Andres Freund wrote:
> > won't enter the branch, because HEAP_XMAX_LOCK_ONLY won't be set. Which
> > will leave t_ctid and HEAP_HOT_UPDATED set differently on the master and
> > standby / after crash recovery. I'm
On 2016-07-13 23:06:07 -0700, Andres Freund wrote:
> won't enter the branch, because HEAP_XMAX_LOCK_ONLY won't be set. Which
> will leave t_ctid and HEAP_HOT_UPDATED set differently on the master and
> standby / after crash recovery. I'm failing to see any harmful
> consequences right now, but d
On 2016-07-14 18:12:42 +0530, Amit Kapila wrote:
> Just thinking out loud. If we set HEAP_XMAX_LOCK_ONLY during update,
> then won't it impact the return value of
> HeapTupleHeaderIsOnlyLocked(). It will start returning true whereas
> otherwise I think it would have returned false due to in_progr
On Thu, Jul 14, 2016 at 11:36 AM, Andres Freund wrote:
> Hi,
>
> Master does
> /* temporarily make it look not-updated */
> oldtup.t_data->t_ctid = oldtup.t_self;
> here, and as is the wal record won't reflect that, because:
> static void
> heap_xlog_lock(XLogReader
Hi,
So I'm generally happy with 0001, baring some relatively minor
adjustments. I am however wondering about one thing:
On 2016-07-11 23:51:05 +0900, Masahiko Sawada wrote:
> diff --git a/src/backend/access/heap/heapam.c
> b/src/backend/access/heap/heapam.c
> index 57da57a..e7cb8ca 100644
> ---
On Fri, Jul 8, 2016 at 10:24 PM, Amit Kapila wrote:
> On Thu, Jul 7, 2016 at 12:34 PM, Masahiko Sawada
> wrote:
>> Than you for reviewing!
>>
>> On Thu, Jul 7, 2016 at 7:06 AM, Andres Freund wrote:
>>> On 2016-07-05 23:37:59 +0900, Masahiko Sawada wrote:
diff --git a/src/backend/access/hea
On Thu, Jul 7, 2016 at 12:34 PM, Masahiko Sawada wrote:
> Than you for reviewing!
>
> On Thu, Jul 7, 2016 at 7:06 AM, Andres Freund wrote:
>> On 2016-07-05 23:37:59 +0900, Masahiko Sawada wrote:
>>> diff --git a/src/backend/access/heap/heapam.c
>>> b/src/backend/access/heap/heapam.c
>>> index 57
On Thu, Jul 7, 2016 at 2:04 PM, Andres Freund wrote:
>> Yeah, that's true, but I'm having a bit of trouble imagining exactly
>> we end up with corruption that actually matters. I guess a torn page
>> could do it.
>
> I think Noah pointed out a bad scenario: If we crash after putting the
> xid in
On 2016-07-07 14:01:05 -0400, Robert Haas wrote:
> On Thu, Jul 7, 2016 at 1:58 PM, Andres Freund wrote:
> > On 2016-07-07 10:37:15 -0400, Robert Haas wrote:
> >> On Wed, Jul 6, 2016 at 6:06 PM, Andres Freund wrote:
> >> > Hm. We can't easily do that in the back-patched version; because a
> >> > s
On Thu, Jul 7, 2016 at 1:58 PM, Andres Freund wrote:
> On 2016-07-07 10:37:15 -0400, Robert Haas wrote:
>> On Wed, Jul 6, 2016 at 6:06 PM, Andres Freund wrote:
>> > 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,
On 2016-07-07 10:37:15 -0400, Robert Haas wrote:
> On Wed, Jul 6, 2016 at 6:06 PM, Andres Freund wrote:
> > 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 tha
On Thu, Jul 7, 2016 at 10:53 AM, Alvaro Herrera
wrote:
> Robert Haas wrote:
>> On Wed, Jul 6, 2016 at 6:06 PM, Andres Freund wrote:
>> > 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
Robert Haas wrote:
> On Wed, Jul 6, 2016 at 6:06 PM, Andres Freund wrote:
> > 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_update. Bu
On Wed, Jul 6, 2016 at 6:06 PM, Andres Freund wrote:
> 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_update. But that better means we don't
Than you for reviewing!
On Thu, Jul 7, 2016 at 7:06 AM, Andres Freund wrote:
> On 2016-07-05 23:37:59 +0900, Masahiko Sawada wrote:
>> diff --git a/src/backend/access/heap/heapam.c
>> b/src/backend/access/heap/heapam.c
>> index 57da57a..fd66527 100644
>> --- a/src/backend/access/heap/heapam.c
>>
On Thu, Jul 7, 2016 at 3:36 AM, Andres Freund wrote:
> On 2016-07-05 23:37:59 +0900, Masahiko Sawada wrote:
>
>> @@ -8694,6 +8761,23 @@ heap_xlog_lock(XLogReaderState *record)
>> }
>> HeapTupleHeaderSetXmax(htup, xlrec->locking_xid);
>> HeapTupleHeaderSetC
On 2016-07-05 23:37:59 +0900, Masahiko Sawada wrote:
> diff --git a/src/backend/access/heap/heapam.c
> b/src/backend/access/heap/heapam.c
> index 57da57a..fd66527 100644
> --- a/src/backend/access/heap/heapam.c
> +++ b/src/backend/access/heap/heapam.c
> @@ -3923,6 +3923,17 @@ l2:
>
> if (n
On Mon, Jul 4, 2016 at 5:44 PM, Masahiko Sawada wrote:
> On Sat, Jul 2, 2016 at 12:34 PM, Amit Kapila wrote:
>> 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 resu
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 clear all-visible and call it good enough?
>>>
>>> Gi
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:
>>> On Fri, Jul 1, 2016 at 10:22 AM, Masahiko Sawada
>>> wrote:
>>> > Ah, you're right, I misunderstood.
>>> >
>>> > Attached updated p
On Sat, Jul 2, 2016 at 12:34 PM, Amit Kapila wrote:
> 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 transacti
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 modifications will anyway be considered
On Sat, Jul 2, 2016 at 12:53 AM, Andres Freund wrote:
> On 2016-07-01 15:18:39 -0400, Robert Haas wrote:
>> On Fri, Jul 1, 2016 at 10:22 AM, Masahiko Sawada
>> wrote:
>> > Ah, you're right, I misunderstood.
>> >
>> > Attached updated patch incorporating your comments.
>> > I've changed it so tha
On 7/1/16 3:43 PM, Andres Freund wrote:
On 2016-07-01 15:42:22 -0500, Jim Nasby wrote:
On 7/1/16 2:23 PM, Andres Freund wrote:
The only
cost of that is that vacuum will come along and mark the page
all-visible again instead of skipping it, but that's probably not an
enormous expense in most cas
On 2016-07-01 15:42:22 -0500, Jim Nasby wrote:
> On 7/1/16 2:23 PM, Andres Freund wrote:
> > > > The only
> > > > cost of that is that vacuum will come along and mark the page
> > > > all-visible again instead of skipping it, but that's probably not an
> > > > enormous expense in most cases.
> > I
On 7/1/16 2:23 PM, Andres Freund wrote:
> The only
> cost of that is that vacuum will come along and mark the page
> all-visible again instead of skipping it, but that's probably not an
> enormous expense in most cases.
I think the main cost is not having the page marked as all-visible for
index
On 2016-07-01 15:18:39 -0400, Robert Haas wrote:
> On Fri, Jul 1, 2016 at 10:22 AM, Masahiko Sawada
> wrote:
> > Ah, you're right, I misunderstood.
> >
> > Attached updated patch incorporating your comments.
> > I've changed it so that heap_xlog_lock clears vm flags if page is
> > marked all froz
On Fri, Jul 1, 2016 at 10:22 AM, Masahiko Sawada wrote:
> Ah, you're right, I misunderstood.
>
> Attached updated patch incorporating your comments.
> I've changed it so that heap_xlog_lock clears vm flags if page is
> marked all frozen.
I believe that this should be separated into two patches, s
On Fri, Jul 1, 2016 at 11:12 AM, Amit Kapila wrote:
> 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, J
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 Freund wrote:
> On 2016-
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 Freund wrote:
>>> > On 2016-06-29 19:04:31 +0530, Amit Kapila wrote:
>>> >> There is nothin
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 recorded the information about
>> >>
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 recorded the information about
> >> visibility clear flag.
> >
> > I think we can actually defer
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?
How about the case if aft
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? A tuple
being locked doesn't require the vm being cleared.
> I think in this approach,
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:
> I'm looking into three approaches
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:
>>>
>>> > I'm looking into three approaches right now:
>>> >
>>> > 3) Use WAL logging for the already_marked
On Tue, Jun 28, 2016 at 8:06 PM, Masahiko Sawada wrote:
> On Tue, Jun 21, 2016 at 6:59 AM, Andres Freund wrote:
>> On 2016-06-20 17:55:19 -0400, Robert Haas wrote:
>>> On Mon, Jun 20, 2016 at 4:24 PM, Andres Freund wrote:
>>> > On 2016-06-20 16:10:23 -0400, Robert Haas wrote:
>>> >> What exactly
On Tue, Jun 21, 2016 at 6:59 AM, Andres Freund wrote:
> On 2016-06-20 17:55:19 -0400, Robert Haas wrote:
>> On Mon, Jun 20, 2016 at 4:24 PM, Andres Freund wrote:
>> > On 2016-06-20 16:10:23 -0400, Robert Haas wrote:
>> >> What exactly is the point of all of that already_marked stuff?
>> >
>> > Pr
On Tue, Jun 21, 2016 at 10:59:25AM +1200, Thomas Munro wrote:
> On Fri, Jun 17, 2016 at 3:36 PM, Noah Misch wrote:
> > I agree the non-atomic, unlogged change is a problem. A related threat
> > doesn't require a torn page:
> >
> > AssignTransactionId() xid=123
> > heapam.c:3931 HeapTupleHeade
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:
>>
>> > I'm looking into three approaches right now:
>> >
>> > 3) Use WAL logging for the already_marked = true case.
>>
>>
>> > 3) This approach so far seems the bes
On 2016-06-23 18:59:57 -0400, Alvaro Herrera wrote:
> Andres Freund wrote:
>
> > I'm looking into three approaches right now:
> >
> > 3) Use WAL logging for the already_marked = true case.
>
>
> > 3) This approach so far seems the best. It's possible to reuse the
> > xl_heap_lock record (in an
Andres Freund wrote:
> I'm looking into three approaches right now:
>
> 3) Use WAL logging for the already_marked = true case.
> 3) This approach so far seems the best. It's possible to reuse the
> xl_heap_lock record (in an afaics backwards compatible manner), and in
> most cases the overhead
On 2016-06-21 16:32:03 -0400, Robert Haas wrote:
> On Tue, Jun 21, 2016 at 3:46 PM, Andres Freund wrote:
> > On 2016-06-21 15:38:25 -0400, Robert Haas wrote:
> >> On Tue, Jun 21, 2016 at 1:49 PM, Andres Freund wrote:
> >> >> I'm also a bit dubious that LockAcquire is safe to call in general
> >>
On Tue, Jun 21, 2016 at 3:46 PM, Andres Freund wrote:
> On 2016-06-21 15:38:25 -0400, Robert Haas wrote:
>> On Tue, Jun 21, 2016 at 1:49 PM, Andres Freund wrote:
>> >> I'm also a bit dubious that LockAcquire is safe to call in general
>> >> with interrupts held.
>> >
>> > Looks like we could just
On 2016-06-21 15:38:25 -0400, Robert Haas wrote:
> On Tue, Jun 21, 2016 at 1:49 PM, Andres Freund wrote:
> >> I'm also a bit dubious that LockAcquire is safe to call in general
> >> with interrupts held.
> >
> > Looks like we could just acquire the tuple-lock *before* doing the
> > toast_insert_or
On Tue, Jun 21, 2016 at 1:49 PM, Andres Freund wrote:
>> I'm also a bit dubious that LockAcquire is safe to call in general
>> with interrupts held.
>
> Looks like we could just acquire the tuple-lock *before* doing the
> toast_insert_or_update/RelationGetBufferForTuple, but after releasing
> the
On 2016-06-21 13:03:24 -0400, Robert Haas wrote:
> On Tue, Jun 21, 2016 at 12:54 PM, Andres Freund wrote:
> >> I don't quite understand the intended semantics of this proposed flag.
> >
> > Whenever the flag is set, we have to acquire the heavyweight tuple lock
> > before continuing. That guarante
On Tue, Jun 21, 2016 at 12:54 PM, Andres Freund wrote:
>> I don't quite understand the intended semantics of this proposed flag.
>
> Whenever the flag is set, we have to acquire the heavyweight tuple lock
> before continuing. That guarantees nobody else can modify the tuple,
> while the lock is re
On 2016-06-21 10:50:36 -0400, Robert Haas wrote:
> On Mon, Jun 20, 2016 at 11:51 PM, Andres Freund wrote:
> >> > So far the best idea I have - and it's really not a good one - is to
> >> > invent a new hint-bit that tells concurrent updates to acquire a
> >> > heavyweight tuple lock, while releasi
On Tue, Jun 21, 2016 at 10:47 AM, Tom Lane wrote:
> Robert Haas writes:
>> On Mon, Jun 20, 2016 at 5:59 PM, Andres Freund wrote:
>>> Consider what happens if we, after restarting at l2, notice that we
>>> can't actually insert, but return in the !HeapTupleMayBeUpdated
>>> branch.
>
>> OK, I see
On Mon, Jun 20, 2016 at 11:51 PM, Andres Freund wrote:
>> > So far the best idea I have - and it's really not a good one - is to
>> > invent a new hint-bit that tells concurrent updates to acquire a
>> > heavyweight tuple lock, while releasing the page-level lock. If that
>> > hint bit does not re
Robert Haas writes:
> On Mon, Jun 20, 2016 at 5:59 PM, Andres Freund wrote:
>> Consider what happens if we, after restarting at l2, notice that we
>> can't actually insert, but return in the !HeapTupleMayBeUpdated
>> branch.
> OK, I see what you mean. Still, that doesn't seem like such a
> terr
On Mon, Jun 20, 2016 at 5:59 PM, Andres Freund wrote:
> On 2016-06-20 17:55:19 -0400, Robert Haas wrote:
>> On Mon, Jun 20, 2016 at 4:24 PM, Andres Freund wrote:
>> > On 2016-06-20 16:10:23 -0400, Robert Haas wrote:
>> >> I
>> >> mean, suppose we just don't do any of that before we go off to do
>
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
> > RelationGetBufferForTuple() will e
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 it. I think this could impact
>
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(), even though that'd be a good ide
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
> RelationGetBufferForTuple() will ensure that it always returns a new buffer
> which has targetblock
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 it. I think this could impact
> performance depending on how frequently we need to perform this a
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(), even though that'd be a good idea. But in this instance,
>> the problem seems relatively fundam
On Tue, Jun 21, 2016 at 1:03 AM, Andres Freund wrote:
>
> On 2016-06-15 08:56:52 -0400, Robert Haas wrote:
> > Yikes: heap_update() sets the tuple's XMAX, CMAX, infomask, infomask2,
> > and CTID without logging anything or clearing the all-visible flag and
> > then releases the lock on the heap pa
On Fri, Jun 17, 2016 at 3:36 PM, Noah Misch wrote:
> I agree the non-atomic, unlogged change is a problem. A related threat
> doesn't require a torn page:
>
> AssignTransactionId() xid=123
> heapam.c:3931 HeapTupleHeaderSetXmax(oldtup.t_data, 123);
> some ERROR before heap_update() finishes
On 2016-06-20 17:55:19 -0400, Robert Haas wrote:
> On Mon, Jun 20, 2016 at 4:24 PM, Andres Freund wrote:
> > On 2016-06-20 16:10:23 -0400, Robert Haas wrote:
> >> What exactly is the point of all of that already_marked stuff?
> >
> > Preventing the old tuple from being locked/updated by another ba
On Mon, Jun 20, 2016 at 4:24 PM, Andres Freund wrote:
> On 2016-06-20 16:10:23 -0400, Robert Haas wrote:
>> What exactly is the point of all of that already_marked stuff?
>
> Preventing the old tuple from being locked/updated by another backend,
> while unlocking the buffer.
>
>> I
>> mean, suppos
On 2016-06-20 16:10:23 -0400, Robert Haas wrote:
> What exactly is the point of all of that already_marked stuff?
Preventing the old tuple from being locked/updated by another backend,
while unlocking the buffer.
> I
> mean, suppose we just don't do any of that before we go off to do
> toast_ins
On Mon, Jun 20, 2016 at 3:33 PM, Andres Freund wrote:
>> I'm not sure what to do about this: this part of the heap_update()
>> logic has been like this forever, and I assume that if it were easy to
>> refactor this away, somebody would have done it by now.
>
> Well, I think generally nobody seriou
On 2016-06-15 08:56:52 -0400, Robert Haas wrote:
> Yikes: heap_update() sets the tuple's XMAX, CMAX, infomask, infomask2,
> and CTID without logging anything or clearing the all-visible flag and
> then releases the lock on the heap page to go do some more work that
> might even ERROR out. Only if
On Wed, Jun 15, 2016 at 08:56:52AM -0400, Robert Haas wrote:
> On Wed, Jun 15, 2016 at 2:41 AM, Thomas Munro
> wrote:
> > I spent some time chasing down the exact circumstances. I suspect
> > that there may be an interlocking problem in heap_update. Using the
> > line numbers from cae1c788 [1],
On Wed, Jun 15, 2016 at 9:59 AM, Amit Kapila wrote:
>> That just kicks the can down the road. Then you have PD_ALL_VISIBLE
>> clear but the VM bit is still set.
>
> I mean to say clear both as we are doing currently in code:
> if (PageIsAllVisible(BufferGetPage(buffer)))
> {
> all_visible_cleared
On Wed, Jun 15, 2016 at 10:03 AM, Masahiko Sawada wrote:
>> I'm not sure what to do about this: this part of the heap_update()
>> logic has been like this forever, and I assume that if it were easy to
>> refactor this away, somebody would have done it by now.
>
> How about changing collect_corrupt
On Wed, Jun 15, 2016 at 9:56 PM, Robert Haas wrote:
> On Wed, Jun 15, 2016 at 2:41 AM, Thomas Munro
> wrote:
>> I spent some time chasing down the exact circumstances. I suspect
>> that there may be an interlocking problem in heap_update. Using the
>> line numbers from cae1c788 [1], I see the f
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 time chasing down the exact circumstances. I su
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 time chasing down the exact circumstances. I suspect
>> > that there may be an interlocking problem in heap_upda
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 time chasing down the exact circumstances. I suspect
> > that there may be an interlocking problem in heap_update. Using the
> > line numbers from cae1c788 [1], I see
On Wed, Jun 15, 2016 at 2:41 AM, Thomas Munro
wrote:
> I spent some time chasing down the exact circumstances. I suspect
> that there may be an interlocking problem in heap_update. Using the
> line numbers from cae1c788 [1], I see the following interaction
> between the VACUUM, UPDATE and SELECT
On Wed, Jun 15, 2016 at 11:43 AM, Thomas Munro
wrote:
> On Wed, Jun 15, 2016 at 12:44 AM, Robert Haas wrote:
>> On Tue, Jun 14, 2016 at 8:11 AM, Robert Haas wrote:
> I noticed that the tuples that it reported were always offset 1 in a
> page, and that the page always had a maxoff over a
On Wed, Jun 15, 2016 at 12:44 AM, Robert Haas wrote:
> On Tue, Jun 14, 2016 at 8:11 AM, Robert Haas wrote:
I noticed that the tuples that it reported were always offset 1 in a
page, and that the page always had a maxoff over a couple of hundred,
and that we called record_corrupt_it
On Tue, Jun 14, 2016 at 8:11 AM, Robert Haas wrote:
>>> I noticed that the tuples that it reported were always offset 1 in a
>>> page, and that the page always had a maxoff over a couple of hundred,
>>> and that we called record_corrupt_item because VM_ALL_VISIBLE returned
>>> true but HeapTupleSa
On Tue, Jun 14, 2016 at 8:08 AM, Robert Haas wrote:
> On Tue, Jun 14, 2016 at 2:53 AM, Thomas Munro
> wrote:
>> On Tue, Jun 14, 2016 at 4:02 AM, Robert Haas wrote:
>>> On Sat, Jun 11, 2016 at 5:00 PM, Robert Haas wrote:
> How about changing the return tuple of heap_prepare_freeze_tuple to
>
1 - 100 of 255 matches
Mail list logo