Re: [PATCHES] bgwriter stats
Magnus Hagander [EMAIL PROTECTED] writes: This seems quite a bizarre way to do things. Why wouldn't you implement this functionality by shipping messages to the stats collector? Would you suggest doing the same with the checkpoint counter, that's already in shared mem? I want to expose that number as well.. The shared-mem checkpoint counters serve an entirely different purpose: they're there to let backends detect when their requested checkpoint has been completed. They're not intended to count checkpoints over any long term (remember that sig_atomic_t need only be 8 bits wide). If you want to track stats about how many checkpoints have been done, I think that's a job for the stats collector. 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: [PATCHES] Win32 shmem
On Tue, Mar 20, 2007 at 07:41:32AM +0100, Magnus Hagander wrote:
Does it seem like I've overlooked anything obvious in this? I do get the
feeling that this is too simple, but I don't know exactly where the
problem is :-)
I think you do still need the on_shmem_exit detach callback. Consider
the situation where the postmaster is trying to reinitialize after a
child crash. The Unix code is designed to detach and destroy the old
segment then create a new one. If that's not what you want to do then
this code still seems not right.
Ok, will look into that. Haven't tested that scenario.
That was indeed so. Added in new version, attached.
There seem to be a lot of system calls not checked for failure here.
Do they really not have any failure possibilities?
I looked it over, and didn't find a lot. I found one or two (which are
now fixed). Are you referring to anything in particular?
//Magnus
/*-
*
* win32_shmem.c
* Implement shared memory using win32 facilities
*
* Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
* $PostgreSQL$
*
*-
*/
#include postgres.h
#include miscadmin.h
#include storage/ipc.h
#include storage/pg_shmem.h
unsigned long UsedShmemSegID = 0;
void *UsedShmemSegAddr = NULL;
static void pgwin32_SharedMemoryDelete(int status, Datum shmId);
/*
* Generate shared memory segment name. Expand the data directory, to generate
* an identifier unique for this data directory. Then replace all backslashes
* with forward slashes, since backslashes aren't permitted in global object names.
*
* XXX: What happens with junctions? It's only someone breaking things on purpose,
* and this is still better than before, but we might want to do something about
* that sometime in the future.
*/
static char *
GetSharedMemName(void)
{
char *retptr;
DWORD bufsize;
DWORD r;
char *cp;
bufsize = GetFullPathName(DataDir, 0, NULL, NULL);
if (bufsize == 0)
elog(FATAL, could not get size for full pathname of datadir %s: %lu,
DataDir, GetLastError());
retptr = malloc(bufsize+1+11); // 1 NULL and 11 for PostgreSQL
if (retptr == NULL)
elog(FATAL, could not allocate memory for shared memory name);
strcpy(retptr,PostgreSQL:);
r = GetFullPathName(DataDir, bufsize, retptr+11, NULL);
if (r == 0 || r bufsize)
elog(FATAL, could not generate full pathname for datadir %s: %lu,
DataDir, GetLastError());
for (cp = retptr; *cp; cp++)
if (*cp == '\\')
*cp = '/';
return retptr;
}
/*
* PGSharedMemoryIsInUse
*
* Is a previously-existing shmem segment still existing and in use?
*
* The point of this exercise is to detect the case where a prior postmaster
* crashed, but it left child backends that are still running. Therefore
* we only care about shmem segments that are associated with the intended
* DataDir. This is an important consideration since accidental matches of
* shmem segment IDs are reasonably common.
*
*/
bool
PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
{
char *szShareMem;
HANDLE hmap;
szShareMem = GetSharedMemName();
hmap = OpenFileMapping(FILE_MAP_READ, FALSE, szShareMem);
free(szShareMem);
if (hmap == NULL)
return false;
CloseHandle(hmap);
return true;
}
/*
* PGSharedMemoryCreate
*
* Create a shared memory segment of the given size and initialize its
* standard header.
*
* makePrivate means to always create a new segment, rather than attach to
* or recycle any existing segment. On win32, we always create a new segment,
* since there is no need for recycling (segments go away automatically
* when the last backend exits)
*
*/
PGShmemHeader *
PGSharedMemoryCreate(Size size, bool makePrivate, int port)
{
void *memAddress;
PGShmemHeader *hdr;
HANDLE hmap, hmap2;
char *szShareMem;
/* Room for a header? */
Assert(size MAXALIGN(sizeof(PGShmemHeader)));
szShareMem = GetSharedMemName();
UsedShmemSegAddr = NULL;
hmap = CreateFileMapping((HANDLE) 0x, /* Use the pagefile */
NULL, /* Default security attrs */
PAGE_READWRITE, /* Memory is Read/Write */
0L, /* Size Upper 32 Bits */
(DWORD) size, /* Size Lower 32 bits */
szShareMem);
if (!hmap)
ereport(FATAL,
(errmsg(could not create shared memory segment: %lu, GetLastError()),
errdetail(Failed system call was CreateFileMapping(size=%lu, name=%s), size, szShareMem)));
/*
* If the segment already existed, CreateFileMapping() will return a handle to the
* existing one.
*/
if (GetLastError() == ERROR_ALREADY_EXISTS)
{
/*
* When recycling a shared memory segment, it may take a short while
* before it gets dropped from the global namespace. So re-try after
*
Re: [PATCHES] Win32 shmem
Magnus Hagander [EMAIL PROTECTED] writes: I think you do still need the on_shmem_exit detach callback. Ok, will look into that. Haven't tested that scenario. That was indeed so. Added in new version, attached. If it handles the restart-after-backend-crash scenario and correctly locks out starting a fresh postmaster (after postmaster crash) until all old backends are gone, then it's probably ready to commit for more-widespread testing. I note that sysv_shmem contains some #ifdef WIN32 and #ifdef __CYGWIN__ kluges; will it now be possible to remove those, or will the Cygwin build still be using that code? regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Fix race condition in size functions
Patch applied by Alvaro. --- Michael Fuhr wrote: Fix a race condition that caused pg_database_size() and pg_tablespace_size() to fail if an object was removed between calls to ReadDir() and stat(). Per discussion in pgsql-hackers. http://archives.postgresql.org/pgsql-hackers/2007-03/msg00671.php -- Michael Fuhr [ Attachment, skipping... ] ---(end of broadcast)--- TIP 6: explain analyze is your friend -- Bruce Momjian [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] pgbench on mingw needs fflush
Magnus Hagander wrote: On Tue, Mar 13, 2007 at 05:09:15PM +0900, ITAGAKI Takahiro wrote: Tatsuo Ishii [EMAIL PROTECTED] wrote: Can we distinguish mingw case from others so that we could ifdef out the extra fflush()? The buffered stderr might be a bug of mingw After a little research, I found that MSDN says the buffered stderr is a specifications on Windows somehow, not a bug. setvbuf() is better solution for the problem. This is more simple and no need to use ifdef. I was just going to suggest this, because this is what we already use in the backend (src/backend/main/main.c). Applied, but with the #ifdefs, because that's cleaner and that's also how we do it in the backend. And the #ifdef documents we need it only on Win32. Without it, someone might remove it thinking Unix doesn't need it. -- Bruce Momjian [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] Make CLUSTER MVCC-safe
This patch makes CLUSTER MVCC-safe. Visibility information and update
chains are preserved like in VACUUM FULL.
I created a new generic rewriteheap-facility to handle rewriting tables
in a visibility-preserving manner. All the update chain tracking is done
in rewriteheap.c, the caller is responsible for supplying the stream of
tuples.
CLUSTER is currently the only user of the facility, but I'm envisioning
we might have other users in the future. For example, a version of
VACUUM FULL that rewrites the whole table. We could also use it to make
ALTER TABLE MVCC-safe, but there's some issues with that. For example,
what to do if RECENTLY_DEAD tuples don't satisfy a newly added constraint.
One complication in the implementation was the fact that heap_insert
overwrites the visibility information, and it doesn't write the full
tuple header to WAL. I ended up implementing a special-purpose
raw_heap_insert function instead, which is optimized for bulk inserting
a lot of tuples, knowing that we have exclusive access to the heap.
raw_heap_insert keeps the current buffer locked over calls, until it
gets full, and inserts the whole page to WAL as a single record using
the existing XLOG_HEAP_NEWPAGE record type.
This makes CLUSTER a more viable alternative to VACUUM FULL. One
motivation for making CLUSTER MVCC-safe is that if some poor soul runs
pg_dump to make a backup concurrently with CLUSTER, the clustered tables
will appear to be empty in the dump file.
The documentation doesn't anything about CLUSTER not being MVCC-safe, so
I suppose there's no need to touch the docs. I sent a doc patch earlier
to add a note about it, that doc patch should still be applied to older
release branches, IMO.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Index: src/backend/access/heap/heapam.c
===
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.228
diff -c -r1.228 heapam.c
*** src/backend/access/heap/heapam.c 9 Feb 2007 03:35:33 - 1.228
--- src/backend/access/heap/heapam.c 20 Mar 2007 17:55:16 -
***
*** 3284,3289
--- 3284,3326
return log_heap_update(reln, oldbuf, from, newbuf, newtup, true);
}
+ /*
+ * Perofrm XLogInsert of a full page image to WAL. Caller must make sure
+ * the page contents on disk are consistent with the page inserted to WAL.
+ */
+ XLogRecPtr
+ log_newpage(RelFileNode *rnode, BlockNumber blkno, Page page)
+ {
+ xl_heap_newpage xlrec;
+ XLogRecPtr recptr;
+ XLogRecData rdata[2];
+
+ /* NO ELOG(ERROR) from here till newpage op is logged */
+ START_CRIT_SECTION();
+
+ xlrec.node = *rnode;
+ xlrec.blkno = blkno;
+
+ rdata[0].data = (char *) xlrec;
+ rdata[0].len = SizeOfHeapNewpage;
+ rdata[0].buffer = InvalidBuffer;
+ rdata[0].next = (rdata[1]);
+
+ rdata[1].data = (char *) page;
+ rdata[1].len = BLCKSZ;
+ rdata[1].buffer = InvalidBuffer;
+ rdata[1].next = NULL;
+
+ recptr = XLogInsert(RM_HEAP_ID, XLOG_HEAP_NEWPAGE, rdata);
+
+ PageSetLSN(page, recptr);
+ PageSetTLI(page, ThisTimeLineID);
+
+ END_CRIT_SECTION();
+
+ return recptr;
+ }
+
static void
heap_xlog_clean(XLogRecPtr lsn, XLogRecord *record)
{
Index: src/backend/access/nbtree/nbtsort.c
===
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/nbtree/nbtsort.c,v
retrieving revision 1.110
diff -c -r1.110 nbtsort.c
*** src/backend/access/nbtree/nbtsort.c 9 Jan 2007 02:14:10 - 1.110
--- src/backend/access/nbtree/nbtsort.c 20 Mar 2007 17:58:20 -
***
*** 64,69
--- 64,70
#include postgres.h
+ #include access/heapam.h
#include access/nbtree.h
#include miscadmin.h
#include storage/smgr.h
***
*** 265,296
if (wstate-btws_use_wal)
{
/* We use the heap NEWPAGE record type for this */
! xl_heap_newpage xlrec;
! XLogRecPtr recptr;
! XLogRecData rdata[2];
!
! /* NO ELOG(ERROR) from here till newpage op is logged */
! START_CRIT_SECTION();
!
! xlrec.node = wstate-index-rd_node;
! xlrec.blkno = blkno;
!
! rdata[0].data = (char *) xlrec;
! rdata[0].len = SizeOfHeapNewpage;
! rdata[0].buffer = InvalidBuffer;
! rdata[0].next = (rdata[1]);
!
! rdata[1].data = (char *) page;
! rdata[1].len = BLCKSZ;
! rdata[1].buffer = InvalidBuffer;
! rdata[1].next = NULL;
!
! recptr = XLogInsert(RM_HEAP_ID, XLOG_HEAP_NEWPAGE, rdata);
!
! PageSetLSN(page, recptr);
! PageSetTLI(page, ThisTimeLineID);
!
! END_CRIT_SECTION();
}
else
{
--- 266,272
if (wstate-btws_use_wal)
{
/* We use the heap NEWPAGE record type for this */
! log_newpage(wstate-index-rd_node, blkno, page);
}
else
{
Index: src/backend/commands/Makefile
===
RCS file:
Re: [PATCHES] patch adding new regexp functions
Jeremy Drake wrote: The patch has been sitting in the unapplied patches queue for a while, and the inevitable bitrot finally caught up with it. This version of the patch is exactly the same as the one in the patches queue, except for using new, non-conflicting oids for the functions. Applied -- thanks for the patch. BTW, a few cosmetic comments: * #includes should be sorted alphabetically (unless another #include sort order rules take precedence, like including postgres.h first). * patch included some trailing CR line endings * IMHO using ++var in the increment component of a for loop is bad style in C (if only because it is needlessly inconsistent; good C++ style is not necessarily good C style) * Comments like /* get text type oid, too lazy to do it some other way */ do not exactly inspire confidence :) -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Code-Cleanup: char* - const char*
Neil Conway wrote: BTW, the preferred format for patches is context diffs, not unified diffs. FYI, for the type of diff he is supplying unified diffs are better because it is single-line changes, and you see the old/new lines next to each other; the developer's FAQ mentions this. -- Bruce Momjian [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
[PATCHES] HOT WIP Patch - Version 5.0
The version 5.0 of HOT WIP patch is attached. This fixes the VACUUM FULL issue with HOT. In all the earlier versions, I'd disabled VACUUM FULL. When we move the HOT-chain, we move the chains but don't carry the HOT_UPDATED or HEAP_ONLY flags and insert as many index entries as there are tuples in the chain. IOW the HOT-update is actually turned into a COLD chain. Apart from this, I'd to make some changes to the VACUUM FULL code so that the number of indexed tuples is counted correctly. With HOT, whenever a HEAP_ONLY tuple is moved, an additional index entry is generated and this needs to be taken into account. Please let me know comments/suggestions. Thanks, Pavan -- EnterpriseDBhttp://www.enterprisedb.com NewHOT-v5.0.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] patch adding new regexp functions
Neil Conway [EMAIL PROTECTED] writes: * Comments like /* get text type oid, too lazy to do it some other way */ do not exactly inspire confidence :) Surely the code was just using the TEXTOID macro? If so, it does not require a comment. If not, it should be fixed ... regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] patch adding new regexp functions
On Wed, 21 Mar 2007, Tom Lane wrote: Neil Conway [EMAIL PROTECTED] writes: * Comments like /* get text type oid, too lazy to do it some other way */ do not exactly inspire confidence :) Surely the code was just using the TEXTOID macro? If so, it does not require a comment. If not, it should be fixed ... Nope, it does get_fn_expr_argtype(fcinfo-flinfo, 0); The too lazy remark was that I thought there may be a better way (like the macro you mentioned) but did not go looking for it because I already had something that worked that I found in the manual. If you like, I can put together another patch that removes the param_type members from the structs and hardcodes TEXTOID in the proper places. BTW, should I be calling get_typlenbyvalalign on TEXTOID or are there macros for those also? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- We're only in it for the volume. -- Black Sabbath ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
