Re: [HACKERS] dtrace probes

[Jesper Pedersen]

On 04/20/2017 10:30 AM, Jesper Pedersen wrote:

I think this fix is harmless and has some value in terms of
consistency.  One minor suggestion is that you should leave a space
after typecasting.

- TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
+ TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), (int)mode);

There should be a space like "(int) mode".




v2 attached.



I managed to attach the same patch again, so here is v3.

Best regards,
 Jesper


From 0d964df84950ca90c08ed6dd77a575d4b70ea7db Mon Sep 17 00:00:00 2001
From: jesperpedersen 
Date: Tue, 18 Apr 2017 11:44:18 -0400
Subject: [PATCH] Explicit cast LWLockMode, which an enum, to int in order to
 match the dtrace definition of the lwlock methods. Thereby all call sites
 will have the same definition instead of a mix between signed and unsigned.

Author: Jesper Pedersen 
---
 src/backend/storage/lmgr/lwlock.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/backend/storage/lmgr/lwlock.c 
b/src/backend/storage/lmgr/lwlock.c
index 3e13394..c551be2 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1226,7 +1226,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode)
 #endif
 
LWLockReportWaitStart(lock);
-   TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), mode);
+   TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), (int) mode);
 
for (;;)
{
@@ -1248,7 +1248,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode)
}
 #endif
 
-   TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
+   TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), (int) mode);
LWLockReportWaitEnd();
 
LOG_LWDEBUG("LWLockAcquire", lock, "awakened");
@@ -1257,7 +1257,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode)
result = false;
}
 
-   TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(lock), mode);
+   TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(lock), (int) mode);
 
/* Add lock to list of locks held by this backend */
held_lwlocks[num_held_lwlocks].lock = lock;
@@ -1308,14 +1308,14 @@ LWLockConditionalAcquire(LWLock *lock, LWLockMode mode)
RESUME_INTERRUPTS();
 
LOG_LWDEBUG("LWLockConditionalAcquire", lock, "failed");
-   TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(T_NAME(lock), mode);
+   TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(T_NAME(lock), (int) 
mode);
}
else
{
/* Add lock to list of locks held by this backend */
held_lwlocks[num_held_lwlocks].lock = lock;
held_lwlocks[num_held_lwlocks++].mode = mode;
-   TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(T_NAME(lock), mode);
+   TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(T_NAME(lock), (int) mode);
}
return !mustwait;
 }
@@ -1387,7 +1387,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 #endif
 
LWLockReportWaitStart(lock);
-   TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), mode);
+   TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), (int) 
mode);
 
for (;;)
{
@@ -1405,7 +1405,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
Assert(nwaiters < MAX_BACKENDS);
}
 #endif
-   TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
+   TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), (int) 
mode);
LWLockReportWaitEnd();
 
LOG_LWDEBUG("LWLockAcquireOrWait", lock, "awakened");
@@ -1435,7 +1435,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
/* Failed to get lock, so release interrupt holdoff */
RESUME_INTERRUPTS();
LOG_LWDEBUG("LWLockAcquireOrWait", lock, "failed");
-   TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT_FAIL(T_NAME(lock), 
mode);
+   TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT_FAIL(T_NAME(lock), 
(int) mode);
}
else
{
@@ -1443,7 +1443,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
/* Add lock to list of locks held by this backend */
held_lwlocks[num_held_lwlocks].lock = lock;
held_lwlocks[num_held_lwlocks++].mode = mode;
-   TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT(T_NAME(lock), mode);
+   TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT(T_NAME(lock), (int) 
mode);
}
 
return !mustwait;
-- 
2.7.4


-- 
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] dtrace probes

[Jesper Pedersen]

Hi,

On 04/20/2017 09:24 AM, Amit Kapila wrote:

The lwlock dtrace probes define LWLockMode as int, and the
TRACE_POSTGRESQL_LWLOCK methods are called using both a variable and
constant definition.

This leads to a mix of argument definitions depending on the call site, as
seen in probes.txt file.

A fix is to explicit cast 'mode' to int such that all call sites will use
the

 argument #2 4 signed   bytes

definition. Attached patch does this.



I think this fix is harmless and has some value in terms of
consistency.  One minor suggestion is that you should leave a space
after typecasting.

- TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
+ TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), (int)mode);

There should be a space like "(int) mode".




v2 attached.


I have verified all dtraces probes for their type, and only the lock__
methods doesn't aligned with its actual types.



Do you see any problem with that?



Not really, but it would be more clear what the value space of each of 
the parameters were.




Depending on the feedback I can add this patch to the open item list in
order to fix it for PostgreSQL 10.



Is there any commit in PG-10 which has caused this behavior?  If not,
then I don't think it should be added to open items of PG-10.




It is really a bug fix, so it could even be back patched.

Thanks for the feedback !

Best regards,
 Jesper


>From 7175dc8e05ff703229bd6cab6b254587ffc076c8 Mon Sep 17 00:00:00 2001
From: jesperpedersen 
Date: Tue, 18 Apr 2017 11:44:18 -0400
Subject: [PATCH] Explicit cast LWLockMode, which an enum, to int in order to
 match the dtrace definition of the lwlock methods. Thereby all call sites
 will have the same definition instead of a mix between signed and unsigned.

Author: Jesper Pedersen 
---
 src/backend/storage/lmgr/lwlock.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 3e13394..abf5fbb 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1226,7 +1226,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode)
 #endif
 
 		LWLockReportWaitStart(lock);
-		TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), (int)mode);
 
 		for (;;)
 		{
@@ -1248,7 +1248,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode)
 		}
 #endif
 
-		TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), (int)mode);
 		LWLockReportWaitEnd();
 
 		LOG_LWDEBUG("LWLockAcquire", lock, "awakened");
@@ -1257,7 +1257,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode)
 		result = false;
 	}
 
-	TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(lock), mode);
+	TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(lock), (int)mode);
 
 	/* Add lock to list of locks held by this backend */
 	held_lwlocks[num_held_lwlocks].lock = lock;
@@ -1308,14 +1308,14 @@ LWLockConditionalAcquire(LWLock *lock, LWLockMode mode)
 		RESUME_INTERRUPTS();
 
 		LOG_LWDEBUG("LWLockConditionalAcquire", lock, "failed");
-		TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(T_NAME(lock), (int)mode);
 	}
 	else
 	{
 		/* Add lock to list of locks held by this backend */
 		held_lwlocks[num_held_lwlocks].lock = lock;
 		held_lwlocks[num_held_lwlocks++].mode = mode;
-		TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(T_NAME(lock), (int)mode);
 	}
 	return !mustwait;
 }
@@ -1387,7 +1387,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 #endif
 
 			LWLockReportWaitStart(lock);
-			TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), mode);
+			TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), (int)mode);
 
 			for (;;)
 			{
@@ -1405,7 +1405,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 Assert(nwaiters < MAX_BACKENDS);
 			}
 #endif
-			TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
+			TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), (int)mode);
 			LWLockReportWaitEnd();
 
 			LOG_LWDEBUG("LWLockAcquireOrWait", lock, "awakened");
@@ -1435,7 +1435,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 		/* Failed to get lock, so release interrupt holdoff */
 		RESUME_INTERRUPTS();
 		LOG_LWDEBUG("LWLockAcquireOrWait", lock, "failed");
-		TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT_FAIL(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT_FAIL(T_NAME(lock), (int)mode);
 	}
 	else
 	{
@@ -1443,7 +1443,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 		/* Add lock to list of locks held by this backend */
 		held_lwlocks[num_held_lwlocks].lock = lock;
 		held_lwlocks[num_held_lwlocks++].mode = mode;
-		TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT(T_NAME(lock), (int)mode);
 	}
 
 	return !mustwait;
-- 
2.7.4


-- 
Sent via pgsql-hackers 

Re: [HACKERS] dtrace probes

[Amit Kapila]

On Tue, Apr 18, 2017 at 9:38 PM, Jesper Pedersen
 wrote:
> Hi,
>
> The lwlock dtrace probes define LWLockMode as int, and the
> TRACE_POSTGRESQL_LWLOCK methods are called using both a variable and
> constant definition.
>
> This leads to a mix of argument definitions depending on the call site, as
> seen in probes.txt file.
>
> A fix is to explicit cast 'mode' to int such that all call sites will use
> the
>
>  argument #2 4 signed   bytes
>
> definition. Attached patch does this.
>

I think this fix is harmless and has some value in terms of
consistency.  One minor suggestion is that you should leave a space
after typecasting.

- TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
+ TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), (int)mode);

There should be a space like "(int) mode".


> I have verified all dtraces probes for their type, and only the lock__
> methods doesn't aligned with its actual types.
>

Do you see any problem with that?

>
> Depending on the feedback I can add this patch to the open item list in
> order to fix it for PostgreSQL 10.
>

Is there any commit in PG-10 which has caused this behavior?  If not,
then I don't think it should be added to open items of PG-10.



-- 
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

[HACKERS] dtrace probes

[Jesper Pedersen]

Hi,

The lwlock dtrace probes define LWLockMode as int, and the 
TRACE_POSTGRESQL_LWLOCK methods are called using both a variable and 
constant definition.


This leads to a mix of argument definitions depending on the call site, 
as seen in probes.txt file.


A fix is to explicit cast 'mode' to int such that all call sites will 
use the


 argument #2 4 signed   bytes

definition. Attached patch does this.

I have verified all dtraces probes for their type, and only the lock__ 
methods doesn't aligned with its actual types. However, that would 
require a change to probes.d, and therefore PostgreSQL 11 material.


Depending on the feedback I can add this patch to the open item list in 
order to fix it for PostgreSQL 10.


Best regards,
 Jesper
>From d6f5c119c057c7ff8c84f88bb4122a1ca245a7d4 Mon Sep 17 00:00:00 2001
From: jesperpedersen 
Date: Tue, 18 Apr 2017 11:44:18 -0400
Subject: [PATCH] Explicit cast LWLockMode, which an enum, to int in order to
 match the dtrace definition of the lwlock methods. Thereby all call sites
 will have the same definition instead of a mix between signed and unsigned.

Author: Jesper Pedersen 
---
 src/backend/storage/lmgr/lwlock.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 3e13394..abf5fbb 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1226,7 +1226,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode)
 #endif
 
 		LWLockReportWaitStart(lock);
-		TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), (int)mode);
 
 		for (;;)
 		{
@@ -1248,7 +1248,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode)
 		}
 #endif
 
-		TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), (int)mode);
 		LWLockReportWaitEnd();
 
 		LOG_LWDEBUG("LWLockAcquire", lock, "awakened");
@@ -1257,7 +1257,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode)
 		result = false;
 	}
 
-	TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(lock), mode);
+	TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(lock), (int)mode);
 
 	/* Add lock to list of locks held by this backend */
 	held_lwlocks[num_held_lwlocks].lock = lock;
@@ -1308,14 +1308,14 @@ LWLockConditionalAcquire(LWLock *lock, LWLockMode mode)
 		RESUME_INTERRUPTS();
 
 		LOG_LWDEBUG("LWLockConditionalAcquire", lock, "failed");
-		TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(T_NAME(lock), (int)mode);
 	}
 	else
 	{
 		/* Add lock to list of locks held by this backend */
 		held_lwlocks[num_held_lwlocks].lock = lock;
 		held_lwlocks[num_held_lwlocks++].mode = mode;
-		TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(T_NAME(lock), (int)mode);
 	}
 	return !mustwait;
 }
@@ -1387,7 +1387,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 #endif
 
 			LWLockReportWaitStart(lock);
-			TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), mode);
+			TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), (int)mode);
 
 			for (;;)
 			{
@@ -1405,7 +1405,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 Assert(nwaiters < MAX_BACKENDS);
 			}
 #endif
-			TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
+			TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), (int)mode);
 			LWLockReportWaitEnd();
 
 			LOG_LWDEBUG("LWLockAcquireOrWait", lock, "awakened");
@@ -1435,7 +1435,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 		/* Failed to get lock, so release interrupt holdoff */
 		RESUME_INTERRUPTS();
 		LOG_LWDEBUG("LWLockAcquireOrWait", lock, "failed");
-		TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT_FAIL(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT_FAIL(T_NAME(lock), (int)mode);
 	}
 	else
 	{
@@ -1443,7 +1443,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 		/* Add lock to list of locks held by this backend */
 		held_lwlocks[num_held_lwlocks].lock = lock;
 		held_lwlocks[num_held_lwlocks++].mode = mode;
-		TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT(T_NAME(lock), mode);
+		TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT(T_NAME(lock), (int)mode);
 	}
 
 	return !mustwait;
-- 
2.7.4

/usr/local/bin/postgres postgresql:clog__checkpoint__start [sema 0xe2351a]
  location #1 0x4fc706
argument #1 1 signed   bytes @ 0
  location #2 0x4fc72d
argument #1 1 signed   bytes @ 1
/usr/local/bin/postgres postgresql:clog__checkpoint__done [sema 0xe2351c]
  location #1 0x4fc725
argument #1 1 signed   bytes @ 0
  location #2 0x4fc74c
argument #1 1 signed   bytes @ 1
/usr/local/bin/postgres postgresql:multixact__checkpoint__start [sema 0xe23522]
  location #1 0x500d07
argument #1 1 signed   bytes @ 0
  location #2 0x500dbc
argument #1 1 signed   bytes @ 1
/usr/local/bin/postgres postgresql:multixact__checkpoint__done [sema 0xe23524]
  

Re: [HACKERS] DTrace build dependency rules

[Robert Haas]

On Tue, Oct 13, 2015 at 3:47 PM, Robert Haas  wrote:
> On Tue, Aug 18, 2015 at 12:08 PM, Alvaro Herrera
>  wrote:
>> Robert Haas wrote:
>>> On Sat, Aug 15, 2015 at 6:45 PM, Mark Johnston  wrote:
>>
>>> > The bug is in src/backend/Makefile. probes.o, the dtrace(1)-generated
>>> > object file, depends on the objfiles.txt for each of the backend
>>> > subdirs. These files depend in turn on the object files themselves; if
>>> > objfiles.txt is out of date with respect to one of its object files, the
>>> > mtime of objfiles.txt is updated with "touch" (see backend/common.mk).
>>> > The problem is that dtrace -G, which runs at the end of the build,
>>> > modifies a number of object files (it overwrites their probe sites with
>>> > NOPs), thus making their corresponding objfiles.txt out of date. Then,
>>> > when "make install" traverses the backend subdirs, it updates
>>> > objfiles.txt, which causes probes.o to be rebuilt, resulting in an error
>>> > from dtrace(1).
>>>
>>> Gosh, that's pretty ugly.  I would have thought it would be a real
>>> no-no to update the .o file once it got generated.  If nothing else, a
>>> modification to the .c file concurrent with a make invocation might
>>> lead to the .o not getting rebuilt the next time make is run.
>>
>> I had the same thought, and wondered for a bit whether we should instead
>> have the compilation rules produce some intermediate file (prior to
>> dtrace fumbling), then emit the .o from dtrace -G.  OTOH this might be
>> more trouble than is worth for a feature that doesn't see a lot of use.
>
> Given the lack of further interest from the PostgreSQL community,
> that's my guess.  I've pushed this patch to master; let's see if we
> get any complaints.  If it makes life better for FreeBSD without
> making life worse for anyone else, I suppose we might as well do it
> until something better comes along.

Per http://www.postgresql.org/message-id/24541.1444928...@sss.pgh.pa.us
I have had to revert this patch, because it's breaking parallel builds
even for non-dtrace users.

Sorry for the inconvenience.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] DTrace build dependency rules

[Robert Haas]

On Tue, Aug 18, 2015 at 12:08 PM, Alvaro Herrera
 wrote:
> Robert Haas wrote:
>> On Sat, Aug 15, 2015 at 6:45 PM, Mark Johnston  wrote:
>
>> > The bug is in src/backend/Makefile. probes.o, the dtrace(1)-generated
>> > object file, depends on the objfiles.txt for each of the backend
>> > subdirs. These files depend in turn on the object files themselves; if
>> > objfiles.txt is out of date with respect to one of its object files, the
>> > mtime of objfiles.txt is updated with "touch" (see backend/common.mk).
>> > The problem is that dtrace -G, which runs at the end of the build,
>> > modifies a number of object files (it overwrites their probe sites with
>> > NOPs), thus making their corresponding objfiles.txt out of date. Then,
>> > when "make install" traverses the backend subdirs, it updates
>> > objfiles.txt, which causes probes.o to be rebuilt, resulting in an error
>> > from dtrace(1).
>>
>> Gosh, that's pretty ugly.  I would have thought it would be a real
>> no-no to update the .o file once it got generated.  If nothing else, a
>> modification to the .c file concurrent with a make invocation might
>> lead to the .o not getting rebuilt the next time make is run.
>
> I had the same thought, and wondered for a bit whether we should instead
> have the compilation rules produce some intermediate file (prior to
> dtrace fumbling), then emit the .o from dtrace -G.  OTOH this might be
> more trouble than is worth for a feature that doesn't see a lot of use.

Given the lack of further interest from the PostgreSQL community,
that's my guess.  I've pushed this patch to master; let's see if we
get any complaints.  If it makes life better for FreeBSD without
making life worse for anyone else, I suppose we might as well do it
until something better comes along.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] DTrace build dependency rules

[Robert Haas]

On Sat, Aug 15, 2015 at 6:45 PM, Mark Johnston ma...@freebsd.org wrote:
 There seems to be a bug in the make rules when DTrace is enabled. It
 causes dtrace -G to be invoked twice when building PostgreSQL as a
 FreeBSD port: once during the build itself, and once during
 installation. For a long time this has been worked around on FreeBSD
 with a change to libdtrace itself, but this workaround is proving
 problematic and I'd like to fix the problem properly. I'm not sure
 whether the problem has been observed on other operating systems that
 support DTrace.

 The bug is in src/backend/Makefile. probes.o, the dtrace(1)-generated
 object file, depends on the objfiles.txt for each of the backend
 subdirs. These files depend in turn on the object files themselves; if
 objfiles.txt is out of date with respect to one of its object files, the
 mtime of objfiles.txt is updated with touch (see backend/common.mk).
 The problem is that dtrace -G, which runs at the end of the build,
 modifies a number of object files (it overwrites their probe sites with
 NOPs), thus making their corresponding objfiles.txt out of date. Then,
 when make install traverses the backend subdirs, it updates
 objfiles.txt, which causes probes.o to be rebuilt, resulting in an error
 from dtrace(1).

Gosh, that's pretty ugly.  I would have thought it would be a real
no-no to update the .o file once it got generated.  If nothing else, a
modification to the .c file concurrent with a make invocation might
lead to the .o not getting rebuilt the next time make is run.

 The attached patch fixes the problem by having probes.o depend on object
 files directly, rather than on objfiles.txt. I've tested it with
 PostgreSQL 9.0-9.4 on FreeBSD CURRENT.

I don't see a particular reason not to make this change, though.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] DTrace build dependency rules

[Alvaro Herrera]

Robert Haas wrote:
 On Sat, Aug 15, 2015 at 6:45 PM, Mark Johnston ma...@freebsd.org wrote:

  The bug is in src/backend/Makefile. probes.o, the dtrace(1)-generated
  object file, depends on the objfiles.txt for each of the backend
  subdirs. These files depend in turn on the object files themselves; if
  objfiles.txt is out of date with respect to one of its object files, the
  mtime of objfiles.txt is updated with touch (see backend/common.mk).
  The problem is that dtrace -G, which runs at the end of the build,
  modifies a number of object files (it overwrites their probe sites with
  NOPs), thus making their corresponding objfiles.txt out of date. Then,
  when make install traverses the backend subdirs, it updates
  objfiles.txt, which causes probes.o to be rebuilt, resulting in an error
  from dtrace(1).
 
 Gosh, that's pretty ugly.  I would have thought it would be a real
 no-no to update the .o file once it got generated.  If nothing else, a
 modification to the .c file concurrent with a make invocation might
 lead to the .o not getting rebuilt the next time make is run.

I had the same thought, and wondered for a bit whether we should instead
have the compilation rules produce some intermediate file (prior to
dtrace fumbling), then emit the .o from dtrace -G.  OTOH this might be
more trouble than is worth for a feature that doesn't see a lot of use.

  The attached patch fixes the problem by having probes.o depend on object
  files directly, rather than on objfiles.txt. I've tested it with
  PostgreSQL 9.0-9.4 on FreeBSD CURRENT.
 
 I don't see a particular reason not to make this change, though.

I wanted to test it on Linux yesterday but didn't get any further than
installing a couple of packages.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


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

[HACKERS] DTrace build dependency rules

[Mark Johnston]

Hi,

There seems to be a bug in the make rules when DTrace is enabled. It
causes dtrace -G to be invoked twice when building PostgreSQL as a
FreeBSD port: once during the build itself, and once during
installation. For a long time this has been worked around on FreeBSD
with a change to libdtrace itself, but this workaround is proving
problematic and I'd like to fix the problem properly. I'm not sure
whether the problem has been observed on other operating systems that
support DTrace.

The bug is in src/backend/Makefile. probes.o, the dtrace(1)-generated
object file, depends on the objfiles.txt for each of the backend
subdirs. These files depend in turn on the object files themselves; if
objfiles.txt is out of date with respect to one of its object files, the
mtime of objfiles.txt is updated with touch (see backend/common.mk).
The problem is that dtrace -G, which runs at the end of the build,
modifies a number of object files (it overwrites their probe sites with
NOPs), thus making their corresponding objfiles.txt out of date. Then,
when make install traverses the backend subdirs, it updates
objfiles.txt, which causes probes.o to be rebuilt, resulting in an error
from dtrace(1).

The attached patch fixes the problem by having probes.o depend on object
files directly, rather than on objfiles.txt. I've tested it with
PostgreSQL 9.0-9.4 on FreeBSD CURRENT.

Thanks!
-Mark
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 98b978f..b1e3969 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -180,8 +180,8 @@ $(top_builddir)/src/include/utils/probes.h: utils/probes.h
 	$(LN_S) ../../../$(subdir)/utils/probes.h .
 
 
-utils/probes.o: utils/probes.d $(SUBDIROBJS)
-	$(DTRACE) $(DTRACEFLAGS) -C -G -s $(call expand_subsys,$^) -o $@
+utils/probes.o: utils/probes.d $(call expand_subsys,$(SUBDIROBJS))
+	$(DTRACE) $(DTRACEFLAGS) -C -G -s $ $(call expand_subsys,$(SUBDIROBJS)) -o $@
 
 
 ##

-- 
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] DTrace compiler warnings

[Bernd Helmle]

--On 14. November 2009 15:25:25 +0100 Zdenek Kotala zdenek.kot...@sun.com 
wrote:



Hmm, const is also problem on solaris. dtrace generates probe.h file and
eats const. It generates following noise on solaris build:

postgres.c, line 554: warning: argument #1 is incompatible with
prototype:
prototype: pointer to char : ../../../src/include/utils/probes.h,
line 582
argument : pointer to const char

IIRC, it was discussed. I cc Robert. He should know answer why const is
ignored.


Have you dug into this further?

--
Thanks

Bernd

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

[HACKERS] DTrace compiler warnings

[Bernd Helmle]

I'm just seeing these kind of compiler warnings in current HEAD:

pgstat.c: In function ‘pgstat_report_activity’:
pgstat.c:2276: warning: passing argument 1 of 
‘__dtrace_probe$postgresql$statement__status$v1$63686172202a’ discards 
qualifiers from pointer target type


There are more of them (e.g. postgres.c), all passing a const char pointer.

Platform is Snow Leopard, 10.6.2, gcc 4.2.1

--
Thanks

Bernd

--
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] DTrace compiler warnings

[Zdenek Kotala]

Hmm, const is also problem on solaris. dtrace generates probe.h file and
eats const. It generates following noise on solaris build:

postgres.c, line 554: warning: argument #1 is incompatible with
prototype:
prototype: pointer to char : ../../../src/include/utils/probes.h,
line 582
argument : pointer to const char

IIRC, it was discussed. I cc Robert. He should know answer why const is
ignored.

Zdenek


Bernd Helmle píše v so 14. 11. 2009 v 14:54 +0100:
 I'm just seeing these kind of compiler warnings in current HEAD:
 
 pgstat.c: In function ‘pgstat_report_activity’:
 pgstat.c:2276: warning: passing argument 1 of 
 ‘__dtrace_probe$postgresql$statement__status$v1$63686172202a’ discards 
 qualifiers from pointer target type
 
 There are more of them (e.g. postgres.c), all passing a const char pointer.
 
 Platform is Snow Leopard, 10.6.2, gcc 4.2.1
 
 -- 
 Thanks
 
   Bernd
 



-- 
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] Dtrace probes documentation

[Frank Ch. Eigler]

Tom Lane t...@sss.pgh.pa.us writes:

 [...]
 See http://blog.endpoint.com/2009/05/postgresql-with-systemtap.html for
 details. Perhaps it's worth noting in the documentation that SystemTap users
 will need to use the double-underscore version?

 I think a better solution is to persuade the Systemtap guys that they
 ought to accept the single-hyphen spelling.  [...]

Will do: http://sourceware.org/PR10225.

- FChE

-- 
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] Dtrace probes documentation

[Tom Lane]

Joshua Tolley eggyk...@gmail.com writes:
 On Thu, May 28, 2009 at 06:28:14PM -0400, Tom Lane wrote:
 Read 26.4.3 and .4.  I don't know why they have this bizarre set of
 conventions, but the single-hyphen version is the spelling
 most visible to end users.

 I thought it might be something like that. I've been playing with SystemTap,
 and found that only the double-underscore version works for ... well, 
 anything.
 See http://blog.endpoint.com/2009/05/postgresql-with-systemtap.html for
 details. Perhaps it's worth noting in the documentation that SystemTap users
 will need to use the double-underscore version?

I think a better solution is to persuade the Systemtap guys that they
ought to accept the single-hyphen spelling.  I've put in a request for
that, we'll see what they think ...

regards, tom lane

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

[HACKERS] Dtrace probes documentation

[Joshua Tolley]

The dtrace probes documentation [1] spells each probe name with dashes
(transaction-start, transaction-commit, etc.). Yet as far as I can see,
dtrace only works if you spell the probe names with double underscores
(transaction__start, transaction__commit, etc.). Why the discrepancy?
Obvious patch attached, in case this needs to be changed.

- Josh / eggyknap

1: http://www.postgresql.org/docs/8.4/static/dynamic-trace.html

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index ea8b017..672a1fe 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1070,95 +1070,95 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS procpid,
tbody
 
 row
- entrytransaction-start/entry
+ entrytransaction__start/entry
  entry(LocalTransactionId)/entry
  entryProbe that fires at the start of a new transaction.
   arg0 is the transaction id./entry
 /row
 row
- entrytransaction-commit/entry
+ entrytransaction__commit/entry
  entry(LocalTransactionId)/entry
  entryProbe that fires when a transaction completes successfully.
   arg0 is the transaction id./entry
 /row
 row
- entrytransaction-abort/entry
+ entrytransaction__abort/entry
  entry(LocalTransactionId)/entry
  entryProbe that fires when a transaction completes unsuccessfully.
   arg0 is the transaction id./entry
 /row
 row
- entryquery-start/entry
+ entryquery__start/entry
  entry(const char *)/entry
  entryProbe that fires when the processing of a query is started.
   arg0 is the query string./entry
 /row
 row
- entryquery-done/entry
+ entryquery__done/entry
  entry(const char *)/entry
  entryProbe that fires when the processing of a query is complete.
   arg0 is the query string./entry
 /row
 row
- entryquery-parse-start/entry
+ entryquery__parse__start/entry
  entry(const char *)/entry
  entryProbe that fires when the parsing of a query is started.
   arg0 is the query string./entry
 /row
 row
- entryquery-parse-done/entry
+ entryquery__parse__done/entry
  entry(const char *)/entry
  entryProbe that fires when the parsing of a query is complete.
   arg0 is the query string./entry
 /row
 row
- entryquery-rewrite-start/entry
+ entryquery__rewrite__start/entry
  entry(const char *)/entry
  entryProbe that fires when the rewriting of a query is started.
   arg0 is the query string./entry
 /row
 row
- entryquery-rewrite-done/entry
+ entryquery__rewrite__done/entry
  entry(const char *)/entry
  entryProbe that fires when the rewriting of a query is complete.
   arg0 is the query string./entry
 /row
 row
- entryquery-plan-start/entry
+ entryquery__plan__start/entry
  entry()/entry
  entryProbe that fires when the planning of a query is started./entry
 /row
 row
- entryquery-plan-done/entry
+ entryquery__plan__done/entry
  entry()/entry
  entryProbe that fires when the planning of a query is complete./entry
 /row
 row
- entryquery-execute-start/entry
+ entryquery__execute__start/entry
  entry()/entry
  entryProbe that fires when the execution of a query is started./entry
 /row
 row
- entryquery-execute-done/entry
+ entryquery__execute__done/entry
  entry()/entry
  entryProbe that fires when the execution of a query is complete./entry
 /row
 row
- entrystatement-status/entry
+ entrystatement__status/entry
  entry(const char *)/entry
  entryProbe that fires anytime the server process updates its
   structnamepg_stat_activity/.structfieldcurrent_query/ status.
   arg0 is the new status string./entry
 /row
 row
- entrycheckpoint-start/entry
+ entrycheckpoint__start/entry
  entry(int)/entry
  entryProbe that fires when a checkpoint is started.
   arg0 holds the bitwise flags used to distinguish different checkpoint
   types, such as shutdown, immediate or force./entry
 /row
 row
- entrycheckpoint-done/entry
+ entrycheckpoint__done/entry
  entry(int, int, int, int, int)/entry
  entryProbe that fires when a checkpoint is complete.
   (The probes listed next fire in sequence during checkpoint processing.)
@@ -1167,20 +1167,20 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS procpid,
   removed and recycled respectively./entry
 /row
 row
- entryclog-checkpoint-start/entry
+ entryclog__checkpoint__start/entry
  entry(bool)/entry
  entryProbe that fires when the CLOG portion of a checkpoint is started.
   arg0 is true for normal checkpoint, false for shutdown
   checkpoint./entry
 /row
 row
- entryclog-checkpoint-done/entry
+ entryclog__checkpoint__done/entry
  entry(bool)/entry
  entryProbe that fires when the CLOG portion of a checkpoint is
-  

Re: [HACKERS] Dtrace probes documentation

[Tom Lane]

Joshua Tolley eggyk...@gmail.com writes:
 The dtrace probes documentation [1] spells each probe name with dashes
 (transaction-start, transaction-commit, etc.). Yet as far as I can see,
 dtrace only works if you spell the probe names with double underscores
 (transaction__start, transaction__commit, etc.). Why the discrepancy?

Read 26.4.3 and .4.  I don't know why they have this bizarre set of
conventions, but the single-hyphen version is the spelling
most visible to end users.

regards, tom lane

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

Re: [HACKERS] Dtrace probes documentation

[Joshua Tolley]

On Thu, May 28, 2009 at 06:28:14PM -0400, Tom Lane wrote:
 Joshua Tolley eggyk...@gmail.com writes:
  The dtrace probes documentation [1] spells each probe name with dashes
  (transaction-start, transaction-commit, etc.). Yet as far as I can see,
  dtrace only works if you spell the probe names with double underscores
  (transaction__start, transaction__commit, etc.). Why the discrepancy?
 
 Read 26.4.3 and .4.  I don't know why they have this bizarre set of
 conventions, but the single-hyphen version is the spelling
 most visible to end users.

I thought it might be something like that. I've been playing with SystemTap,
and found that only the double-underscore version works for ... well, anything.
See http://blog.endpoint.com/2009/05/postgresql-with-systemtap.html for
details. Perhaps it's worth noting in the documentation that SystemTap users
will need to use the double-underscore version?

- Josh / eggyknap


signature.asc
Description: Digital signature

Re: [HACKERS] DTrace probes broken in HEAD on Solaris?

[Tom Lane]

Robert Lor robert@sun.com writes:
 Tom Lane wrote:
 [ complaining about disabled probes not being no-ops ]

 1) Only use if (foo_ENABLED()) test  for probes with expensive function 
 call/computation in arguments. This will keep the code clean, and we can 
 document this in the Definine New Probes section in the online doc.
 ...
 I prefer option 1 the most and 3 the least.

I got the same advice from the systemtap people, so we'll do it that
way.

regards, tom lane

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

Re: [HACKERS] DTrace probes broken in HEAD on Solaris?

[Zdenek Kotala]

Dne 24.03.09 22:31, Robert Lor napsal(a):

I think the is-enabled test will address the issues you encountered. I 
see a few alternative to fixing this:


1) Only use if (foo_ENABLED()) test  for probes with expensive function 
call/computation in arguments. This will keep the code clean, and we can 
document this in the Definine New Probes section in the online doc.


2) Add the if(foo_ENABLED()) test to all probes manually and make this a 
requirement for all future probes. This makes the check explicit and 
avoid confusion.


3) Post-process probes.h so if(foo_ENABLED()) test is added to every 
probe. We're doing some post-processing now by pre-pending TRACE_ to the 
macros with a sed script. Personally, I don't like doing complex 
post-processing of output from another tool because the script can break 
if for some reason DTrace's output is changed.


I prefer option 1 the most and 3 the least.


I prefer also option 1. In many cases if(foo_ENABLED) has same or bigger 
 performance penalty like disabled probe itself.


Zdenek

--
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] DTrace probes broken in HEAD on Solaris?

[Robert Lor]

Tom Lane wrote:

Zdenek Kotala zdenek.kot...@sun.com writes:
  
Answer why it happens when probes are disabled is, that for user 
application there are piece of code which prepare DTrace probes 
arguments which will be passed into kernel DTrace modul. This code has 
performance penalty which depends on number of arguments.



The other thing I don't like is that this implementation exposes people
to any bugs that may exist in the probe arguments, *even when they don't
have any tracing turned on*.  (Again, we had two different instances of
that last week, so don't bother arguing that it doesn't happen.)
  


Yes, the above scenario can occur when compiling with DTrace (passing 
--enable-dtrace to configure) even when the probes are not enabled. It 
won't be an issue if you don't compile with DTrace though as the macros 
are converted to no-ops. Hopefully, any bugs in the probe arguments 
would be caught during testing ;-)  


Both of these considerations are strong arguments for not building
production installations with --enable-dtrace, just as we don't
encourage people to build for production with --enable-cassert.

But of course part of the argument for dtrace support is that people
would like to have such probing available in production installations.

What I've found out about this is that for each probe macro, DTrace
also defines a foo_ENABLED() macro that can be used like this:

if (foo_ENABLED())
foo(...);

I think what we should do about these considerations is fix our macro
definitions so that the if(ENABLED()) test is built into the macros.
I'm not sure what this will require ... probably some post-processing
of the probes.h file ... but if we don't do it we're going to keep
getting bit.
  
I was contemplating on using the is-enabled test, but when checking the 
arguments to all the probes, they were quite simple (except the 
relpath() call which is now removed).


I think the is-enabled test will address the issues you encountered. I 
see a few alternative to fixing this:


1) Only use if (foo_ENABLED()) test  for probes with expensive function 
call/computation in arguments. This will keep the code clean, and we can 
document this in the Definine New Probes section in the online doc.


2) Add the if(foo_ENABLED()) test to all probes manually and make this a 
requirement for all future probes. This makes the check explicit and 
avoid confusion.


3) Post-process probes.h so if(foo_ENABLED()) test is added to every 
probe. We're doing some post-processing now by pre-pending TRACE_ to the 
macros with a sed script. Personally, I don't like doing complex 
post-processing of output from another tool because the script can break 
if for some reason DTrace's output is changed.


I prefer option 1 the most and 3 the least.


-Robert



--
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] DTrace probes broken in HEAD on Solaris?

[Greg Stark]

On Tue, Mar 24, 2009 at 9:31 PM, Robert Lor robert@sun.com wrote:

 I think the is-enabled test will address the issues you encountered. I see a
 few alternative to fixing this:


Another option is to impose a policy that all arguments to probes must
be simple local variables -- no expressions.

I'm starting to think that would be the better option and more in tune
with the dtrace way. Introducing the _ENABLED check defeats the whole
performance aim of dtrace that when it's disabled it introduces no
overhead. But using the _ENABLED macro only sparsely risks missing
some expression somewhere which looks innocuous but isn't.

I wonder if there's a gcc extension that would let us check if a macro
parameter is a simple variable or expression. That would let us
enforce the polilcy strictly at build-time.

-- 
greg

-- 
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] DTrace probes broken in HEAD on Solaris?

[Martijn van Oosterhout]

On Tue, Mar 24, 2009 at 09:49:50PM +, Greg Stark wrote:
 I wonder if there's a gcc extension that would let us check if a macro
 parameter is a simple variable or expression. That would let us
 enforce the polilcy strictly at build-time.

Not really a GCC extension, but you could have the macro check that it
can take the address of the arguments, which amounts to almost the same
thing. It only doesn't work on constants.

Have a nice day,
-- 
Martijn van Oosterhout   klep...@svana.org   http://svana.org/kleptog/
 Please line up in a tree and maintain the heap invariant while 
 boarding. Thank you for flying nlogn airlines.


signature.asc
Description: Digital signature

Re: [HACKERS] DTrace probes broken in HEAD on Solaris?

[Greg Stark]

On Tue, Mar 24, 2009 at 10:12 PM, Martijn van Oosterhout
klep...@svana.org wrote:
 Not really a GCC extension, but you could have the macro check that it
 can take the address of the arguments, which amounts to almost the same
 thing. It only doesn't work on constants.

No, there are all kinds of complex expressions which are lvalues --
anything ending in a pointer dereference for example, which is
precisely the most likely kind of expression to introduce seg fault if
something is unexpectedly null
.

-- 
greg

-- 
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] DTrace probes broken in HEAD on Solaris?

[Martijn van Oosterhout]

On Tue, Mar 24, 2009 at 10:18:08PM +, Greg Stark wrote:
 On Tue, Mar 24, 2009 at 10:12 PM, Martijn van Oosterhout
 klep...@svana.org wrote:
  Not really a GCC extension, but you could have the macro check that it
  can take the address of the arguments, which amounts to almost the same
  thing. It only doesn't work on constants.
 
 No, there are all kinds of complex expressions which are lvalues --
 anything ending in a pointer dereference for example, which is
 precisely the most likely kind of expression to introduce seg fault if
 something is unexpectedly null

Sorry, I meant to say that the compiler could determine the address at
compile time, something like:

__builtin_constant_p( !(__x)) )

Have a nice day,
-- 
Martijn van Oosterhout   klep...@svana.org   http://svana.org/kleptog/
 Please line up in a tree and maintain the heap invariant while 
 boarding. Thank you for flying nlogn airlines.


signature.asc
Description: Digital signature

Re: [HACKERS] DTrace probes broken in HEAD on Solaris?

[Greg Stark]

On Tue, Mar 24, 2009 at 10:53 PM, Martijn van Oosterhout
klep...@svana.org wrote:
 Sorry, I meant to say that the compiler could determine the address at
 compile time, something like:

 __builtin_constant_p( !(__x)) )

Hm, that's a better idea.

How about something like

(__builtin_constant_p(__x) || __builtin_constant_p(!(__x)))

This would still pass expressions like foo[x] even if x might be
out-of-bounds or foo might be pointed to a freed object or something
like that though.




-- 
greg

-- 
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] DTrace probes broken in HEAD on Solaris?

[Tom Lane]

Greg Stark st...@enterprisedb.com writes:
 On Tue, Mar 24, 2009 at 9:31 PM, Robert Lor robert@sun.com wrote:
 I think the is-enabled test will address the issues you encountered. I see a
 few alternative to fixing this:

 Another option is to impose a policy that all arguments to probes must
 be simple local variables -- no expressions.

IOW, if you need to trace on an expression, you have to calculate it
whether or not ENABLE_DTRACE is even defined?  This doesn't seem to
me that it solves anything.  The cases that are interesting are where a
probe needs a value that otherwise wouldn't be needed.

regards, tom lane

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

Re: [HACKERS] DTrace probes broken in HEAD on Solaris?

[Zdenek Kotala]

Dne 17.03.09 19:49, Tom Lane napsal(a):

Zdenek Kotala zdenek.kot...@sun.com writes:
Answer why it happens when probes are disabled is, that for user 
application there are piece of code which prepare DTrace probes 
arguments which will be passed into kernel DTrace modul. This code has 
performance penalty which depends on number of arguments.


More specifically, it seems that DTrace is designed so that it evaluates
all the arguments to a probe macro before it decides whether to actually
take the trap or not.


yeah, it is valid for USDT. I guess main reason for this is 
implementation is that DTrace does not have idea how compiler generates 
code and where he can find arguments. Only what it knows is how to call 
a probe and this call is noped.



This seems to me to be a pretty bad/surprising behavior; it's not the
way that our Assert macros work, for example.  There's a performance
issue, which would probably only be brutally obvious if you had an
expensive function in the arguments.  (Before you say no one would do
that, note the relpath() calls I was complaining about last week.)
But we've been muttering about dropping probes into some extremely
hot hot-spots, like spinlock acquisition, and even a few more
instructions to copy local variables could make a difference there.


yeah, spinlock is problem, but on other side the probes are only in the 
branch when spinlock is not free and sleep anyway. And LWLOCK_ACQUIRE 
probe has minimal impact if you compare it with rest of LWLockAcquire 
function.



The other thing I don't like is that this implementation exposes people
to any bugs that may exist in the probe arguments, *even when they don't
have any tracing turned on*.  (Again, we had two different instances of
that last week, so don't bother arguing that it doesn't happen.)


I don't accept your argument here. Better if it fails every time not 
only when you enable the probe. DTrace is designated to be safe on 
production machine. It is not good to shut down postgresql server in 
production...



Both of these considerations are strong arguments for not building
production installations with --enable-dtrace, just as we don't
encourage people to build for production with --enable-cassert.

But of course part of the argument for dtrace support is that people
would like to have such probing available in production installations.

What I've found out about this is that for each probe macro, DTrace
also defines a foo_ENABLED() macro that can be used like this:

if (foo_ENABLED())
foo(...);

I think what we should do about these considerations is fix our macro
definitions so that the if(ENABLED()) test is built into the macros.
I'm not sure what this will require ... probably some post-processing
of the probes.h file ... but if we don't do it we're going to keep
getting bit.


I like this idea to used if(foo_ENABLED), but maybe we should used it 
only when args evaluation is more complicated?


I'm not sure if it is good idea to modify macros definition.

See how macro definition look like:

#define TRACE_POSTGRESQL_LWLOCK_ACQUIRE(arg0, arg1) \
__dtrace_postgresql___lwlock__acquire(arg0, arg1)
#define TRACE_POSTGRESQL_LWLOCK_ACQUIRE_ENABLED() \
__dtraceenabled_postgresql___lwlock__acquire()

Maybe add something like this at the end of probe.h:

#define TRACE_POSTGRESQL_LWLOCK_ACQUIRE_COND(arg0, arg1) \
if( TRACE_POSTGRESQL_LWLOCK_ACQUIRE_ENABLED() ) \
TRACE_POSTGRESQL_LWLOCK_ACQUIRE(arg0, arg1)



Zdenek

-
--
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] DTrace probes broken in HEAD on Solaris?

[Zdenek Kotala]

Sorry for late answer, I was on vacation last week. I see that you 
already fix a problem.


Answer why it happens when probes are disabled is, that for user 
application there are piece of code which prepare DTrace probes 
arguments which will be passed into kernel DTrace modul. This code has 
performance penalty which depends on number of arguments.


See the code:

FlushBuffer+0x69:   call   +0x128c6 smgropen
FlushBuffer+0x6e:   addl   $0x10,%esp
FlushBuffer+0x71:   movl   %eax,0xc(%ebp)
FlushBuffer+0x74:   subl   $0x4,%esp

FlushBuffer+0x77:   movl   0xc(%ebp),%eax
FlushBuffer+0x7a:   pushl  0x8(%eax)
FlushBuffer+0x7d:   pushl  0x4(%eax)
FlushBuffer+0x80:   pushl  (%eax)
FlushBuffer+0x82:   nop
FlushBuffer+0x83:   nop
FlushBuffer+0x84:   nop
FlushBuffer+0x85:   nop
FlushBuffer+0x86:   nop
FlushBuffer+0x87:   addl   $0x10,%esp


nops reserve space for probe.


For better explanation see:

http://blogs.sun.com/ahl/entry/user_land_tracing_gets_better


Zdenek


Dne 13.03.09 15:15, Tom Lane napsal(a):

Most of the Solaris buildfarm members are unhappy this morning.
It looks like the ones that are busted have --enable-dtrace,
which points the finger at the dtrace probe changes I made yesterday:
http://archives.postgresql.org/pgsql-committers/2009-03/msg00079.php

Those changes work on Linux and OS X, so it's not clear what I did
wrong.  Can anyone with a Solaris machine poke into it, at least
to the extent of finding out where it's dumping core?

regards, tom lane




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

Re: [HACKERS] DTrace probes broken in HEAD on Solaris?

[Tom Lane]

Zdenek Kotala zdenek.kot...@sun.com writes:
 Answer why it happens when probes are disabled is, that for user 
 application there are piece of code which prepare DTrace probes 
 arguments which will be passed into kernel DTrace modul. This code has 
 performance penalty which depends on number of arguments.

More specifically, it seems that DTrace is designed so that it evaluates
all the arguments to a probe macro before it decides whether to actually
take the trap or not.

This seems to me to be a pretty bad/surprising behavior; it's not the
way that our Assert macros work, for example.  There's a performance
issue, which would probably only be brutally obvious if you had an
expensive function in the arguments.  (Before you say no one would do
that, note the relpath() calls I was complaining about last week.)
But we've been muttering about dropping probes into some extremely
hot hot-spots, like spinlock acquisition, and even a few more
instructions to copy local variables could make a difference there.

The other thing I don't like is that this implementation exposes people
to any bugs that may exist in the probe arguments, *even when they don't
have any tracing turned on*.  (Again, we had two different instances of
that last week, so don't bother arguing that it doesn't happen.)

Both of these considerations are strong arguments for not building
production installations with --enable-dtrace, just as we don't
encourage people to build for production with --enable-cassert.

But of course part of the argument for dtrace support is that people
would like to have such probing available in production installations.

What I've found out about this is that for each probe macro, DTrace
also defines a foo_ENABLED() macro that can be used like this:

if (foo_ENABLED())
foo(...);

I think what we should do about these considerations is fix our macro
definitions so that the if(ENABLED()) test is built into the macros.
I'm not sure what this will require ... probably some post-processing
of the probes.h file ... but if we don't do it we're going to keep
getting bit.

Comments?

regards, tom lane

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

[HACKERS] DTrace probes broken in HEAD on Solaris?

[Tom Lane]

Most of the Solaris buildfarm members are unhappy this morning.
It looks like the ones that are busted have --enable-dtrace,
which points the finger at the dtrace probe changes I made yesterday:
http://archives.postgresql.org/pgsql-committers/2009-03/msg00079.php

Those changes work on Linux and OS X, so it's not clear what I did
wrong.  Can anyone with a Solaris machine poke into it, at least
to the extent of finding out where it's dumping core?

regards, tom lane

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

Re: [HACKERS] DTrace doc patch for new probes in 8.4

[Bruce Momjian]

Patch applied.  Thanks.

---


Robert Lor wrote:
 Attached is the doc patch for the recently added probes.
 
 -Robert
 

 Index: doc/src/sgml/monitoring.sgml
 ===
 RCS file: /projects/cvsroot/pgsql/doc/src/sgml/monitoring.sgml,v
 retrieving revision 1.63
 diff -u -3 -p -r1.63 monitoring.sgml
 --- doc/src/sgml/monitoring.sgml  11 Nov 2008 20:06:21 -  1.63
 +++ doc/src/sgml/monitoring.sgml  26 Feb 2009 22:18:31 -
 @@ -1009,23 +1009,25 @@ SELECT pg_stat_get_backend_pid(s.backend
 productnamePostgreSQL/productname provides facilities to support
 dynamic tracing of the database server. This allows an external
 utility to be called at specific points in the code and thereby trace
 -   execution.  Currently, this facility is primarily intended for use by
 -   database developers, as it requires substantial familiarity with the code.
 +   execution.
/para
  
para
 -   A number of probes or trace points are already inserted
 -   into the source code.  By default these probes are not compiled into the
 +   A number of probes or trace points are already inserted into the source
 +   code. These probes are intended to be used by database developers and
 +   administrators. By default the probes are not compiled into the
 binary, and the user needs to explicitly tell the configure script to make
 the probes available in productnamePostgreSQL/productname.
/para
  
para 
 -   Currently, only the DTrace utility is supported, which is available
 -   on Solaris Express, Solaris 10, and Mac OS X Leopard. It is expected that
 -   DTrace will be available in the future on FreeBSD.
 -   Supporting other dynamic tracing utilities is theoretically possible by
 -   changing the definitions for the macros in
 +   Currently, only the
 +   ulink url=http://opensolaris.org/os/community/dtrace/;DTrace/ulink
 +   utility is supported, which is available
 +   on OpenSolaris, Solaris 10, and Mac OS X Leopard. It is expected that
 +   DTrace will be available in the future on FreeBSD and possibly other
 +   operating systems. Supporting other dynamic tracing utilities is
 +   theoretically possible by changing the definitions for the macros in
 filenamesrc/include/utils/probes.h/.
/para
  
 @@ -1045,93 +1047,387 @@ SELECT pg_stat_get_backend_pid(s.backend
 titleBuilt-in Probes/title
  
para
 -   A few standard probes are provided in the source code
 -   (of course, more can be added as needed for a particular problem).
 -   These are shown in xref linkend=trace-point-table.
 +   A number of standard probes are provided in the source code, and more 
 +   can certainly be added to enhance PostgreSQL's observability. There are 
 two categories
 +   of probes, those that are targeted toward database administrators and 
 those for developers.  
 +   They are shown in xref linkend=admin-trace-point-table and
 +   xref linkend=dev-trace-point-table.
/para
  
 - table id=trace-point-table
 -  titleBuilt-in Probes/title
 + table id=admin-trace-point-table
 +  titleBuilt-in Probes for Administrators/title
tgroup cols=3
 thead
  row
   entryName/entry
   entryParameters/entry
 - entryOverview/entry
 + entryDescription/entry
  /row
 /thead
  
 tbody
 +
  row
   entrytransaction-start/entry
 - entry(int transactionId)/entry
 - entryThe start of a new transaction./entry
 + entry(LocalTransactionId)/entry
 + entryProbe that fires at the start of a new transaction. arg0 is the 
 transaction id./entry
  /row
  row
   entrytransaction-commit/entry
 - entry(int transactionId)/entry
 - entryThe successful completion of a transaction./entry
 + entry(LocalTransactionId)/entry
 + entryProbe that fires when a transaction completes successfully. arg0 
 is the transaction id./entry
  /row
  row
   entrytransaction-abort/entry
 - entry(int transactionId)/entry
 - entryThe unsuccessful completion of a transaction./entry
 + entry(LocalTransactionId)/entry
 + entryProbes that fires when a transaction does not complete 
 successfully. arg0 is the transaction id./entry
 +/row
 +row
 + entryquery-start/entry
 + entry(const char *)/entry
 + entryProbe that fires when the execution of a statement is started. 
 arg0 is the query string./entry
 +/row
 +row
 + entryquery-done/entry
 + entry(const char *)/entry
 + entryProbe that fires when the execution of a statement is complete. 
 arg0 is the query string./entry
 +/row
 +row
 + entryquery-parse-start/entry
 + entry(const char *)/entry
 + entryProbe that fires when the parsing of a query is started. arg0 is 
 the query string./entry
 +/row
 +row
 + entryquery-parse-done/entry
 + entry(const char *)/entry
 + entryProbe 

[HACKERS] DTrace doc patch for new probes in 8.4

[Robert Lor]

Attached is the doc patch for the recently added probes.

-Robert

Index: doc/src/sgml/monitoring.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/monitoring.sgml,v
retrieving revision 1.63
diff -u -3 -p -r1.63 monitoring.sgml
--- doc/src/sgml/monitoring.sgml11 Nov 2008 20:06:21 -  1.63
+++ doc/src/sgml/monitoring.sgml26 Feb 2009 22:18:31 -
@@ -1009,23 +1009,25 @@ SELECT pg_stat_get_backend_pid(s.backend
productnamePostgreSQL/productname provides facilities to support
dynamic tracing of the database server. This allows an external
utility to be called at specific points in the code and thereby trace
-   execution.  Currently, this facility is primarily intended for use by
-   database developers, as it requires substantial familiarity with the code.
+   execution.
   /para
 
   para
-   A number of probes or trace points are already inserted
-   into the source code.  By default these probes are not compiled into the
+   A number of probes or trace points are already inserted into the source
+   code. These probes are intended to be used by database developers and
+   administrators. By default the probes are not compiled into the
binary, and the user needs to explicitly tell the configure script to make
the probes available in productnamePostgreSQL/productname.
   /para
 
   para 
-   Currently, only the DTrace utility is supported, which is available
-   on Solaris Express, Solaris 10, and Mac OS X Leopard. It is expected that
-   DTrace will be available in the future on FreeBSD.
-   Supporting other dynamic tracing utilities is theoretically possible by
-   changing the definitions for the macros in
+   Currently, only the
+   ulink url=http://opensolaris.org/os/community/dtrace/;DTrace/ulink
+   utility is supported, which is available
+   on OpenSolaris, Solaris 10, and Mac OS X Leopard. It is expected that
+   DTrace will be available in the future on FreeBSD and possibly other
+   operating systems. Supporting other dynamic tracing utilities is
+   theoretically possible by changing the definitions for the macros in
filenamesrc/include/utils/probes.h/.
   /para
 
@@ -1045,93 +1047,387 @@ SELECT pg_stat_get_backend_pid(s.backend
titleBuilt-in Probes/title
 
   para
-   A few standard probes are provided in the source code
-   (of course, more can be added as needed for a particular problem).
-   These are shown in xref linkend=trace-point-table.
+   A number of standard probes are provided in the source code, and more 
+   can certainly be added to enhance PostgreSQL's observability. There are two 
categories
+   of probes, those that are targeted toward database administrators and those 
for developers.  
+   They are shown in xref linkend=admin-trace-point-table and
+   xref linkend=dev-trace-point-table.
   /para
 
- table id=trace-point-table
-  titleBuilt-in Probes/title
+ table id=admin-trace-point-table
+  titleBuilt-in Probes for Administrators/title
   tgroup cols=3
thead
 row
  entryName/entry
  entryParameters/entry
- entryOverview/entry
+ entryDescription/entry
 /row
/thead
 
tbody
+
 row
  entrytransaction-start/entry
- entry(int transactionId)/entry
- entryThe start of a new transaction./entry
+ entry(LocalTransactionId)/entry
+ entryProbe that fires at the start of a new transaction. arg0 is the 
transaction id./entry
 /row
 row
  entrytransaction-commit/entry
- entry(int transactionId)/entry
- entryThe successful completion of a transaction./entry
+ entry(LocalTransactionId)/entry
+ entryProbe that fires when a transaction completes successfully. arg0 
is the transaction id./entry
 /row
 row
  entrytransaction-abort/entry
- entry(int transactionId)/entry
- entryThe unsuccessful completion of a transaction./entry
+ entry(LocalTransactionId)/entry
+ entryProbes that fires when a transaction does not complete 
successfully. arg0 is the transaction id./entry
+/row
+row
+ entryquery-start/entry
+ entry(const char *)/entry
+ entryProbe that fires when the execution of a statement is started. 
arg0 is the query string./entry
+/row
+row
+ entryquery-done/entry
+ entry(const char *)/entry
+ entryProbe that fires when the execution of a statement is complete. 
arg0 is the query string./entry
+/row
+row
+ entryquery-parse-start/entry
+ entry(const char *)/entry
+ entryProbe that fires when the parsing of a query is started. arg0 is 
the query string./entry
+/row
+row
+ entryquery-parse-done/entry
+ entry(const char *)/entry
+ entryProbe that fires when the parsing of a query is complete. arg0 is 
the query string./entry
+/row
+row
+ entryquery-rewrite-start/entry
+ entry(const char *)/entry
+ entryProbe that fires when the rewriting of a query is started. 

Re: [HACKERS] DTrace probes patch

[Bruce Momjian]

Thanks, applied.

---

Robert Lor wrote:
 Tom Lane wrote:
  Robert Lor robert@sun.com writes:

  Tom Lane wrote:
  
  I agree.  If the probe is meant to track only *some* WAL writes
  then it needs to be named something less generic than
  TRACE_POSTGRESQL_WAL_BUFFER_WRITE.
 

  How about change it to TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY similar to 
  TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY for shared buffers?
  
 
  Works for me...
 
  
 Attached is the patch for the above name change.
 
 
 -Robert

 Index: src/backend/access/transam/xlog.c
 ===
 RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
 retrieving revision 1.324
 diff -u -3 -p -r1.324 xlog.c
 --- src/backend/access/transam/xlog.c 17 Dec 2008 01:39:03 -  1.324
 +++ src/backend/access/transam/xlog.c 22 Dec 2008 16:28:00 -
 @@ -1318,14 +1318,14 @@ AdvanceXLInsertBuffer(bool new_segment)
* Have to write buffers while holding insert 
 lock. This is
* not good, so only write as much as we 
 absolutely must.
*/
 - TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START();
 + TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY_START();
   WriteRqst.Write = OldPageRqstPtr;
   WriteRqst.Flush.xlogid = 0;
   WriteRqst.Flush.xrecoff = 0;
   XLogWrite(WriteRqst, false, false);
   LWLockRelease(WALWriteLock);
   Insert-LogwrtResult = LogwrtResult;
 - TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DONE();
 + TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY_DONE();
   }
   }
   }
 Index: src/backend/utils/probes.d
 ===
 RCS file: /projects/cvsroot/pgsql/src/backend/utils/probes.d,v
 retrieving revision 1.4
 diff -u -3 -p -r1.4 probes.d
 --- src/backend/utils/probes.d17 Dec 2008 01:39:04 -  1.4
 +++ src/backend/utils/probes.d22 Dec 2008 16:28:01 -
 @@ -89,6 +89,6 @@ provider postgresql {
  
   probe xlog__insert(unsigned char, unsigned char);
   probe xlog__switch();
 - probe wal__buffer__write__start();
 - probe wal__buffer__write__done();
 + probe wal__buffer__write__dirty__start();
 + probe wal__buffer__write__dirty__done();
  };

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] DTrace probes patch

[Robert Lor]

Tom Lane wrote:

Robert Lor robert@sun.com writes:
  

Tom Lane wrote:


I agree.  If the probe is meant to track only *some* WAL writes
then it needs to be named something less generic than
TRACE_POSTGRESQL_WAL_BUFFER_WRITE.

  

How about change it to TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY similar to 
TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY for shared buffers?



Works for me...



Attached is the patch for the above name change.


-Robert
Index: src/backend/access/transam/xlog.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.324
diff -u -3 -p -r1.324 xlog.c
--- src/backend/access/transam/xlog.c   17 Dec 2008 01:39:03 -  1.324
+++ src/backend/access/transam/xlog.c   22 Dec 2008 16:28:00 -
@@ -1318,14 +1318,14 @@ AdvanceXLInsertBuffer(bool new_segment)
 * Have to write buffers while holding insert 
lock. This is
 * not good, so only write as much as we 
absolutely must.
 */
-   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START();
+   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY_START();
WriteRqst.Write = OldPageRqstPtr;
WriteRqst.Flush.xlogid = 0;
WriteRqst.Flush.xrecoff = 0;
XLogWrite(WriteRqst, false, false);
LWLockRelease(WALWriteLock);
Insert-LogwrtResult = LogwrtResult;
-   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DONE();
+   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY_DONE();
}
}
}
Index: src/backend/utils/probes.d
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/probes.d,v
retrieving revision 1.4
diff -u -3 -p -r1.4 probes.d
--- src/backend/utils/probes.d  17 Dec 2008 01:39:04 -  1.4
+++ src/backend/utils/probes.d  22 Dec 2008 16:28:01 -
@@ -89,6 +89,6 @@ provider postgresql {
 
probe xlog__insert(unsigned char, unsigned char);
probe xlog__switch();
-   probe wal__buffer__write__start();
-   probe wal__buffer__write__done();
+   probe wal__buffer__write__dirty__start();
+   probe wal__buffer__write__dirty__done();
 };

-- 
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] DTrace probes patch

[Robert Lor]

Tom Lane wrote:

Fujii Masao masao.fu...@gmail.com writes:
  

I understood your intention. But, I think that its function name is somewhat
confusing.



I agree.  If the probe is meant to track only *some* WAL writes
then it needs to be named something less generic than
TRACE_POSTGRESQL_WAL_BUFFER_WRITE.

  

How about change it to TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY similar to 
TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY for shared buffers?


-Robert


--
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] DTrace probes patch

[Tom Lane]

Robert Lor robert@sun.com writes:
 Tom Lane wrote:
 I agree.  If the probe is meant to track only *some* WAL writes
 then it needs to be named something less generic than
 TRACE_POSTGRESQL_WAL_BUFFER_WRITE.
 
 How about change it to TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY similar to 
 TRACE_POSTGRESQL_BUFFER_WRITE_DIRTY for shared buffers?

Works for me...

regards, tom lane

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

Re: [HACKERS] DTrace probes patch

[Tom Lane]

Fujii Masao masao.fu...@gmail.com writes:
 On Thu, Dec 18, 2008 at 4:49 AM, Robert Lor robert@sun.com wrote:
 My understanding is that we only want to track the XLogWrite when advancing
 to the next buffer page, and if there is unwritten data in the new buffer
 page, that indicates no more empty WAL buffer pages available, but I may be
 wrong. I did some tests by adjusting wal_buffers, and I could observe this
 behavior, more calls to XLogWrite with smaller wal_buffers.

 I understood your intention. But, I think that its function name is somewhat
 confusing.

I agree.  If the probe is meant to track only *some* WAL writes
then it needs to be named something less generic than
TRACE_POSTGRESQL_WAL_BUFFER_WRITE.

regards, tom lane

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

Re: [HACKERS] DTrace probes patch

[Robert Lor]

Alvaro Herrera wrote:

But there are 5 callers of XLogWrite ... why aren't the other ones being
tracked too?

  


This probe originally came from Simon, so it can't possibly be wrong :-)

My understanding is that we only want to track the XLogWrite when 
advancing to the next buffer page, and if there is unwritten data in the 
new buffer page, that indicates no more empty WAL buffer pages 
available, but I may be wrong. I did some tests by adjusting 
wal_buffers, and I could observe this behavior, more calls to XLogWrite 
with smaller wal_buffers.


-Robert



--
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] DTrace probes patch

[Alvaro Herrera]

Robert Lor escribió:
 Fujii Masao wrote:
 Hi,

 On Wed, Dec 17, 2008 at 4:53 AM, Robert Lor robert@sun.com wrote:

 Why is TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START/DONE called
 only in AdvanceXLInsertBuffer? We can trace only a part of WAL buffer write?
   
 The intention of these probes is to determine if wal_buffers is too  
 small by monitoring how frequent the server has to write out the buffers  
 along with the I/O time.

But there are 5 callers of XLogWrite ... why aren't the other ones being
tracked too?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] DTrace probes patch

[Robert Lor]

Fujii Masao wrote:

Hi,

On Wed, Dec 17, 2008 at 4:53 AM, Robert Lor robert@sun.com wrote:
  

@@ -1313,12 +1318,14 @@ AdvanceXLInsertBuffer(bool new_segment)
* Have to write buffers while holding insert
lock. This is
* not good, so only write as much as we
absolutely must.
*/
+   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START();
   WriteRqst.Write = OldPageRqstPtr;
   WriteRqst.Flush.xlogid = 0;
   WriteRqst.Flush.xrecoff = 0;
   XLogWrite(WriteRqst, false, false);
   LWLockRelease(WALWriteLock);
   Insert-LogwrtResult = LogwrtResult;
+   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DONE();



Why is TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START/DONE called
only in AdvanceXLInsertBuffer? We can trace only a part of WAL buffer write?

  
The intention of these probes is to determine if wal_buffers is too 
small by monitoring how frequent the server has to write out the buffers 
along with the I/O time.


-Robert



--
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] DTrace probes patch

[Fujii Masao]

Hi,

On Thu, Dec 18, 2008 at 4:49 AM, Robert Lor robert@sun.com wrote:
 Alvaro Herrera wrote:

 But there are 5 callers of XLogWrite ... why aren't the other ones being
 tracked too?



 This probe originally came from Simon, so it can't possibly be wrong :-)

 My understanding is that we only want to track the XLogWrite when advancing
 to the next buffer page, and if there is unwritten data in the new buffer
 page, that indicates no more empty WAL buffer pages available, but I may be
 wrong. I did some tests by adjusting wal_buffers, and I could observe this
 behavior, more calls to XLogWrite with smaller wal_buffers.

I understood your intention. But, I think that its function name is somewhat
confusing.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] DTrace probes patch

[Robert Lor]

Peter Eisentraut wrote:

Robert Lor wrote:


The attached patch contains a couple of fixes in the existing probes 
and includes a few new ones.


- Fixed compilation errors on OS X for probes that use typedefs


Could you explain what these errors are about?  I don't see any errors 
on my machine.


In the current probes.d, the following probe definitions are commented 
out because they cause compilation errors on OS X.


* probe lock__wait__start(unsigned int, LOCKMODE);
* probe lock__wait__done(unsigned int, LOCKMODE);
* probe buffer__read__start(BlockNumber, Oid, Oid, Oid, bool);
* probe buffer__read__done(BlockNumber, Oid, Oid, Oid, bool, bool);

The problem was fixed by making the changes below.  probes.d is 
preprocessed with cpp and as such only  macros get expanded.


From:

typedef unsigned int LocalTransactionId;
typedef int LWLockId;
typedef int LWLockMode;
typedef int LOCKMODE;
typedef unsigned int BlockNumber;
typedef unsigned int Oid;
typedef int ForkNumber;


To:

#define LocalTransactionId unsigned int
#define LWLockId int
#define LWLockMode int
#define LOCKMODE int
#define BlockNumber unsigned int
#define Oid unsigned int
#define ForkNumber int





--
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] DTrace probes patch

[Bruce Momjian]

Robert Lor wrote:
 Bruce Momjian wrote:
  Should I apply this or hold it for 8.5?
 
 

 I think it should go into 8.4 as it also fixes existing problems.

I am seeing the following error when linking the backend on a BSD machine:

./../src/port/libpgport_srv.a -lssl -lcrypto -lgetopt -ldl -lutil -lm -o
postgres
storage/buffer/bufmgr.o: In function `ReadBuffer_common':
storage/buffer/bufmgr.o(.text+0x4e2): undefined reference to
`TRACE_POSTGRESQL_BUFFER_READ_DONE'
storage/smgr/md.o: In function `mdread':
storage/smgr/md.o(.text+0x8a7): undefined reference to
`TRACE_POSTGRESQL_SMGR_MD_READ_DONE'
storage/smgr/md.o: In function `mdwrite':
storage/smgr/md.o(.text+0xab6): undefined reference to
`TRACE_POSTGRESQL_SMGR_MD_WRITE_DONE'
gmake[2]: *** [postgres] Error 1
gmake[2]: Leaving directory
`/usr/var/local/src/gen/pgsql/CURRENT/pgsql/src/backend'
gmake[1]: *** [all] Error 2
gmake[1]: Leaving directory
`/usr/var/local/src/gen/pgsql/CURRENT/pgsql/src'


-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

-- 
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] DTrace probes patch

[Alvaro Herrera]

Bruce Momjian wrote:

 I am seeing the following error when linking the backend on a BSD machine:
 
   ./../src/port/libpgport_srv.a -lssl -lcrypto -lgetopt -ldl -lutil -lm -o
   postgres
   storage/buffer/bufmgr.o: In function `ReadBuffer_common':
   storage/buffer/bufmgr.o(.text+0x4e2): undefined reference to
   `TRACE_POSTGRESQL_BUFFER_READ_DONE'
   storage/smgr/md.o: In function `mdread':
   storage/smgr/md.o(.text+0x8a7): undefined reference to

The reason is that Gen_dummy_probes.sed handles only up to 6 args, and
this function has 7.  Just add one more line to that file.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] DTrace probes patch

[Alvaro Herrera]

Robert Lor wrote:
 Peter Eisentraut wrote:
 Robert Lor wrote:

 The attached patch contains a couple of fixes in the existing probes  
 and includes a few new ones.

 - Fixed compilation errors on OS X for probes that use typedefs

 Could you explain what these errors are about?  I don't see any errors  
 on my machine.

 In the current probes.d, the following probe definitions are commented  
 out because they cause compilation errors on OS X.

Yeah, this was something we agreed to fix when we committed the previous
DTrace patch.  The current code was said to be a stopgap.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] DTrace probes patch

[Robert Lor]

Bruce Momjian wrote:

Should I apply this or hold it for 8.5?


  

I think it should go into 8.4 as it also fixes existing problems.

-Robert


--
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] DTrace probes patch

[Peter Eisentraut]

Robert Lor wrote:


The attached patch contains a couple of fixes in the existing probes and 
includes a few new ones.


- Fixed compilation errors on OS X for probes that use typedefs


Could you explain what these errors are about?  I don't see any errors 
on my machine.



--
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] DTrace probes patch

[Robert Lor]

Bruce Momjian wrote:


I am seeing the following error when linking the backend on a BSD machine:


  
The updated patch attached should fix this problem. My bad for 
overlooking this.


-Robert


Index: src/backend/access/transam/xlog.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.322
diff -u -3 -p -r1.322 xlog.c
--- src/backend/access/transam/xlog.c   9 Nov 2008 17:51:15 -   1.322
+++ src/backend/access/transam/xlog.c   16 Dec 2008 19:46:08 -
@@ -48,6 +48,7 @@
 #include utils/builtins.h
 #include utils/guc.h
 #include utils/ps_status.h
+#include pg_trace.h
 
 
 /* File path names (all relative to $PGDATA) */
@@ -486,6 +487,8 @@ XLogInsert(RmgrId rmid, uint8 info, XLog
if (info  XLR_INFO_MASK)
elog(PANIC, invalid xlog info mask %02X, info);
 
+   TRACE_POSTGRESQL_XLOG_INSERT(rmid, info);
+
/*
 * In bootstrap mode, we don't actually log anything but XLOG resources;
 * return a phony record pointer.
@@ -914,6 +917,8 @@ begin:;
XLogwrtRqst FlushRqst;
XLogRecPtr  OldSegEnd;
 
+   TRACE_POSTGRESQL_XLOG_SWITCH();
+
LWLockAcquire(WALWriteLock, LW_EXCLUSIVE);
 
/*
@@ -1313,12 +1318,14 @@ AdvanceXLInsertBuffer(bool new_segment)
 * Have to write buffers while holding insert 
lock. This is
 * not good, so only write as much as we 
absolutely must.
 */
+   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START();
WriteRqst.Write = OldPageRqstPtr;
WriteRqst.Flush.xlogid = 0;
WriteRqst.Flush.xrecoff = 0;
XLogWrite(WriteRqst, false, false);
LWLockRelease(WALWriteLock);
Insert-LogwrtResult = LogwrtResult;
+   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DONE();
}
}
}
@@ -5904,6 +5911,8 @@ CreateCheckPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags);
 
+   TRACE_POSTGRESQL_CHECKPOINT_START(flags);
+
/*
 * Before flushing data, we must wait for any transactions that are
 * currently in their commit critical sections.  If an xact inserted its
@@ -6069,6 +6078,11 @@ CreateCheckPoint(int flags)
if (log_checkpoints)
LogCheckpointEnd();
 
+TRACE_POSTGRESQL_CHECKPOINT_DONE(CheckpointStats.ckpt_bufs_written,
+NBuffers, CheckpointStats.ckpt_segs_added,
+CheckpointStats.ckpt_segs_removed,
+CheckpointStats.ckpt_segs_recycled);
+
LWLockRelease(CheckpointLock);
 }
 
Index: src/backend/storage/buffer/bufmgr.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v
retrieving revision 1.242
diff -u -3 -p -r1.242 bufmgr.c
--- src/backend/storage/buffer/bufmgr.c 19 Nov 2008 10:34:52 -  1.242
+++ src/backend/storage/buffer/bufmgr.c 16 Dec 2008 19:46:11 -
@@ -203,8 +203,7 @@ ReadBuffer_common(SMgrRelation smgr, boo
if (isExtend)
blockNum = smgrnblocks(smgr, forkNum);
 
-   TRACE_POSTGRESQL_BUFFER_READ_START(blockNum, smgr-smgr_rnode.spcNode,
-   smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode, isLocalBuf);
+   TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum, 
smgr-smgr_rnode.spcNode, smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode, 
isLocalBuf);
 
if (isLocalBuf)
{
@@ -253,7 +252,7 @@ ReadBuffer_common(SMgrRelation smgr, boo
if (VacuumCostActive)
VacuumCostBalance += VacuumCostPageHit;
 
-   TRACE_POSTGRESQL_BUFFER_READ_DONE(blockNum,
+   TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
smgr-smgr_rnode.spcNode,
smgr-smgr_rnode.dbNode,
smgr-smgr_rnode.relNode, isLocalBuf, found);
@@ -380,9 +379,9 @@ ReadBuffer_common(SMgrRelation smgr, boo
if (VacuumCostActive)
VacuumCostBalance += VacuumCostPageMiss;
 
-   TRACE_POSTGRESQL_BUFFER_READ_DONE(blockNum, smgr-smgr_rnode.spcNode,
-   smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode,
-   isLocalBuf, found);
+   TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
+   smgr-smgr_rnode.spcNode, smgr-smgr_rnode.dbNode,
+   smgr-smgr_rnode.relNode, isLocalBuf, found);
 
return BufferDescriptorGetBuffer(bufHdr);
 }
@@ 

Re: [HACKERS] DTrace probes patch

[Bruce Momjian]

Thanks, applied.

---

Robert Lor wrote:
 Bruce Momjian wrote:
 
  I am seeing the following error when linking the backend on a BSD machine:
 
 

 The updated patch attached should fix this problem. My bad for 
 overlooking this.
 
 -Robert
 
 

 Index: src/backend/access/transam/xlog.c
 ===
 RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
 retrieving revision 1.322
 diff -u -3 -p -r1.322 xlog.c
 --- src/backend/access/transam/xlog.c 9 Nov 2008 17:51:15 -   1.322
 +++ src/backend/access/transam/xlog.c 16 Dec 2008 19:46:08 -
 @@ -48,6 +48,7 @@
  #include utils/builtins.h
  #include utils/guc.h
  #include utils/ps_status.h
 +#include pg_trace.h
  
  
  /* File path names (all relative to $PGDATA) */
 @@ -486,6 +487,8 @@ XLogInsert(RmgrId rmid, uint8 info, XLog
   if (info  XLR_INFO_MASK)
   elog(PANIC, invalid xlog info mask %02X, info);
  
 + TRACE_POSTGRESQL_XLOG_INSERT(rmid, info);
 +
   /*
* In bootstrap mode, we don't actually log anything but XLOG resources;
* return a phony record pointer.
 @@ -914,6 +917,8 @@ begin:;
   XLogwrtRqst FlushRqst;
   XLogRecPtr  OldSegEnd;
  
 + TRACE_POSTGRESQL_XLOG_SWITCH();
 +
   LWLockAcquire(WALWriteLock, LW_EXCLUSIVE);
  
   /*
 @@ -1313,12 +1318,14 @@ AdvanceXLInsertBuffer(bool new_segment)
* Have to write buffers while holding insert 
 lock. This is
* not good, so only write as much as we 
 absolutely must.
*/
 + TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START();
   WriteRqst.Write = OldPageRqstPtr;
   WriteRqst.Flush.xlogid = 0;
   WriteRqst.Flush.xrecoff = 0;
   XLogWrite(WriteRqst, false, false);
   LWLockRelease(WALWriteLock);
   Insert-LogwrtResult = LogwrtResult;
 + TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DONE();
   }
   }
   }
 @@ -5904,6 +5911,8 @@ CreateCheckPoint(int flags)
   if (log_checkpoints)
   LogCheckpointStart(flags);
  
 + TRACE_POSTGRESQL_CHECKPOINT_START(flags);
 +
   /*
* Before flushing data, we must wait for any transactions that are
* currently in their commit critical sections.  If an xact inserted its
 @@ -6069,6 +6078,11 @@ CreateCheckPoint(int flags)
   if (log_checkpoints)
   LogCheckpointEnd();
  
 +TRACE_POSTGRESQL_CHECKPOINT_DONE(CheckpointStats.ckpt_bufs_written,
 +NBuffers, CheckpointStats.ckpt_segs_added,
 +CheckpointStats.ckpt_segs_removed,
 +CheckpointStats.ckpt_segs_recycled);
 +
   LWLockRelease(CheckpointLock);
  }
  
 Index: src/backend/storage/buffer/bufmgr.c
 ===
 RCS file: /projects/cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v
 retrieving revision 1.242
 diff -u -3 -p -r1.242 bufmgr.c
 --- src/backend/storage/buffer/bufmgr.c   19 Nov 2008 10:34:52 -  
 1.242
 +++ src/backend/storage/buffer/bufmgr.c   16 Dec 2008 19:46:11 -
 @@ -203,8 +203,7 @@ ReadBuffer_common(SMgrRelation smgr, boo
   if (isExtend)
   blockNum = smgrnblocks(smgr, forkNum);
  
 - TRACE_POSTGRESQL_BUFFER_READ_START(blockNum, smgr-smgr_rnode.spcNode,
 - smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode, isLocalBuf);
 + TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum, 
 smgr-smgr_rnode.spcNode, smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode, 
 isLocalBuf);
  
   if (isLocalBuf)
   {
 @@ -253,7 +252,7 @@ ReadBuffer_common(SMgrRelation smgr, boo
   if (VacuumCostActive)
   VacuumCostBalance += VacuumCostPageHit;
  
 - TRACE_POSTGRESQL_BUFFER_READ_DONE(blockNum,
 + TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
   smgr-smgr_rnode.spcNode,
   smgr-smgr_rnode.dbNode,
   smgr-smgr_rnode.relNode, isLocalBuf, found);
 @@ -380,9 +379,9 @@ ReadBuffer_common(SMgrRelation smgr, boo
   if (VacuumCostActive)
   VacuumCostBalance += VacuumCostPageMiss;
  
 - TRACE_POSTGRESQL_BUFFER_READ_DONE(blockNum, smgr-smgr_rnode.spcNode,
 - smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode,
 - isLocalBuf, found);
 + TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
 + smgr-smgr_rnode.spcNode, 

Re: [HACKERS] DTrace probes patch

[Fujii Masao]

Hi,

On Wed, Dec 17, 2008 at 4:53 AM, Robert Lor robert@sun.com wrote:
 @@ -1313,12 +1318,14 @@ AdvanceXLInsertBuffer(bool new_segment)
 * Have to write buffers while holding insert
 lock. This is
 * not good, so only write as much as we
 absolutely must.
 */
 +   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START();
WriteRqst.Write = OldPageRqstPtr;
WriteRqst.Flush.xlogid = 0;
WriteRqst.Flush.xrecoff = 0;
XLogWrite(WriteRqst, false, false);
LWLockRelease(WALWriteLock);
Insert-LogwrtResult = LogwrtResult;
 +   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DONE();

Why is TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START/DONE called
only in AdvanceXLInsertBuffer? We can trace only a part of WAL buffer write?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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

[HACKERS] DTrace probes patch

[Robert Lor]

The attached patch contains a couple of fixes in the existing probes and 
includes a few new ones.


- Fixed compilation errors on OS X for probes that use typedefs
- Fixed a number of probes to pass ForkNumber per the relation forks patch
- The new probes are those that were taken out from the previous 
submitted patch and required simple fixes. Will submit the other probes 
that may require more discussion in a separate patch.



-Robert




Index: src/backend/access/transam/xlog.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.322
diff -u -3 -p -r1.322 xlog.c
--- src/backend/access/transam/xlog.c   9 Nov 2008 17:51:15 -   1.322
+++ src/backend/access/transam/xlog.c   15 Dec 2008 05:12:41 -
@@ -48,6 +48,7 @@
 #include utils/builtins.h
 #include utils/guc.h
 #include utils/ps_status.h
+#include pg_trace.h
 
 
 /* File path names (all relative to $PGDATA) */
@@ -486,6 +487,8 @@ XLogInsert(RmgrId rmid, uint8 info, XLog
if (info  XLR_INFO_MASK)
elog(PANIC, invalid xlog info mask %02X, info);
 
+   TRACE_POSTGRESQL_XLOG_INSERT(rmid, info);
+
/*
 * In bootstrap mode, we don't actually log anything but XLOG resources;
 * return a phony record pointer.
@@ -914,6 +917,8 @@ begin:;
XLogwrtRqst FlushRqst;
XLogRecPtr  OldSegEnd;
 
+   TRACE_POSTGRESQL_XLOG_SWITCH();
+
LWLockAcquire(WALWriteLock, LW_EXCLUSIVE);
 
/*
@@ -1313,12 +1318,14 @@ AdvanceXLInsertBuffer(bool new_segment)
 * Have to write buffers while holding insert 
lock. This is
 * not good, so only write as much as we 
absolutely must.
 */
+   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START();
WriteRqst.Write = OldPageRqstPtr;
WriteRqst.Flush.xlogid = 0;
WriteRqst.Flush.xrecoff = 0;
XLogWrite(WriteRqst, false, false);
LWLockRelease(WALWriteLock);
Insert-LogwrtResult = LogwrtResult;
+   TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DONE();
}
}
}
@@ -5904,6 +5911,8 @@ CreateCheckPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags);
 
+   TRACE_POSTGRESQL_CHECKPOINT_START(flags);
+
/*
 * Before flushing data, we must wait for any transactions that are
 * currently in their commit critical sections.  If an xact inserted its
@@ -6069,6 +6078,11 @@ CreateCheckPoint(int flags)
if (log_checkpoints)
LogCheckpointEnd();
 
+TRACE_POSTGRESQL_CHECKPOINT_DONE(CheckpointStats.ckpt_bufs_written,
+NBuffers, CheckpointStats.ckpt_segs_added,
+CheckpointStats.ckpt_segs_removed,
+CheckpointStats.ckpt_segs_recycled);
+
LWLockRelease(CheckpointLock);
 }
 
Index: src/backend/storage/buffer/bufmgr.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v
retrieving revision 1.242
diff -u -3 -p -r1.242 bufmgr.c
--- src/backend/storage/buffer/bufmgr.c 19 Nov 2008 10:34:52 -  1.242
+++ src/backend/storage/buffer/bufmgr.c 15 Dec 2008 05:12:45 -
@@ -203,8 +203,7 @@ ReadBuffer_common(SMgrRelation smgr, boo
if (isExtend)
blockNum = smgrnblocks(smgr, forkNum);
 
-   TRACE_POSTGRESQL_BUFFER_READ_START(blockNum, smgr-smgr_rnode.spcNode,
-   smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode, isLocalBuf);
+   TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum, 
smgr-smgr_rnode.spcNode, smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode, 
isLocalBuf);
 
if (isLocalBuf)
{
@@ -253,7 +252,7 @@ ReadBuffer_common(SMgrRelation smgr, boo
if (VacuumCostActive)
VacuumCostBalance += VacuumCostPageHit;
 
-   TRACE_POSTGRESQL_BUFFER_READ_DONE(blockNum,
+   TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
smgr-smgr_rnode.spcNode,
smgr-smgr_rnode.dbNode,
smgr-smgr_rnode.relNode, isLocalBuf, found);
@@ -380,9 +379,9 @@ ReadBuffer_common(SMgrRelation smgr, boo
if (VacuumCostActive)
VacuumCostBalance += VacuumCostPageMiss;
 
-   TRACE_POSTGRESQL_BUFFER_READ_DONE(blockNum, smgr-smgr_rnode.spcNode,
-   smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode,
-   isLocalBuf, found);
+   

Re: [HACKERS] DTrace probes patch

[Bruce Momjian]

Should I apply this or hold it for 8.5?

---

Robert Lor wrote:
 
 The attached patch contains a couple of fixes in the existing probes and 
 includes a few new ones.
 
 - Fixed compilation errors on OS X for probes that use typedefs
 - Fixed a number of probes to pass ForkNumber per the relation forks patch
 - The new probes are those that were taken out from the previous 
 submitted patch and required simple fixes. Will submit the other probes 
 that may require more discussion in a separate patch.
 
 
 -Robert
 
 
 
 

 Index: src/backend/access/transam/xlog.c
 ===
 RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
 retrieving revision 1.322
 diff -u -3 -p -r1.322 xlog.c
 --- src/backend/access/transam/xlog.c 9 Nov 2008 17:51:15 -   1.322
 +++ src/backend/access/transam/xlog.c 15 Dec 2008 05:12:41 -
 @@ -48,6 +48,7 @@
  #include utils/builtins.h
  #include utils/guc.h
  #include utils/ps_status.h
 +#include pg_trace.h
  
  
  /* File path names (all relative to $PGDATA) */
 @@ -486,6 +487,8 @@ XLogInsert(RmgrId rmid, uint8 info, XLog
   if (info  XLR_INFO_MASK)
   elog(PANIC, invalid xlog info mask %02X, info);
  
 + TRACE_POSTGRESQL_XLOG_INSERT(rmid, info);
 +
   /*
* In bootstrap mode, we don't actually log anything but XLOG resources;
* return a phony record pointer.
 @@ -914,6 +917,8 @@ begin:;
   XLogwrtRqst FlushRqst;
   XLogRecPtr  OldSegEnd;
  
 + TRACE_POSTGRESQL_XLOG_SWITCH();
 +
   LWLockAcquire(WALWriteLock, LW_EXCLUSIVE);
  
   /*
 @@ -1313,12 +1318,14 @@ AdvanceXLInsertBuffer(bool new_segment)
* Have to write buffers while holding insert 
 lock. This is
* not good, so only write as much as we 
 absolutely must.
*/
 + TRACE_POSTGRESQL_WAL_BUFFER_WRITE_START();
   WriteRqst.Write = OldPageRqstPtr;
   WriteRqst.Flush.xlogid = 0;
   WriteRqst.Flush.xrecoff = 0;
   XLogWrite(WriteRqst, false, false);
   LWLockRelease(WALWriteLock);
   Insert-LogwrtResult = LogwrtResult;
 + TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DONE();
   }
   }
   }
 @@ -5904,6 +5911,8 @@ CreateCheckPoint(int flags)
   if (log_checkpoints)
   LogCheckpointStart(flags);
  
 + TRACE_POSTGRESQL_CHECKPOINT_START(flags);
 +
   /*
* Before flushing data, we must wait for any transactions that are
* currently in their commit critical sections.  If an xact inserted its
 @@ -6069,6 +6078,11 @@ CreateCheckPoint(int flags)
   if (log_checkpoints)
   LogCheckpointEnd();
  
 +TRACE_POSTGRESQL_CHECKPOINT_DONE(CheckpointStats.ckpt_bufs_written,
 +NBuffers, CheckpointStats.ckpt_segs_added,
 +CheckpointStats.ckpt_segs_removed,
 +CheckpointStats.ckpt_segs_recycled);
 +
   LWLockRelease(CheckpointLock);
  }
  
 Index: src/backend/storage/buffer/bufmgr.c
 ===
 RCS file: /projects/cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v
 retrieving revision 1.242
 diff -u -3 -p -r1.242 bufmgr.c
 --- src/backend/storage/buffer/bufmgr.c   19 Nov 2008 10:34:52 -  
 1.242
 +++ src/backend/storage/buffer/bufmgr.c   15 Dec 2008 05:12:45 -
 @@ -203,8 +203,7 @@ ReadBuffer_common(SMgrRelation smgr, boo
   if (isExtend)
   blockNum = smgrnblocks(smgr, forkNum);
  
 - TRACE_POSTGRESQL_BUFFER_READ_START(blockNum, smgr-smgr_rnode.spcNode,
 - smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode, isLocalBuf);
 + TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum, 
 smgr-smgr_rnode.spcNode, smgr-smgr_rnode.dbNode, smgr-smgr_rnode.relNode, 
 isLocalBuf);
  
   if (isLocalBuf)
   {
 @@ -253,7 +252,7 @@ ReadBuffer_common(SMgrRelation smgr, boo
   if (VacuumCostActive)
   VacuumCostBalance += VacuumCostPageHit;
  
 - TRACE_POSTGRESQL_BUFFER_READ_DONE(blockNum,
 + TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
   smgr-smgr_rnode.spcNode,
   smgr-smgr_rnode.dbNode,
   smgr-smgr_rnode.relNode, isLocalBuf, found);
 @@ -380,9 +379,9 @@ ReadBuffer_common(SMgrRelation smgr, boo
   if (VacuumCostActive)
   VacuumCostBalance += VacuumCostPageMiss;
  
 - 

Re: [HACKERS] DTrace probes.

[Robert Treat]

On Monday 19 May 2008 11:32:28 Theo Schlossnagle wrote:
 Howdy,

 I just saw Robert Lor's patch w.r.t. dtrace probes.  It looks very
 similar in what we've done.  We run a nice set of probes in production
 here that allow us to track the details of checkpointing and statement
 execution.  I've given a few presentations around these probes and
 have had very positive feedback.   They've been available for a while
 now, but I never got around to sending them to the list:

 https://labs.omniti.com/trac/project-dtrace/browser/trunk/postgresql/8.3.1.
patch?format=txt

 Documentation is in wiki format, but I'd be happy to convert it to
 something else:

 https://labs.omniti.com/trac/project-dtrace/wiki/Applications#PostgreSQL


Attached is the patch combining Theo's patch from above into the original 
patch from Robert Lor.  It is supposed to build on OSX and Solaris.  I'll be 
updating the July commitfest entry to point to this patch, case anyone wants 
to review it. 

-- 
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
Index: src/backend/access/transam/clog.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/clog.c,v
retrieving revision 1.46
diff -u -3 -p -r1.46 clog.c
--- src/backend/access/transam/clog.c	1 Jan 2008 19:45:46 -	1.46
+++ src/backend/access/transam/clog.c	26 Jun 2008 23:18:30 -
@@ -36,6 +36,7 @@
 #include access/slru.h
 #include access/transam.h
 #include postmaster/bgwriter.h
+#include pg_trace.h
 
 /*
  * Defines for CLOG page sizes.  A page is the same BLCKSZ as is used
@@ -323,7 +324,9 @@ void
 CheckPointCLOG(void)
 {
 	/* Flush dirty CLOG pages to disk */
+	TRACE_POSTGRESQL_CLOG_CHECKPOINT_START();
 	SimpleLruFlush(ClogCtl, true);
+	TRACE_POSTGRESQL_CLOG_CHECKPOINT_DONE();
 }
 
 
Index: src/backend/access/transam/multixact.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/multixact.c,v
retrieving revision 1.27
diff -u -3 -p -r1.27 multixact.c
--- src/backend/access/transam/multixact.c	1 Jan 2008 19:45:46 -	1.27
+++ src/backend/access/transam/multixact.c	26 Jun 2008 23:18:31 -
@@ -57,6 +57,7 @@
 #include storage/lmgr.h
 #include utils/memutils.h
 #include storage/procarray.h
+#include pg_trace.h
 
 
 /*
@@ -1526,6 +1527,8 @@ MultiXactGetCheckptMulti(bool is_shutdow
 void
 CheckPointMultiXact(void)
 {
+	TRACE_POSTGRESQL_MULTIXACT_CHECKPOINT_START();
+
 	/* Flush dirty MultiXact pages to disk */
 	SimpleLruFlush(MultiXactOffsetCtl, true);
 	SimpleLruFlush(MultiXactMemberCtl, true);
@@ -1540,6 +1543,8 @@ CheckPointMultiXact(void)
 	 */
 	if (!InRecovery)
 		TruncateMultiXact();
+
+	TRACE_POSTGRESQL_MULTIXACT_CHECKPOINT_DONE();
 }
 
 /*
Index: src/backend/access/transam/slru.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/slru.c,v
retrieving revision 1.44
diff -u -3 -p -r1.44 slru.c
--- src/backend/access/transam/slru.c	1 Jan 2008 19:45:48 -	1.44
+++ src/backend/access/transam/slru.c	26 Jun 2008 23:18:32 -
@@ -57,6 +57,7 @@
 #include storage/fd.h
 #include storage/shmem.h
 #include miscadmin.h
+#include pg_trace.h
 
 
 /*
@@ -372,6 +373,8 @@ SimpleLruReadPage(SlruCtl ctl, int pagen
 {
 	SlruShared	shared = ctl-shared;
 
+	TRACE_POSTGRESQL_SLRU_READPAGE_START((uintptr_t)ctl, pageno, write_ok, xid);
+
 	/* Outer loop handles restart if we must wait for someone else's I/O */
 	for (;;)
 	{
@@ -399,6 +402,7 @@ SimpleLruReadPage(SlruCtl ctl, int pagen
 			}
 			/* Otherwise, it's ready to use */
 			SlruRecentlyUsed(shared, slotno);
+			TRACE_POSTGRESQL_SLRU_READPAGE_DONE(slotno);
 			return slotno;
 		}
 
@@ -446,6 +450,7 @@ SimpleLruReadPage(SlruCtl ctl, int pagen
 			SlruReportIOError(ctl, pageno, xid);
 
 		SlruRecentlyUsed(shared, slotno);
+		TRACE_POSTGRESQL_SLRU_READPAGE_DONE(slotno);
 		return slotno;
 	}
 }
@@ -470,6 +475,8 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, 
 	SlruShared	shared = ctl-shared;
 	int			slotno;
 
+	TRACE_POSTGRESQL_SLRU_READPAGE_READONLY((uintptr_t)ctl, pageno, xid);
+
 	/* Try to find the page while holding only shared lock */
 	LWLockAcquire(shared-ControlLock, LW_SHARED);
 
@@ -511,6 +518,8 @@ SimpleLruWritePage(SlruCtl ctl, int slot
 	int			pageno = shared-page_number[slotno];
 	bool		ok;
 
+	TRACE_POSTGRESQL_SLRU_WRITEPAGE_START((uintptr_t)ctl, pageno, slotno);
+
 	/* If a write is in progress, wait for it to finish */
 	while (shared-page_status[slotno] == SLRU_PAGE_WRITE_IN_PROGRESS 
 		   shared-page_number[slotno] == pageno)
@@ -525,7 +534,10 @@ SimpleLruWritePage(SlruCtl ctl, int slot
 	if (!shared-page_dirty[slotno] ||
 		shared-page_status[slotno] != SLRU_PAGE_VALID ||
 		shared-page_number[slotno] != pageno)
+	{
+		TRACE_POSTGRESQL_SLRU_WRITEPAGE_DONE();
 		return;
+	}
 
 	/*
 	 * Mark the slot write-busy, and clear the dirtybit.  After this point, a
@@ -569,6 +581,8 

[HACKERS] DTrace probes.

[Theo Schlossnagle]

Howdy,

I just saw Robert Lor's patch w.r.t. dtrace probes.  It looks very  
similar in what we've done.  We run a nice set of probes in production  
here that allow us to track the details of checkpointing and statement  
execution.  I've given a few presentations around these probes and  
have had very positive feedback.   They've been available for a while  
now, but I never got around to sending them to the list:


https://labs.omniti.com/trac/project-dtrace/browser/trunk/postgresql/8.3.1.patch?format=txt

Documentation is in wiki format, but I'd be happy to convert it to  
something else:


https://labs.omniti.com/trac/project-dtrace/wiki/Applications#PostgreSQL

Best regards,

Theo


--
Theo Schlossnagle
Esoteric Curio -- http://lethargy.org/
OmniTI Computer Consulting, Inc. -- http://omniti.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] DTrace enabled build fails

[Bruce Momjian]

Robert Lor wrote:
 Peter Eisentraut wrote:
 
 That rings a bell.  Can we get a more precise designation on what 
 version of DTrace we support?  And where can one get that required 
 update?
 
   
 
 Peter,
 
 The problem with static function was fixed recently and is now available 
 in Solaris Express (the development version of Solaris). You can get the 
 bits from http://www.sun.com/software/solaris/solaris-express/get.jsp. I 
 forgot to mention this know issue in my previous emails!
 
 I was told by the DTrace engineer that this fix will be in the next 
 update of Solaris 10.

Do we need to add detection logic to catch buggy versions?

-- 
  Bruce Momjian   [EMAIL PROTECTED]
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [HACKERS] DTrace enabled build fails

[Robert Lor]

Bruce Momjian wrote:



Do we need to add detection logic to catch buggy versions?

 

Instead of adding extra logic, I think it's sufficient with 
documentation since the issue will soon be fixed in the next Solaris update.


Regards,
-Robert

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] DTrace enabled build fails

[Tom Lane]

Robert Lor [EMAIL PROTECTED] writes:
 Bruce Momjian wrote:
 Do we need to add detection logic to catch buggy versions?
 
 Instead of adding extra logic, I think it's sufficient with 
 documentation since the issue will soon be fixed in the next Solaris update.

I agree ... it's not like this is a feature aimed at novices.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] DTrace enabled build fails

[Robert Lor]

Peter Eisentraut wrote:

That rings a bell.  Can we get a more precise designation on what 
version of DTrace we support?  And where can one get that required 
update?


 


Peter,

The problem with static function was fixed recently and is now available 
in Solaris Express (the development version of Solaris). You can get the 
bits from http://www.sun.com/software/solaris/solaris-express/get.jsp. I 
forgot to mention this know issue in my previous emails!


I was told by the DTrace engineer that this fix will be in the next 
update of Solaris 10.


Regards,
-Robert

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [HACKERS] DTrace enabled build fails

[Peter Eisentraut]

Tom Lane wrote:
 Peter Eisentraut [EMAIL PROTECTED] writes:
  Does it not like static functions?

 I seem to recall Robert mentioning that they'd only recently fixed
 DTrace to cope with probes in static functions.  Maybe you need to
 get an update?

That rings a bell.  Can we get a more precise designation on what 
version of DTrace we support?  And where can one get that required 
update?

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

[HACKERS] DTrace enabled build fails

[Peter Eisentraut]

/usr/sbin/dtrace  -G -s utils/probes.d access/SUBSYS.o bootstrap/SUBSYS.o 
catalog/SUBSYS.o parser/SUBSYS.o commands/SUBSYS.o executor/SUBSYS.o 
lib/SUBSYS.o libpq/SUBSYS.o main/SUBSYS.o nodes/SUBSYS.o optimizer/SUBSYS.o 
port/SUBSYS.o postmaster/SUBSYS.o regex/SUBSYS.o rewrite/SUBSYS.o 
storage/SUBSYS.o tcop/SUBSYS.o utils/SUBSYS.o ../../src/timezone/SUBSYS.o -o 
utils/probes.o
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline 
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -g 
-L../../src/port -L/opt/csw/lib/ -Wl,-R'/export/home/pei/devel/pg-install/lib'  
access/SUBSYS.o bootstrap/SUBSYS.o catalog/SUBSYS.o parser/SUBSYS.o 
commands/SUBSYS.o executor/SUBSYS.o lib/SUBSYS.o libpq/SUBSYS.o main/SUBSYS.o 
nodes/SUBSYS.o optimizer/SUBSYS.o port/SUBSYS.o postmaster/SUBSYS.o 
regex/SUBSYS.o rewrite/SUBSYS.o storage/SUBSYS.o tcop/SUBSYS.o utils/SUBSYS.o 
../../src/timezone/SUBSYS.o utils/probes.o ../../src/port/libpgport_srv.a -lrt 
-lsocket -lm -o postgres
Undefined   first referenced
 symbol in file
AbortTransactionutils/probes.o
CommitTransaction   utils/probes.o
ld: fatal: Symbol referencing errors. No output written to postgres
collect2: ld returned 1 exit status
gmake: *** [postgres] Error 1

Does it not like static functions?

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [HACKERS] DTrace enabled build fails

[Tom Lane]

Peter Eisentraut [EMAIL PROTECTED] writes:
 Does it not like static functions?

I seem to recall Robert mentioning that they'd only recently fixed
DTrace to cope with probes in static functions.  Maybe you need to
get an update?

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [HACKERS] DTrace?

[Jim C. Nasby]

There is work going on to add dtrace support to FreeBSD, which I expect
could migrate to the other BSDs as well. AFAIK *BSD is actually more
popular among the developers than linux, so dtrace support could well
happen.

On Mon, Nov 07, 2005 at 02:29:12PM -0700, Aly Dharshi wrote:
 From what I understand DTrace is rather tough to use. Secondly it will 
 provide Solaris only information, so if you are suggesting helpfulness for 
 just Solaris, then yes it would be. I don't think that DTrace is available 
 for Solaris 8 and 9, the company I work for is still on 8 with possibly 
 some 7's hanging around somewhere, which is where I expect alot of people 
 to still be, Solaris 10 hasn't been adopted as widely as expected by Sun, 
 it may gain some momentum with OpenSolaris, but we shall have to see.
 
 karen hill wrote:
 I skimmed the thread Spinlocks, yet again: analysis
 and proposed patches.  Wouldn't Solaris 10's DTrace
 be helpful in seeing what's going on?  It seems DTrace
 was meant for these types of problems.
 
 
  
 __ 
 Yahoo! FareChase: Search multiple travel sites in one click.
 http://farechase.yahoo.com
 
 ---(end of broadcast)---
 TIP 3: Have you checked our extensive FAQ?
 
http://www.postgresql.org/docs/faq
 
 -- 
 Aly S.P Dharshi
 [EMAIL PROTECTED]
 
A good speech is like a good dress
 that's short enough to be interesting
 and long enough to cover the subject
 
 ---(end of broadcast)---
 TIP 3: Have you checked our extensive FAQ?
 
   http://www.postgresql.org/docs/faq
 

-- 
Jim C. Nasby, Sr. Engineering Consultant  [EMAIL PROTECTED]
Pervasive Software  http://pervasive.comwork: 512-231-6117
vcard: http://jim.nasby.net/pervasive.vcf   cell: 512-569-9461

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

[HACKERS] DTrace?

[karen hill]

I skimmed the thread Spinlocks, yet again: analysis
and proposed patches.  Wouldn't Solaris 10's DTrace
be helpful in seeing what's going on?  It seems DTrace
was meant for these types of problems.



__ 
Yahoo! FareChase: Search multiple travel sites in one click.
http://farechase.yahoo.com

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

Re: [HACKERS] DTrace?

[Aly Dharshi]

From what I understand DTrace is rather tough to use. Secondly it will provide 
Solaris only information, so if you are suggesting helpfulness for just Solaris, 
then yes it would be. I don't think that DTrace is available for Solaris 8 and 
9, the company I work for is still on 8 with possibly some 7's hanging around 
somewhere, which is where I expect alot of people to still be, Solaris 10 hasn't 
been adopted as widely as expected by Sun, it may gain some momentum with 
OpenSolaris, but we shall have to see.


karen hill wrote:

I skimmed the thread Spinlocks, yet again: analysis
and proposed patches.  Wouldn't Solaris 10's DTrace
be helpful in seeing what's going on?  It seems DTrace
was meant for these types of problems.



__ 
Yahoo! FareChase: Search multiple travel sites in one click.

http://farechase.yahoo.com

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


--
Aly S.P Dharshi
[EMAIL PROTECTED]

 A good speech is like a good dress
  that's short enough to be interesting
  and long enough to cover the subject

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

  http://www.postgresql.org/docs/faq

Re: [HACKERS] DTrace Probes?

[Nicolai Tufar]

On 6/17/05, Josh Berkus josh@agliodbs.com wrote:
 Hey, Folks,
 
 I need to find someone who's really interesed in working with DTrace.  Sun
 has offered to help put DTrace probes into PostgreSQL for advanced
 profiling, but need to know where to probe.   Anyone?
 
 I'm afraid that I won't get around to this quickly enough.

I played a little with DTrace probes when Solaris 10 just came
out. DTrace is useful when you have no source code of application
or when you are collecting statistics on a live system. Otherwise
it is not much different from gprof apart, maybe, that it can collect
statistics about kernel syscalls. 

Anyways, DTrace is a very powerful yet lightweight tool. Creating a 
strace program to attach to a running PostgreSQL instance and collect 
statistics will be a nice thing to do. We may even find some bottlenecks
in our code. I can volunteer to do it but I do not have a through understanding
of PostgreSQL internals. 

 --Josh
Regards, 
Nicolai

---(end of broadcast)---
TIP 8: explain analyze is your friend

[HACKERS] DTrace Probes?

[Josh Berkus]

Hey, Folks,

I need to find someone who's really interesed in working with DTrace.  Sun 
has offered to help put DTrace probes into PostgreSQL for advanced 
profiling, but need to know where to probe.   Anyone?

I'm afraid that I won't get around to this quickly enough.

-- 
--Josh

Josh Berkus
Aglio Database Solutions
San Francisco

---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings