Re: [PERFORM] Hanging queries on dual CPU windows
If so,
we could perhaps recode that part using a Mutex instead ofa critical
section - since it's not a performance critical path, the
difference
shouldn't be large. If I code up a patch for that, can you re-apply
SP1 and test it? Or is this a production system you can'treally touch?
I can do whatever the hell I want with it, so if you could
cook up a patch that would be great.As a BTW: I reinstalled SP1 and turned stats collection off.
That also seems to work, but is not really a solution since
we want to use autovacuuming.
Ok, I've coded up a patch that changes the code to use a mutex instead.
Patch attached. You can get a precompiled postgres.exe at
http://www.hagander.net/download/postgres.exe_mutex.zip. You need to
copy this file to postmaster.exe as well - they are supposed to be
identical. It's based off a snapshot of 8.1-stable.
Looking a my system while testing this it still loooked like it was
hanging on that plac ein the code, even though I saw no problems. So I'm
not convinced we can actually trust the stacktrace from the non-default
threads. So I don't think this patch will actually work :-( But it's
worth a try.
(Oh, and I moved the thread over to -hackers, seems more correct at this
time)
//Magnus
Attachments:
mutex.patchapplication/octet-stream; name=mutex.patchDownload
Index: src/backend/port/win32/signal.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/port/win32/signal.c,v
retrieving revision 1.14.2.1
diff -c -r1.14.2.1 signal.c
*** src/backend/port/win32/signal.c 22 Nov 2005 18:23:15 -0000 1.14.2.1
--- src/backend/port/win32/signal.c 12 Mar 2006 11:50:11 -0000
***************
*** 28,37 ****
HANDLE pgwin32_initial_signal_pipe = INVALID_HANDLE_VALUE;
/*
! * pg_signal_crit_sec is used to protect only pg_signal_queue. That is the only
* variable that can be accessed from the signal sending threads!
*/
! static CRITICAL_SECTION pg_signal_crit_sec;
static pqsigfunc pg_signal_array[PG_SIGNAL_COUNT];
static pqsigfunc pg_signal_defaults[PG_SIGNAL_COUNT];
--- 28,37 ----
HANDLE pgwin32_initial_signal_pipe = INVALID_HANDLE_VALUE;
/*
! * pg_signal_mutex is used to protect only pg_signal_queue. That is the only
* variable that can be accessed from the signal sending threads!
*/
! static HANDLE pg_signal_mutex;
static pqsigfunc pg_signal_array[PG_SIGNAL_COUNT];
static pqsigfunc pg_signal_defaults[PG_SIGNAL_COUNT];
***************
*** 61,67 ****
int i;
HANDLE signal_thread_handle;
! InitializeCriticalSection(&pg_signal_crit_sec);
for (i = 0; i < PG_SIGNAL_COUNT; i++)
{
--- 61,70 ----
int i;
HANDLE signal_thread_handle;
! pg_signal_mutex = CreateMutex(NULL, FALSE, NULL);
! if (pg_signal_mutex == NULL)
! ereport(FATAL,
! (errmsg_internal("failed to create signal mutex: %d", (int) GetLastError())));
for (i = 0; i < PG_SIGNAL_COUNT; i++)
{
***************
*** 99,105 ****
{
int i;
! EnterCriticalSection(&pg_signal_crit_sec);
while (UNBLOCKED_SIGNAL_QUEUE())
{
/* One or more unblocked signals queued for execution */
--- 102,111 ----
{
int i;
! if (WaitForSingleObject(pg_signal_mutex, INFINITE) != WAIT_OBJECT_0)
! ereport(FATAL,
! (errmsg_internal("failed to wait for signal mutex: %d", (int) GetLastError())));
!
while (UNBLOCKED_SIGNAL_QUEUE())
{
/* One or more unblocked signals queued for execution */
***************
*** 117,125 ****
pg_signal_queue &= ~sigmask(i);
if (sig != SIG_ERR && sig != SIG_IGN && sig != SIG_DFL)
{
! LeaveCriticalSection(&pg_signal_crit_sec);
sig(i);
! EnterCriticalSection(&pg_signal_crit_sec);
break; /* Restart outer loop, in case signal mask or
* queue has been modified inside signal
* handler */
--- 123,137 ----
pg_signal_queue &= ~sigmask(i);
if (sig != SIG_ERR && sig != SIG_IGN && sig != SIG_DFL)
{
! if (!ReleaseMutex(pg_signal_mutex))
! ereport(FATAL,
! (errmsg_internal("failed to release signal mutex: %d", (int) GetLastError())));
!
sig(i);
!
! if (WaitForSingleObject(pg_signal_mutex, INFINITE) != WAIT_OBJECT_0)
! ereport(FATAL,
! (errmsg_internal("failed to wait for signal mutex: %d", (int) GetLastError())));
break; /* Restart outer loop, in case signal mask or
* queue has been modified inside signal
* handler */
***************
*** 128,134 ****
}
}
ResetEvent(pgwin32_signal_event);
! LeaveCriticalSection(&pg_signal_crit_sec);
}
/* signal masking. Only called on main thread, no sync required */
--- 140,148 ----
}
}
ResetEvent(pgwin32_signal_event);
! if (!ReleaseMutex(pg_signal_mutex))
! ereport(FATAL,
! (errmsg_internal("failed to release signal mutex: %d", (int) GetLastError())));
}
/* signal masking. Only called on main thread, no sync required */
***************
*** 199,207 ****
if (signum >= PG_SIGNAL_COUNT || signum <= 0)
return;
! EnterCriticalSection(&pg_signal_crit_sec);
pg_signal_queue |= sigmask(signum);
! LeaveCriticalSection(&pg_signal_crit_sec);
SetEvent(pgwin32_signal_event);
}
--- 213,225 ----
if (signum >= PG_SIGNAL_COUNT || signum <= 0)
return;
! if (WaitForSingleObject(pg_signal_mutex, INFINITE) != WAIT_OBJECT_0)
! write_stderr("failed to wait for signal mutex: %d", (int) GetLastError());
!
pg_signal_queue |= sigmask(signum);
!
! if (!ReleaseMutex(pg_signal_mutex))
! write_stderr("failed to release signal mutex: %d", (int) GetLastError());
SetEvent(pgwin32_signal_event);
}
On Sunday 12 March 2006 09:40, Magnus Hagander wrote:
If so,
we could perhaps recode that part using a Mutex instead ofa critical
section - since it's not a performance critical path, the
difference
shouldn't be large. If I code up a patch for that, can you re-apply
SP1 and test it? Or is this a production system you can'treally touch?
I can do whatever the hell I want with it, so if you could
cook up a patch that would be great.As a BTW: I reinstalled SP1 and turned stats collection off.
That also seems to work, but is not really a solution since
we want to use autovacuuming.Ok, I've coded up a patch that changes the code to use a mutex instead.
Patch attached. You can get a precompiled postgres.exe at
http://www.hagander.net/download/postgres.exe_mutex.zip. You need to
copy this file to postmaster.exe as well - they are supposed to be
identical. It's based off a snapshot of 8.1-stable.Looking a my system while testing this it still loooked like it was
hanging on that plac ein the code, even though I saw no problems. So I'm
not convinced we can actually trust the stacktrace from the non-default
threads. So I don't think this patch will actually work :-( But it's
worth a try.(Oh, and I moved the thread over to -hackers, seems more correct at this
time)
Thanks Magnus,
I'll try tomorrow. Will let you know ASAP (8:30 EST I guess :).
If this doesn't work, how do we progress?
//Magnus
jan
--
--------------------------------------------------------------
Jan de Visser jdevisser@digitalfairway.com
Baruk Khazad! Khazad ai-menu!
--------------------------------------------------------------
""Magnus Hagander"" <mha@sollentuna.net> wrote
Ok, I've coded up a patch that changes the code to use a mutex instead.
Are we asserting the problem is caused by the spinlock random wake-up order?
I am not sure why this would fix the problem. If my memory serves, a
critical section might be a problem if one process aborts unexpected while
it is inside. Other waiting processes can never have a chance to enter it
(also have no chance to handle SIGQUIT) -- so this patch may solve this.
There is another suspect in http://www.devisser-siderius.com/stack1.jpg,
i.e., process 3 does shmctl. I once filed a server core dump bug in win32 of
reporting WSAEWOULDBLOCK.
(http://archives.postgresql.org/pgsql-bugs/2006-02/msg00185.php). AFAICS, it
is actually an mistranslated EINTR. There seems some relation between these
issues, but I didn't come up with a complete theory of it.
Regards,
Qingqing
Ok, I've coded up a patch that changes the code to use a
mutex instead.
Are we asserting the problem is caused by the spinlock random
wake-up order?
Not asserting, more making a wild guess. Which I, as I said, no lnoger
really beleive in - but since the patch was already coded up it's worth
a try.
I am not sure why this would fix the problem. If my memory
serves, a critical section might be a problem if one process
aborts unexpected while it is inside. Other waiting processes
can never have a chance to enter it (also have no chance to
handle SIGQUIT) -- so this patch may solve this.
A critical section only exists within a single process, so that realliy
doesn't apply. And if a thread crashes, the whole process exists.
There is another suspect in
http://www.devisser-siderius.com/stack1.jpg,
i.e., process 3 does shmctl. I once filed a server core dump
bug in win32 of reporting WSAEWOULDBLOCK.
(http://archives.postgresql.org/pgsql-bugs/2006-02/msg00185.ph
p). AFAICS, it is actually an mistranslated EINTR. There
seems some relation between these issues, but I didn't come
up with a complete theory of it.
There could well be. Except the link you sent pointed to a thread stuck
in pgwin32_waitforsinglesocket() insider pgwin32_send() - this is where
I beleive the problem is now.
I'm less-than-trusting the function names in the stacktrace after
examining some more. I'm suspecting process explorer can only see
non-static functions, and that the "pg_queue_signal+0x120" actually
points into a different function. (really, pg_queue_signal cannot
possibly be 0x120 bytes machine code..) I bet it's just in
pg_signal_thread(), which is a perfectlyi normal place to block. It also
matches the behaviour I see on a completely fresh backend - which also
shows that pg_queue_signal+0x120.
A good thing to test would be to rebuild signal.c and socket.c without
any functions declared as static and see if the picture changes. (If
nothing else it would confirm this behaviour in process explorer)
Mvh,
Magnus
Import Notes
Resolved by subject fallback
On Sunday 12 March 2006 09:40, Magnus Hagander wrote:
Looking a my system while testing this it still loooked like it was
hanging on that plac ein the code, even though I saw no problems. So I'm
not convinced we can actually trust the stacktrace from the non-default
threads. So I don't think this patch will actually work :-( But it's
worth a try.
I'm afraid you're right. Hangs again :(
jan
--
--------------------------------------------------------------
Jan de Visser jdevisser@digitalfairway.com
Baruk Khazad! Khazad ai-menu!
--------------------------------------------------------------
On Monday 13 March 2006 09:26, Jan de Visser wrote:
On Sunday 12 March 2006 09:40, Magnus Hagander wrote:
Looking a my system while testing this it still loooked like it was
hanging on that plac ein the code, even though I saw no problems. So I'm
not convinced we can actually trust the stacktrace from the non-default
threads. So I don't think this patch will actually work :-( But it's
worth a try.I'm afraid you're right. Hangs again :(
I now have the toolchain set up, so if you want me to try stuff, please let me
know. Resolving this is important to us.
On a whim, I replaced InitializeCriticalSection with
InitializeCriticalSectionAndSpinCount, since MSDN told me that would be
better for SMP. No joy.
jan
--
--------------------------------------------------------------
Jan de Visser jdevisser@digitalfairway.com
Baruk Khazad! Khazad ai-menu!
--------------------------------------------------------------
Looking a my system while testing this it still loooked
like it was
hanging on that plac ein the code, even though I saw no
problems. So
I'm not convinced we can actually trust the stacktrace from the
non-default threads. So I don't think this patch willactually work
:-( But it's worth a try.
I'm afraid you're right. Hangs again :(
I now have the toolchain set up, so if you want me to try
stuff, please let me know. Resolving this is important to us.
Great. That'll certainly help - now you don't have to wait for binaries
from me.
What I'd be interested in seeing is new stackdumps from a version where
you:
1) Do *not* have the patch for mutexes applied
2) Have removed "static" from all the function devlarations in signal.c
and socket.c, bnoth in src/backend/port/win32.
If you can, it'd be interesting to see it from the pre-SP1 install as
well - once it hangs.
On a whim, I replaced InitializeCriticalSection with
InitializeCriticalSectionAndSpinCount, since MSDN told me
that would be better for SMP. No joy.
No, that should make no difference - except possibly a tiny difference
in speed.
Do you have the ability to test 8.0 on the same machine? We did some
extensive modifications to the signal stuff between 8.0 and 8.1, it'd be
interesting to see if that changed things.
//Magnus
Import Notes
Resolved by subject fallback
Do you have the ability to test 8.0 on the same machine? We did some
extensive modifications to the signal stuff between 8.0 and 8.1, it'd be
interesting to see if that changed things.
I had very similar behavior some weeks back on a machine that had not
been upgraded to 8.1. It was a dual opteron on win2k server. Some
simple queries (select 1 + 2) would work ok but anything that did real
work on tables would hang and the backend would not respond to
signals. Only recourse was to end task from task mgr which cycled the
entire server with no data loss.
This may or may not be the same problem but it sounds similar.
Unfortunately the problem was extremely time sensitive and I could not
play with it much. However, since this is pre-8.1 this argues against
Qingqing's signal changes (maybe). I've since moved on to a linux
environment so my win32 contributions will diminish greatly :)
merlin
On Monday 13 March 2006 12:27, Magnus Hagander wrote:
Great. That'll certainly help - now you don't have to wait for binaries
from me.What I'd be interested in seeing is new stackdumps from a version where
you:
1) Do *not* have the patch for mutexes applied
2) Have removed "static" from all the function devlarations in signal.c
and socket.c, bnoth in src/backend/port/win32.
I did that, and the interesting thing is that:
1. It takes much longer to hang.
2. Once it hangs, the stacktraces are the same.
3 (and this is the kicker). The thing starts working again after a couple (+/-
5) minutes ?????????
1. can probably be explained by the fact that I didn't compile with any
optimization. Can you tell me what CFLAGS the binary distro uses? 2. I don't
know (are there other tools I can use?), and 3. I frankly don't understand. I
know for sure that with the stock 8.1.3 it would not revive itself (I let it
running for a *long* time).
If you can, it'd be interesting to see it from the pre-SP1 install as
well - once it hangs.
I've never seen a pre-SP1 install hang.
On a whim, I replaced InitializeCriticalSection with
InitializeCriticalSectionAndSpinCount, since MSDN told me
that would be better for SMP. No joy.No, that should make no difference - except possibly a tiny difference
in speed.Do you have the ability to test 8.0 on the same machine? We did some
extensive modifications to the signal stuff between 8.0 and 8.1, it'd be
interesting to see if that changed things.
I seem to remember we made ourselves dependend on 8.1 somehow, but will check.
jan
--
--------------------------------------------------------------
Jan de Visser jdevisser@digitalfairway.com
Baruk Khazad! Khazad ai-menu!
--------------------------------------------------------------
On Monday 13 March 2006 12:27, Magnus Hagander wrote:
Great. That'll certainly help - now you don't have to wait for
binaries from me.What I'd be interested in seeing is new stackdumps from a version
where
you:
1) Do *not* have the patch for mutexes applied
2) Have removed "static" from all the function devlarations in
signal.c and socket.c, bnoth in src/backend/port/win32.I did that, and the interesting thing is that:
1. It takes much longer to hang.
?! That shouldn't be related :-)
2. Once it hangs, the stacktraces are the same.
Hmm. That's weird :-(
Did you do a make clean? Sometimes needed to get the port stuff in,
mingw messes up sometimes.
3 (and this is the kicker). The thing starts working again
after a couple (+/-
5) minutes ?????????
Interesting. And you get nothing in the logs? (pg_log / eventlog)
1. can probably be explained by the fact that I didn't
compile with any optimization. Can you tell me what CFLAGS
the binary distro uses?
You can use pg_config to see that.
2. I don't know (are there other tools I can use?), and
Not really, but try the make clean.
3. I frankly don't understand. I know
for sure that with the stock 8.1.3 it would not revive itself
(I let it running for a *long* time).
Very interesting.
If you can, it'd be interesting to see it from the pre-SP1
install as
well - once it hangs.
I've never seen a pre-SP1 install hang.
Oh, hang on, what I meant was with the post-SP1 hang but with stats
disabled. :-)
Do you have the ability to test 8.0 on the same machine? We
did some
extensive modifications to the signal stuff between 8.0 and
8.1, it'd
be interesting to see if that changed things.
I seem to remember we made ourselves dependend on 8.1
somehow, but will check.
Ok. Please do.
//Magnus
Import Notes
Resolved by subject fallback
Magnus Hagander wrote:
On Monday 13 March 2006 12:27, Magnus Hagander wrote:
Great. That'll certainly help - now you don't have to wait for
binaries from me.What I'd be interested in seeing is new stackdumps from a version
where
you:
1) Do *not* have the patch for mutexes applied
2) Have removed "static" from all the function devlarations in
signal.c and socket.c, bnoth in src/backend/port/win32.I did that, and the interesting thing is that:
1. It takes much longer to hang.?! That shouldn't be related :-)
2. Once it hangs, the stacktraces are the same.
Hmm. That's weird :-(
Did you do a make clean? Sometimes needed to get the port stuff in,
mingw messes up sometimes.3 (and this is the kicker). The thing starts working again
after a couple (+/-
5) minutes ?????????Interesting. And you get nothing in the logs? (pg_log / eventlog)
There's a report on -bugs ([BUGS] Random hang during commit /
b.lin@dl.ac.uk) that might well be related to this.
--
Richard Huxton
Archonet Ltd