On 2014-03-21 22:52:33 +0100, Andres Freund wrote:
> The committed version doesn't compile with LWLOCK_STATS...
Just noticed that it seems to also break the dtrace stuff:
http://pgbuildfarm.org/cgi-bin/show_log.pl?nm=rover_firefly&dt=2014-03-21%2018%3A04%3A00
Greetings,
Andres Freund
--
Andre
Hi,
I see you've committed this, cool. Sorry for not getting back to the
topic earlier..
On 2014-03-13 22:44:03 +0200, Heikki Linnakangas wrote:
> On 03/12/2014 09:29 PM, Andres Freund wrote:
> >On 2014-03-07 17:54:32 +0200, Heikki Linnakangas wrote:
> >>So there are some unexplained differences
On 2014-03-07 17:54:32 +0200, Heikki Linnakangas wrote:
> So there are some unexplained differences there, but based on these results,
> I'm still OK with committing the patch.
So, I am looking at this right now.
I think there are some minor things I'd like to see addressed:
1) I think there nee
On 02/17/2014 10:36 PM, Andres Freund wrote:
On 2014-02-17 22:30:54 +0200, Heikki Linnakangas wrote:
This is what I came up with. I like it, I didn't have to contort lwlocks as
much as I feared. I added one field to LWLock structure, which is used to
store the position of how far a WAL inserter
From: "Peter Geoghegan"
You mentioned a hang during a B-Tree insert operation - do you happen
to have a backtrace that relates to that?
Sorry, I may have misunderstood. The three stack traces I attached are not
related to btree. I recall that I saw one stack trace containing
bt_insert(), b
On Wed, Feb 12, 2014 at 3:55 AM, MauMau wrote:
> FYI, the following stack traces are the ones obtained during two instances
> of hang.
You mentioned a hang during a B-Tree insert operation - do you happen
to have a backtrace that relates to that?
--
Peter Geoghegan
--
Sent via pgsql-hackers
On 2014-02-17 22:30:54 +0200, Heikki Linnakangas wrote:
> This is what I came up with. I like it, I didn't have to contort lwlocks as
> much as I feared. I added one field to LWLock structure, which is used to
> store the position of how far a WAL inserter has progressed. The LWLock code
> calls it
On 02/10/2014 08:33 PM, Heikki Linnakangas wrote:
On 02/10/2014 08:03 PM, Tom Lane wrote:
Heikki Linnakangas writes:
On 02/10/2014 06:41 PM, Andres Freund wrote:
Well, it's not actually using any lwlock.c code, it's a special case
locking logic, just reusing the datastructures. That said, I a
On 2014-02-17 14:06:43 -0500, Robert Haas wrote:
> On Mon, Feb 17, 2014 at 1:55 PM, Andres Freund wrote:
> > On 2014-02-17 13:49:01 -0500, Robert Haas wrote:
> > It's just a write barrier which evaluates to a pure compiler barrier on
> > x86 anyway?
> > And it's in a loop that's only entered when
On Mon, Feb 17, 2014 at 1:55 PM, Andres Freund wrote:
> On 2014-02-17 13:49:01 -0500, Robert Haas wrote:
>> On Sat, Feb 15, 2014 at 11:17 AM, Andres Freund
>> wrote:
>> > On 2014-02-15 16:18:00 +0100, Andres Freund wrote:
>> >> On 2014-02-15 10:06:41 -0500, Tom Lane wrote:
>> >> > Andres Freund
On 2014-02-17 13:49:01 -0500, Robert Haas wrote:
> On Sat, Feb 15, 2014 at 11:17 AM, Andres Freund
> wrote:
> > On 2014-02-15 16:18:00 +0100, Andres Freund wrote:
> >> On 2014-02-15 10:06:41 -0500, Tom Lane wrote:
> >> > Andres Freund writes:
> >> > > My current conclusion is that backporting ba
On Sat, Feb 15, 2014 at 11:17 AM, Andres Freund wrote:
> On 2014-02-15 16:18:00 +0100, Andres Freund wrote:
>> On 2014-02-15 10:06:41 -0500, Tom Lane wrote:
>> > Andres Freund writes:
>> > > My current conclusion is that backporting barriers.h is by far the most
>> > > reasonable way to go. The c
On 2014-02-15 16:18:00 +0100, Andres Freund wrote:
> On 2014-02-15 10:06:41 -0500, Tom Lane wrote:
> > Andres Freund writes:
> > > My current conclusion is that backporting barriers.h is by far the most
> > > reasonable way to go. The compiler problems have been ironed out by
> > > now...
> >
> >
On 2014-02-15 10:06:41 -0500, Tom Lane wrote:
> Andres Freund writes:
> > My current conclusion is that backporting barriers.h is by far the most
> > reasonable way to go. The compiler problems have been ironed out by
> > now...
>
> -1. IMO that code is still quite unproven, and what's more, the
Andres Freund writes:
> My current conclusion is that backporting barriers.h is by far the most
> reasonable way to go. The compiler problems have been ironed out by
> now...
-1. IMO that code is still quite unproven, and what's more, the
problem we're discussing here is completely hypothetical.
On 2014-02-15 04:20:17 +0100, Florian Pflug wrote:
> Another idea would be to do as you suggest and only mark the PGPROC pointers
> volatile, but to additionally add a check for queue corruption somewhere. We
> should
> be able to detect that - if we ever hit this issue, LWLockRelease should find
On Feb14, 2014, at 19:21 , Andres Freund wrote:
> On 2014-02-14 18:49:33 +0100, Florian Pflug wrote:
>> Well, the assumption isn't all that new. We already have the situation that
>> a PGPROC may be not on any wait queue, yet its lwWaitLink may be non-NULL.
>> Currently, the process who took it of
On 2014-02-14 18:49:33 +0100, Florian Pflug wrote:
> > I wouldn't consider it a major architecture... And I am not sure how
> > much out of order a CPU has to be to be affected by this, the read side
> > uses spinlocks, which in most of the spinlock implementations implies a
> > full memory barrier
On Feb14, 2014, at 16:51 , Andres Freund wrote:
> On 2014-02-14 15:03:16 +0100, Florian Pflug wrote:
>> On Feb14, 2014, at 14:07 , Andres Freund wrote:
>>> On 2014-02-14 13:52:45 +0100, Florian Pflug wrote:
> I agree we should do that, but imo not in the backbranches. Anything
> more than
On 02/14/2014 08:28 PM, Andres Freund wrote:
On 2014-02-14 20:23:32 +0400, knizhnik wrote:
we'll trade correctness for cleanliness if we continue to reset lwWaitLink
without protecting against the race. That's a bit of a weird trade-off to make.
It's not just cleanliness, it's being able to ac
On 2014-02-14 20:23:32 +0400, knizhnik wrote:
> >>we'll trade correctness for cleanliness if we continue to reset lwWaitLink
> >>without protecting against the race. That's a bit of a weird trade-off to
> >>make.
> >
> >It's not just cleanliness, it's being able to actually debug crashes.
>
>
>
On 02/14/2014 07:51 PM, Andres Freund wrote:
On 2014-02-14 15:03:16 +0100, Florian Pflug wrote:
On Feb14, 2014, at 14:07 , Andres Freund wrote:
On 2014-02-14 13:52:45 +0100, Florian Pflug wrote:
I agree we should do that, but imo not in the backbranches. Anything
more than than the minimal fi
On 2014-02-14 15:03:16 +0100, Florian Pflug wrote:
> On Feb14, 2014, at 14:07 , Andres Freund wrote:
> > On 2014-02-14 13:52:45 +0100, Florian Pflug wrote:
> >>> I agree we should do that, but imo not in the backbranches. Anything
> >>> more than than the minimal fix in that code should be avoided
On Feb14, 2014, at 16:32 , Andres Freund wrote:
> On 2014-02-14 10:26:07 -0500, Tom Lane wrote:
>> Florian Pflug writes:
>>> Another idea for a fix would be to conflate lwWaiting and lwWaitLink into
>>> one
>>> field. We could replace "lwWaiting" by "lwWaitLink != NULL" everywhere it's
>>> teste
On 2014-02-14 10:26:07 -0500, Tom Lane wrote:
> Florian Pflug writes:
> > Another idea for a fix would be to conflate lwWaiting and lwWaitLink into
> > one
> > field. We could replace "lwWaiting" by "lwWaitLink != NULL" everywhere it's
> > tested, and set lwWaitLink to some special non-NULL value
Florian Pflug writes:
> Another idea for a fix would be to conflate lwWaiting and lwWaitLink into one
> field. We could replace "lwWaiting" by "lwWaitLink != NULL" everywhere it's
> tested, and set lwWaitLink to some special non-NULL value (say 0x1) when we
> enqueue a PGPROC, instead of setting i
On Feb14, 2014, at 14:07 , Andres Freund wrote:
> On 2014-02-14 13:52:45 +0100, Florian Pflug wrote:
>>> I agree we should do that, but imo not in the backbranches. Anything
>>> more than than the minimal fix in that code should be avoided in the
>>> stable branches, this stuff is friggin performa
On 2014-02-14 13:52:45 +0100, Florian Pflug wrote:
> > I agree we should do that, but imo not in the backbranches. Anything
> > more than than the minimal fix in that code should be avoided in the
> > stable branches, this stuff is friggin performance sensitive, and the
> > spinlock already is a *m
On Feb14, 2014, at 13:36 , Andres Freund wrote:
> On 2014-02-14 13:28:47 +0100, Florian Pflug wrote:
>>> I don't think that can actually happen because the head of the wait list
>>> isn't the lock holder's lwWaitLink, but LWLock->head. I thought the same
>>> for a while...
>>
>> Hm, true, but doe
On 2014-02-14 13:28:47 +0100, Florian Pflug wrote:
> > I don't think that can actually happen because the head of the wait list
> > isn't the lock holder's lwWaitLink, but LWLock->head. I thought the same
> > for a while...
>
> Hm, true, but does that protect us under all circumstances? If another
On Feb14, 2014, at 11:45 , Andres Freund wrote:
> On 2014-02-13 15:34:09 +0100, Florian Pflug wrote:
>> On Feb10, 2014, at 17:38 , Andres Freund wrote:
>>> On 2014-02-10 11:11:28 -0500, Tom Lane wrote:
Andres Freund writes:
> So what we need to do is to acquire a write barrier between t
On 2014-02-13 15:34:09 +0100, Florian Pflug wrote:
> On Feb10, 2014, at 17:38 , Andres Freund wrote:
> > On 2014-02-10 11:11:28 -0500, Tom Lane wrote:
> >> Andres Freund writes:
> >>> So what we need to do is to acquire a write barrier between the
> >>> assignments to lwWaitLink and lwWaiting, i.
On Feb10, 2014, at 17:38 , Andres Freund wrote:
> On 2014-02-10 11:11:28 -0500, Tom Lane wrote:
>> Andres Freund writes:
>>> So what we need to do is to acquire a write barrier between the
>>> assignments to lwWaitLink and lwWaiting, i.e.
>>>proc->lwWaitLink = NULL;
>>>pg_write_ba
On 02/12/2014 06:10 PM, Ants Aasma wrote:
On Wed, Feb 12, 2014 at 4:04 PM, knizhnik wrote:
Even if reordering was not done by compiler, it still can happen while
execution.
There is no warranty that two subsequent assignments will be observed by all
CPU cores in the same order.
The x86 memory
eFrom: "Andres Freund"
could you try if you get more readable dumps by using disassemble/m?
That might at least print line numbers if you have debug info installed.
Please find the attached file. I hope this will reveal something.
Regards
MauMau
Dump of assembler code for function LWLockRel
On Wed, Feb 12, 2014 at 4:04 PM, knizhnik wrote:
> Even if reordering was not done by compiler, it still can happen while
> execution.
> There is no warranty that two subsequent assignments will be observed by all
> CPU cores in the same order.
The x86 memory model (total store order) provides th
On 02/12/2014 05:42 PM, Florian Pflug wrote:
On Feb12, 2014, at 12:55 , MauMau wrote:
From: "Andres Freund"
It's x86, right? Then it's unlikely to be actual unordered memory
accesses, but if the compiler reordered:
LOG_LWDEBUG("LWLockRelease", T_NAME(l), T_ID(l), "release waiter");
proc
On Feb12, 2014, at 12:55 , MauMau wrote:
> From: "Andres Freund"
>> It's x86, right? Then it's unlikely to be actual unordered memory
>> accesses, but if the compiler reordered:
>> LOG_LWDEBUG("LWLockRelease", T_NAME(l), T_ID(l), "release waiter");
>> proc = head;
>> head = proc->lwWaitLink
From: "Andres Freund"
could you try if you get more readable dumps by using disassemble/m?
That might at least print line numbers if you have debug info installed.
OK, I'll try that tomorrow. However, the debug info is not available,
because they use PostgreSQL built by themselves, not the c
On 2014-02-12 20:55:32 +0900, MauMau wrote:
> Dump of assembler code for function LWLockRelease:
could you try if you get more readable dumps by using disassemble/m?
That might at least print line numbers if you have debug info installed.
Greetings,
Andres Freund
--
Andres Freund
From: "Andres Freund"
It's x86, right? Then it's unlikely to be actual unordered memory
accesses, but if the compiler reordered:
LOG_LWDEBUG("LWLockRelease", T_NAME(l), T_ID(l), "release waiter");
proc = head;
head = proc->lwWaitLink;
proc->lwWaitLink = NULL;
proc->lwWaiting = fal
On 2014-02-11 21:46:04 +0900, MauMau wrote:
> From: "Andres Freund"
> >which means they manipulate the lwWaitLink queue without
> >protection. That's done intentionally. The code tries to protect against
> >corruption of the list to do a woken up backend acquiring a lock (this
> >or an independent
From: "Andres Freund"
which means they manipulate the lwWaitLink queue without
protection. That's done intentionally. The code tries to protect against
corruption of the list to do a woken up backend acquiring a lock (this
or an independent one) by only continuing when the lwWaiting flag is set
On 2014-02-10 19:48:47 +0200, Heikki Linnakangas wrote:
> On 02/10/2014 06:41 PM, Andres Freund wrote:
> >On 2014-02-10 11:20:30 -0500, Tom Lane wrote:
> >>I wrote:
> >>>You didn't really explain why you think that ordering is necessary?
> >>
> >>Actually, after grepping to check my memory of what
On 02/10/2014 08:03 PM, Tom Lane wrote:
Heikki Linnakangas writes:
On 02/10/2014 06:41 PM, Andres Freund wrote:
Well, it's not actually using any lwlock.c code, it's a special case
locking logic, just reusing the datastructures. That said, I am not
particularly happy about the amount of code i
On 02/10/2014 03:46 PM, Andres Freund wrote:
Hi,
During the lwlock scalability work I noticed a longstanding issue with
the lwlock code. LWLockRelease() and the other mentioned locations do
the following to wake up any waiters, without holding the lock's
spinlock:
/*
* Awaken any wait
Heikki Linnakangas writes:
> On 02/10/2014 06:41 PM, Andres Freund wrote:
>> Well, it's not actually using any lwlock.c code, it's a special case
>> locking logic, just reusing the datastructures. That said, I am not
>> particularly happy about the amount of code it's duplicating from
>> lwlock.c.
On 02/10/2014 06:41 PM, Andres Freund wrote:
On 2014-02-10 11:20:30 -0500, Tom Lane wrote:
I wrote:
You didn't really explain why you think that ordering is necessary?
Actually, after grepping to check my memory of what those fields are
being used for, I have a bigger question: WTF is xlog.c
I wrote:
> You didn't really explain why you think that ordering is necessary?
Actually, after grepping to check my memory of what those fields are
being used for, I have a bigger question: WTF is xlog.c doing being
so friendly with the innards of LWLocks? Surely this needs to get
refactored so t
On 2014-02-10 11:20:30 -0500, Tom Lane wrote:
> I wrote:
> > You didn't really explain why you think that ordering is necessary?
>
> Actually, after grepping to check my memory of what those fields are
> being used for, I have a bigger question: WTF is xlog.c doing being
> so friendly with the inn
On 2014-02-10 11:11:28 -0500, Tom Lane wrote:
> Andres Freund writes:
> > So what we need to do is to acquire a write barrier between the
> > assignments to lwWaitLink and lwWaiting, i.e.
> > proc->lwWaitLink = NULL;
> > pg_write_barrier();
> > proc->lwWaiting = false;
>
>
Andres Freund writes:
> So what we need to do is to acquire a write barrier between the
> assignments to lwWaitLink and lwWaiting, i.e.
> proc->lwWaitLink = NULL;
> pg_write_barrier();
> proc->lwWaiting = false;
You didn't really explain why you think that ordering is nece
Hi,
During the lwlock scalability work I noticed a longstanding issue with
the lwlock code. LWLockRelease() and the other mentioned locations do
the following to wake up any waiters, without holding the lock's
spinlock:
/*
* Awaken any waiters I removed from the queue.
*/
while (
53 matches
Mail list logo
