Thread: Inefficient handling of LO-restore + Patch
I've detected that the restoring of large objects may consume huge amounts of diskspace when using unusual blocksizes (e.g. 32KB). My setup is Postgresql-7.2.1 + 32KB blocks + LOBLKSIZE 16KB, a unusual combination I think, , because this setup gave the very best performance. I wanted to restore a database containing 2 gigabytes of large objects, and noticed that it took around 6 gigabytes of diskspace to finish. After it finished I ran "VACUUM FULL VERBOSE pg_largeobject", and had around 140000 of live tuples, and around 480000 of dead tuples (I don't remember the exact values, but I think there were 3 times dead tuples to live tuples). I checked the pg_dump sources and found out that data is writen in 4KB chunks to the large object. Since in my database the LO tuples are 16KB each, that would mean: 1. write 4KB -> have 1 live 4KB tuple 2. write 4KB -> 1 live 8KB tuple and 1 dead 4KB tuple 3. write 4KB -> 1 live 12KB tuple and 2 dead tuples 3. write 4KB -> 1 live 16KB tuple and 3 dead tuples So creating a 16KB chunk took 16+12+8+4 => 40KB of diskspace, so recovering 2GB large objects takes around 40/16 * 2 => 5GB diskspace and leaves 3 times the number of dead tuples (supposing all LO's have sizes which are multples of 16KB). I've written a patch which buffers LO's in 32KB blocks and tested again, and had 140000 live tuples and nearly no dead tuples (around 10000, I'm still not sure where they're coming from). Is there a better way to fix this? Can I post the patch to this list (~150 lines).And I did not find out how I can detectthe large object chunksize, either from getting it from the headers (include "storage/large_object.h" did not work)or how to get it from the database I restore to. Any hints? Best regards, Mario Weilguni
"Mario Weilguni" <mario.weilguni@icomedias.com> writes:
> And I did not find out how I can detect the large object
> chunksize, either from getting it from the headers (include
> "storage/large_object.h" did not work)
Why not?
Still, it might make sense to move the LOBLKSIZE definition into
pg_config.h, since as you say it's of some interest to clients like
pg_dump.
regards, tom lane
Am Donnerstag, 11. April 2002 17:44 schrieb Tom Lane:
> "Mario Weilguni" <mario.weilguni@icomedias.com> writes:
> > And I did not find out how I can detect the large object
> > chunksize, either from getting it from the headers (include
> > "storage/large_object.h" did not work)
>
You did not answer if it's ok to post the patch, hope it's ok:
==================================
diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_archiver.c
postgresql-7.2.1/src/bin/pg_dump/pg_backup_archiver.c
--- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_archiver.c Mon Feb 11
01:18:20 2002
+++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_archiver.c Thu Apr 11 10:41:09
2002
@@ -819,6 +819,9 @@ AH->createdBlobXref = 1; }
+ /* Initialize the LO Buffer */
+ AH->lo_buf_used = 0;
+ /* * Start long-running TXs if necessary */
@@ -848,6 +851,19 @@voidEndRestoreBlob(ArchiveHandle *AH, Oid oid){
+ if(AH->lo_buf_used > 0) {
+ /* Write remaining bytes from the LO buffer */
+ int res;
+ res = lo_write(AH->connection, AH->loFd, (void *) AH->lo_buf,
AH->lo_buf_used);
+
+ ahlog(AH, 5, "wrote remaining %d bytes of large object data (result =
%d)\n",
+ (int)AH->lo_buf_used, res);
+ if (res != AH->lo_buf_used)
+ die_horribly(AH, modulename, "could not write to large object (result: %d,
expected: %d)\n",
+ res, AH->lo_buf_used);
+ AH->lo_buf_used = 0;
+ }
+ lo_close(AH->connection, AH->loFd); AH->writingBlob = 0;
@@ -1228,14 +1244,27 @@ if (AH->writingBlob) {
- res = lo_write(AH->connection, AH->loFd, (void *) ptr, size * nmemb);
- ahlog(AH, 5, "wrote %d bytes of large object data (result = %d)\n",
- (int) (size * nmemb), res);
- if (res != size * nmemb)
+ if(AH->lo_buf_used + size * nmemb > AH->lo_buf_size) {
+ /* Split LO buffer */
+ int remaining = AH->lo_buf_size - AH->lo_buf_used;
+ int slack = nmemb * size - remaining;
+
+ memcpy(AH->lo_buf + AH->lo_buf_used, ptr, remaining);
+ res = lo_write(AH->connection, AH->loFd, AH->lo_buf, AH->lo_buf_size);
+ ahlog(AH, 5, "wrote %d bytes of large object data (result = %d)\n",
+ AH->lo_buf_size, res);
+ if (res != AH->lo_buf_size) die_horribly(AH, modulename, "could not write to large object
(result:%d,
expected: %d)\n",
- res, (int) (size * nmemb));
+ res, AH->lo_buf_size);
+ memcpy(AH->lo_buf, ptr + remaining, slack);
+ AH->lo_buf_used = slack;
+ } else {
+ /* LO Buffer is still large enough, buffer it */
+ memcpy(AH->lo_buf + AH->lo_buf_used, ptr, size * nmemb);
+ AH->lo_buf_used += size * nmemb;
+ }
- return res;
+ return size * nmemb; } else if (AH->gzOut) {
diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_archiver.h
postgresql-7.2.1/src/bin/pg_dump/pg_backup_archiver.h
--- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_archiver.h Mon Nov 5
18:46:30 2001
+++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_archiver.h Thu Apr 11 10:41:14
2002
@@ -41,6 +41,7 @@#include <errno.h>#include "pqexpbuffer.h"
+#define LOBBUFSIZE 32768#ifdef HAVE_LIBZ#include <zlib.h>
@@ -240,6 +241,9 @@ RestoreOptions *ropt; /* Used to check restore options in
*ahwrite etc */
+ void *lo_buf;
+ int lo_buf_used;
+ int lo_buf_size;} ArchiveHandle;typedef struct _tocEntry
diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_custom.c
postgresql-7.2.1/src/bin/pg_dump/pg_backup_custom.c
--- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_custom.c Wed Nov 28
00:48:12 2001
+++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_custom.c Thu Apr 11 10:42:45
2002
@@ -153,6 +153,12 @@ if (ctx->zp == NULL) die_horribly(AH, modulename, "out of memory\n");
+ /* Initialize LO buffering */
+ AH->lo_buf_size = LOBBUFSIZE;
+ AH->lo_buf = (void *)malloc(LOBBUFSIZE);
+ if(AH->lo_buf == NULL)
+ die_horribly(AH, modulename, "out of memory\n");
+ /* * zlibOutSize is the buffer size we tell zlib it can output to. We * actually allocate one extra byte
becausesome routines want to
diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_files.c
postgresql-7.2.1/src/bin/pg_dump/pg_backup_files.c
--- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_files.c Thu Oct 25
07:49:52 2001
+++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_files.c Thu Apr 11 10:43:01
2002
@@ -113,6 +113,12 @@ AH->formatData = (void *) ctx; ctx->filePos = 0;
+ /* Initialize LO buffering */
+ AH->lo_buf_size = LOBBUFSIZE;
+ AH->lo_buf = (void *)malloc(LOBBUFSIZE);
+ if(AH->lo_buf == NULL)
+ die_horribly(AH, modulename, "out of memory\n");
+ /* * Now open the TOC file */
diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_null.c
postgresql-7.2.1/src/bin/pg_dump/pg_backup_null.c
--- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_null.c Wed Jun 27 23:21:37
2001
+++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_null.c Thu Apr 11 10:44:53 2002
@@ -64,7 +64,6 @@ */ if (AH->mode == archModeRead) die_horribly(AH, NULL, "this format cannot be
read\n");
-}/*
diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_tar.c
postgresql-7.2.1/src/bin/pg_dump/pg_backup_tar.c
--- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_tar.c Sun Oct 28 07:25:58
2001
+++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_tar.c Thu Apr 11 10:44:08 2002
@@ -157,6 +157,12 @@ ctx = (lclContext *) malloc(sizeof(lclContext)); AH->formatData = (void *) ctx;
ctx->filePos= 0;
+
+ /* Initialize LO buffering */
+ AH->lo_buf_size = LOBBUFSIZE;
+ AH->lo_buf = (void *)malloc(LOBBUFSIZE);
+ if(AH->lo_buf == NULL)
+ die_horribly(AH, modulename, "out of memory\n"); /* * Now open the TOC file
============================
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>"Mario Weilguni" <mario.weilguni@icomedias.com> writes:
>> And I did not find out how I can detect the large object
>> chunksize, either from getting it from the headers (include
>> "storage/large_object.h" did not work)
>
>Why not?
>
>Still, it might make sense to move the LOBLKSIZE definition into
>pg_config.h, since as you say it's of some interest to clients like
>pg_dump.
I tried another approach to detect the LOBLKSIZE of the destination server:
* at restore time, create a LO large enough to be split in two parts (e.g. BLCSIZE+1)
* select octet_length(data) from pg_largeobject where loid=OIDOFOBJECT and pageno=0
* select lo_unlink(OIDOFOBJECT)
IMO this gives the advantage that the LOBLKSIZE is taken from the database I'm restoring to, and not a constant defined
atcompile time. Otherwise, it wastes an OID.
Is there a way to get compile-time settings (such as BLCSIZE, LOBLKSIZE and such via functions - e.g.
select pginternal('BLCSIZE') or something similar?
I tested with and without my patch against 2 Gigabytes of LO's using MD5, and got exactly the same result on all 25000
largeobjects. So I think my patch is safe. If there's interest for integration into pg_dump, I'll prepare a patch for
thecurrent CVS version.
And how about getting database internals via SQL-functions - e.g. getting BLCSIZE, LOBBLCSIZE?
-----Ursprüngliche Nachricht-----
Von: Tom Lane [mailto:tgl@sss.pgh.pa.us]
Gesendet: Montag, 15. April 2002 16:32
An: Mario Weilguni
Cc: pgsql-hackers@postgresql.org
Betreff: Re: [HACKERS] Inefficient handling of LO-restore + Patch
"Mario Weilguni" <mario.weilguni@icomedias.com> writes:
> * select octet_length(data) from pg_largeobject where loid=OIDOFOBJECT and pageno=0
This really should not work if you're not superuser. Right now it does,
but I think that's an oversight in the default permissions settings for
system tables. Anyone object if I turn off public read access to
pg_largeobject?
regards, tom lane
"Mario Weilguni" <mario.weilguni@icomedias.com> writes:
> * select octet_length(data) from pg_largeobject where loid=OIDOFOBJECT and pageno=0
This really should not work if you're not superuser. Right now it does,
but I think that's an oversight in the default permissions settings for
system tables. Anyone object if I turn off public read access to
pg_largeobject?
regards, tom lane
Tom Lane wrote: > "Mario Weilguni" <mario.weilguni@icomedias.com> writes: > > * select octet_length(data) from pg_largeobject where loid=OIDOFOBJECT and pageno=0 > > This really should not work if you're not superuser. Right now it does, > but I think that's an oversight in the default permissions settings for > system tables. Anyone object if I turn off public read access to > pg_largeobject? Please do whatever you can to tighten it up. I thought we needed to keep read access so people could get to their large objects, but maybe not. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Anyone object if I turn off public read access to
>> pg_largeobject?
> Please do whatever you can to tighten it up. I thought we needed to keep
> read access so people could get to their large objects, but maybe not.
Yeah, right after sending that message I remembered that we had already
discussed this and concluded it would break clients :-(.
There's really no security for large objects anyway, since if you know
or can guess the OID of one you can read (or write!) it regardless.
Not much point in turning off read access on pg_largeobject unless we
rethink that ...
regards, tom lane
Your patch has been added to the PostgreSQL unapplied patches list at: http://candle.pha.pa.us/cgi-bin/pgpatches I will try to apply it within the next 48 hours. --------------------------------------------------------------------------- Mario Weilguni wrote: > Am Donnerstag, 11. April 2002 17:44 schrieb Tom Lane: > > "Mario Weilguni" <mario.weilguni@icomedias.com> writes: > > > And I did not find out how I can detect the large object > > > chunksize, either from getting it from the headers (include > > > "storage/large_object.h" did not work) > > > > You did not answer if it's ok to post the patch, hope it's ok: > ================================== > diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_archiver.c > postgresql-7.2.1/src/bin/pg_dump/pg_backup_archiver.c > --- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_archiver.c Mon Feb 11 > 01:18:20 2002 > +++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_archiver.c Thu Apr 11 10:41:09 > 2002 > @@ -819,6 +819,9 @@ > AH->createdBlobXref = 1; > } > > + /* Initialize the LO Buffer */ > + AH->lo_buf_used = 0; > + > /* > * Start long-running TXs if necessary > */ > @@ -848,6 +851,19 @@ > void > EndRestoreBlob(ArchiveHandle *AH, Oid oid) > { > + if(AH->lo_buf_used > 0) { > + /* Write remaining bytes from the LO buffer */ > + int res; > + res = lo_write(AH->connection, AH->loFd, (void *) AH->lo_buf, > AH->lo_buf_used); > + > + ahlog(AH, 5, "wrote remaining %d bytes of large object data (result = > %d)\n", > + (int)AH->lo_buf_used, res); > + if (res != AH->lo_buf_used) > + die_horribly(AH, modulename, "could not write to large object (result: %d, > expected: %d)\n", > + res, AH->lo_buf_used); > + AH->lo_buf_used = 0; > + } > + > lo_close(AH->connection, AH->loFd); > AH->writingBlob = 0; > > @@ -1228,14 +1244,27 @@ > > if (AH->writingBlob) > { > - res = lo_write(AH->connection, AH->loFd, (void *) ptr, size * nmemb); > - ahlog(AH, 5, "wrote %d bytes of large object data (result = %d)\n", > - (int) (size * nmemb), res); > - if (res != size * nmemb) > + if(AH->lo_buf_used + size * nmemb > AH->lo_buf_size) { > + /* Split LO buffer */ > + int remaining = AH->lo_buf_size - AH->lo_buf_used; > + int slack = nmemb * size - remaining; > + > + memcpy(AH->lo_buf + AH->lo_buf_used, ptr, remaining); > + res = lo_write(AH->connection, AH->loFd, AH->lo_buf, AH->lo_buf_size); > + ahlog(AH, 5, "wrote %d bytes of large object data (result = %d)\n", > + AH->lo_buf_size, res); > + if (res != AH->lo_buf_size) > die_horribly(AH, modulename, "could not write to large object (result: %d, > expected: %d)\n", > - res, (int) (size * nmemb)); > + res, AH->lo_buf_size); > + memcpy(AH->lo_buf, ptr + remaining, slack); > + AH->lo_buf_used = slack; > + } else { > + /* LO Buffer is still large enough, buffer it */ > + memcpy(AH->lo_buf + AH->lo_buf_used, ptr, size * nmemb); > + AH->lo_buf_used += size * nmemb; > + } > > - return res; > + return size * nmemb; > } > else if (AH->gzOut) > { > diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_archiver.h > postgresql-7.2.1/src/bin/pg_dump/pg_backup_archiver.h > --- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_archiver.h Mon Nov 5 > 18:46:30 2001 > +++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_archiver.h Thu Apr 11 10:41:14 > 2002 > @@ -41,6 +41,7 @@ > #include <errno.h> > > #include "pqexpbuffer.h" > +#define LOBBUFSIZE 32768 > > #ifdef HAVE_LIBZ > #include <zlib.h> > @@ -240,6 +241,9 @@ > > RestoreOptions *ropt; /* Used to check restore options in > * ahwrite etc */ > + void *lo_buf; > + int lo_buf_used; > + int lo_buf_size; > } ArchiveHandle; > > typedef struct _tocEntry > diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_custom.c > postgresql-7.2.1/src/bin/pg_dump/pg_backup_custom.c > --- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_custom.c Wed Nov 28 > 00:48:12 2001 > +++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_custom.c Thu Apr 11 10:42:45 > 2002 > @@ -153,6 +153,12 @@ > if (ctx->zp == NULL) > die_horribly(AH, modulename, "out of memory\n"); > > + /* Initialize LO buffering */ > + AH->lo_buf_size = LOBBUFSIZE; > + AH->lo_buf = (void *)malloc(LOBBUFSIZE); > + if(AH->lo_buf == NULL) > + die_horribly(AH, modulename, "out of memory\n"); > + > /* > * zlibOutSize is the buffer size we tell zlib it can output to. We > * actually allocate one extra byte because some routines want to > diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_files.c > postgresql-7.2.1/src/bin/pg_dump/pg_backup_files.c > --- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_files.c Thu Oct 25 > 07:49:52 2001 > +++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_files.c Thu Apr 11 10:43:01 > 2002 > @@ -113,6 +113,12 @@ > AH->formatData = (void *) ctx; > ctx->filePos = 0; > > + /* Initialize LO buffering */ > + AH->lo_buf_size = LOBBUFSIZE; > + AH->lo_buf = (void *)malloc(LOBBUFSIZE); > + if(AH->lo_buf == NULL) > + die_horribly(AH, modulename, "out of memory\n"); > + > /* > * Now open the TOC file > */ > diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_null.c > postgresql-7.2.1/src/bin/pg_dump/pg_backup_null.c > --- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_null.c Wed Jun 27 23:21:37 > 2001 > +++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_null.c Thu Apr 11 10:44:53 2002 > @@ -64,7 +64,6 @@ > */ > if (AH->mode == archModeRead) > die_horribly(AH, NULL, "this format cannot be read\n"); > - > } > > /* > diff -Nur postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_tar.c > postgresql-7.2.1/src/bin/pg_dump/pg_backup_tar.c > --- postgresql-7.2.1-orig/src/bin/pg_dump/pg_backup_tar.c Sun Oct 28 07:25:58 > 2001 > +++ postgresql-7.2.1/src/bin/pg_dump/pg_backup_tar.c Thu Apr 11 10:44:08 2002 > @@ -157,6 +157,12 @@ > ctx = (lclContext *) malloc(sizeof(lclContext)); > AH->formatData = (void *) ctx; > ctx->filePos = 0; > + > + /* Initialize LO buffering */ > + AH->lo_buf_size = LOBBUFSIZE; > + AH->lo_buf = (void *)malloc(LOBBUFSIZE); > + if(AH->lo_buf == NULL) > + die_horribly(AH, modulename, "out of memory\n"); > > /* > * Now open the TOC file > ============================ > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 4: Don't 'kill -9' the postmaster > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Patch applied. Thanks. --------------------------------------------------------------------------- Mario Weilguni wrote: > Am Donnerstag, 11. April 2002 17:44 schrieb Tom Lane: > > "Mario Weilguni" <mario.weilguni@icomedias.com> writes: > > > And I did not find out how I can detect the large object > > > chunksize, either from getting it from the headers (include > > > "storage/large_object.h" did not work) > > > > You did not answer if it's ok to post the patch, hope it's ok: -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
This patch does not compile correctly: pg_backup_archiver.c: In function `ahwrite': pg_backup_archiver.c:1252: warning: pointer of type `void *' used in arithmetic pg_backup_archiver.c:1259: warning: pointer of type `void *' used in arithmetic pg_backup_archiver.c:1263: warning: pointer of type `void *' used in arithmetic make: *** [pg_backup_archiver.o] Error 1 Bruce Momjian writes: > > Patch applied. Thanks. > > --------------------------------------------------------------------------- > > > Mario Weilguni wrote: > > Am Donnerstag, 11. April 2002 17:44 schrieb Tom Lane: > > > "Mario Weilguni" <mario.weilguni@icomedias.com> writes: > > > > And I did not find out how I can detect the large object > > > > chunksize, either from getting it from the headers (include > > > > "storage/large_object.h" did not work) > > > > > > > You did not answer if it's ok to post the patch, hope it's ok: > > -- Peter Eisentraut peter_e@gmx.net
At 16:46 15/04/02 +0200, Mario Weilguni wrote: >And how about getting database internals via SQL-functions - e.g. getting >BLCSIZE, LOBBLCSIZE? ISTM that there would be some merit in making a selection of compile-time options available via SQL. Is this worth considering?
OK, I have applied the following patch to fix these warnings. However, I need Mario to confirm these are the right changes. Thanks. --------------------------------------------------------------------------- Peter Eisentraut wrote: > This patch does not compile correctly: > > pg_backup_archiver.c: In function `ahwrite': > pg_backup_archiver.c:1252: warning: pointer of type `void *' used in arithmetic > pg_backup_archiver.c:1259: warning: pointer of type `void *' used in arithmetic > pg_backup_archiver.c:1263: warning: pointer of type `void *' used in arithmetic > make: *** [pg_backup_archiver.o] Error 1 > > > Bruce Momjian writes: > > > > > Patch applied. Thanks. > > > > --------------------------------------------------------------------------- > > > > > > Mario Weilguni wrote: > > > Am Donnerstag, 11. April 2002 17:44 schrieb Tom Lane: > > > > "Mario Weilguni" <mario.weilguni@icomedias.com> writes: > > > > > And I did not find out how I can detect the large object > > > > > chunksize, either from getting it from the headers (include > > > > > "storage/large_object.h" did not work) > > > > > > > > > > You did not answer if it's ok to post the patch, hope it's ok: > > > > > > -- > Peter Eisentraut peter_e@gmx.net > > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 Index: src/bin/pg_dump/pg_backup_archiver.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v retrieving revision 1.43 diff -c -r1.43 pg_backup_archiver.c *** src/bin/pg_dump/pg_backup_archiver.c 24 Apr 2002 02:21:04 -0000 1.43 --- src/bin/pg_dump/pg_backup_archiver.c 24 Apr 2002 14:01:15 -0000 *************** *** 1249,1266 **** int remaining = AH->lo_buf_size - AH->lo_buf_used; int slack = nmemb * size - remaining; ! memcpy(AH->lo_buf + AH->lo_buf_used, ptr, remaining); res = lo_write(AH->connection, AH->loFd, AH->lo_buf, AH->lo_buf_size); ahlog(AH, 5, "wrote %d bytes of large object data (result = %d)\n", AH->lo_buf_size, res); if (res != AH->lo_buf_size) die_horribly(AH, modulename, "could not write to large object (result: %d, expected: %d)\n", res, AH->lo_buf_size); ! memcpy(AH->lo_buf, ptr + remaining, slack); AH->lo_buf_used = slack; } else { /* LO Buffer is still large enough, buffer it */ ! memcpy(AH->lo_buf + AH->lo_buf_used, ptr, size * nmemb); AH->lo_buf_used += size * nmemb; } --- 1249,1266 ---- int remaining = AH->lo_buf_size - AH->lo_buf_used; int slack = nmemb * size - remaining; ! memcpy((char *)AH->lo_buf + AH->lo_buf_used, ptr, remaining); res = lo_write(AH->connection, AH->loFd, AH->lo_buf, AH->lo_buf_size); ahlog(AH, 5, "wrote %d bytes of large object data (result = %d)\n", AH->lo_buf_size, res); if (res != AH->lo_buf_size) die_horribly(AH, modulename, "could not write to large object (result: %d, expected: %d)\n", res, AH->lo_buf_size); ! memcpy(AH->lo_buf, (char *)ptr + remaining, slack); AH->lo_buf_used = slack; } else { /* LO Buffer is still large enough, buffer it */ ! memcpy((char *)AH->lo_buf + AH->lo_buf_used, ptr, size * nmemb); AH->lo_buf_used += size * nmemb; }
I wanted to correct the patch this evening after work, and will check your changes. Thanks! -----Ursprüngliche Nachricht----- Von: Bruce Momjian [mailto:pgman@candle.pha.pa.us] Gesendet: Mittwoch, 24. April 2002 16:03 An: Peter Eisentraut Cc: Mario Weilguni; pgsql-hackers@postgresql.org Betreff: Re: [HACKERS] Inefficient handling of LO-restore + Patch OK, I have applied the following patch to fix these warnings. However, I need Mario to confirm these are the right changes. Thanks. --------------------------------------------------------------------------- Peter Eisentraut wrote: > This patch does not compile correctly: > > pg_backup_archiver.c: In function `ahwrite': > pg_backup_archiver.c:1252: warning: pointer of type `void *' used in arithmetic > pg_backup_archiver.c:1259: warning: pointer of type `void *' used in arithmetic > pg_backup_archiver.c:1263: warning: pointer of type `void *' used in arithmetic > make: *** [pg_backup_archiver.o] Error 1 > > > Bruce Momjian writes: > > > > > Patch applied. Thanks. > > > > --------------------------------------------------------------------------- > > > > > > Mario Weilguni wrote: > > > Am Donnerstag, 11. April 2002 17:44 schrieb Tom Lane: > > > > "Mario Weilguni" <mario.weilguni@icomedias.com> writes: > > > > > And I did not find out how I can detect the large object > > > > > chunksize, either from getting it from the headers (include > > > > > "storage/large_object.h" did not work) > > > > > > > > > > You did not answer if it's ok to post the patch, hope it's ok: > > > > > > -- > Peter Eisentraut peter_e@gmx.net > > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Philip Warner <pjw@rhyme.com.au> writes:
> At 16:46 15/04/02 +0200, Mario Weilguni wrote:
>> And how about getting database internals via SQL-functions - e.g. getting
>> BLCSIZE, LOBBLCSIZE?
> ISTM that there would be some merit in making a selection of compile-time
> options available via SQL. Is this worth considering?
This could usefully be combined with the nearby thread about recording
configuration options (started by Thomas). I'd be inclined to make it
a low-footprint affair where you do something like
select compilation_options();
and you get back a long textual list of var=value settings, say
VERSION=7.3devel
PLATFORM=hppa-hp-hpux10.20, compiled by GCC 2.95.3
BLCKSZ=8192
MULTIBYTE=yes
etc etc etc etc
This would solve the diagnostic need as-is, and it doesn't seem
unreasonable to me to expect applications to look through the output
for a particular line if they need to get the state of a specific
configuration option. It's also trivial to extend/modify as the set
of options changes over time.
regards, tom lane
Am Mittwoch, 24. April 2002 16:03 schrieb Bruce Momjian: > OK, I have applied the following patch to fix these warnings. However, > I need Mario to confirm these are the right changes. Thanks. I've checked it and works fine, but the memcpy() prototype says it should be void pointers. Will this give errors with non-gcc compilers?
Tom Lane writes: > This could usefully be combined with the nearby thread about recording > configuration options (started by Thomas). I'd be inclined to make it > a low-footprint affair where you do something like > > select compilation_options(); > > and you get back a long textual list of var=value settings, say > > VERSION=7.3devel > PLATFORM=hppa-hp-hpux10.20, compiled by GCC 2.95.3 > BLCKSZ=8192 > MULTIBYTE=yes > etc etc etc etc This assumes that compilation options only matter in the server and that only SQL users would be interested in them. In fact, compilation options affect client and utility programs as well, and it's not unusual to have a wild mix (if only unintentional). IMHO, the best place to put this information is in the version output, as in: $ ./psql --version psql (PostgreSQL) 7.3devel contains support for: readline An SQL interface in addition to that would be OK, too. But let's not dump everything into one SHOW command. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> select compilation_options();
> This assumes that compilation options only matter in the server and that
> only SQL users would be interested in them. In fact, compilation options
> affect client and utility programs as well, and it's not unusual to have a
> wild mix (if only unintentional).
Good point. It'd be worthwhile to have some way of extracting such
information from the clients as well.
> IMHO, the best place to put this information is in the version output, as
> in:
> $ ./psql --version
> psql (PostgreSQL) 7.3devel
> contains support for: readline
Is that sufficient? The clients probably are not affected by quite as
many config options as the server, but they still have a nontrivial
list. (Multibyte, SSL, Kerberos come to mind at once.) I'd not like
to see us assume that a one-line output format will do the job.
A way to interrogate the libpq being used by psql might be good too.
regards, tom lane
Mario Weilguni wrote: > Am Mittwoch, 24. April 2002 16:03 schrieb Bruce Momjian: > > OK, I have applied the following patch to fix these warnings. However, > > I need Mario to confirm these are the right changes. Thanks. > > I've checked it and works fine, but the memcpy() prototype says it should be > void pointers. Will this give errors with non-gcc compilers? No, it is fine. Anything can be cast _to_ a void pointer. You just can't do arithmetic on them. Are you sure you want to use 'void *' in your code. Looking at the backend large object code, I see char *: extern int inv_read(LargeObjectDesc *obj_desc, char *buf, int nbytes); extern int inv_write(LargeObjectDesc *obj_desc,char *buf, int nbytes); I guess my point is that these are just streams of bytes, _not_ really streams of items of unknown length. We know the length, and the length is char. This may simplify the code. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
On Wed, Apr 24, 2002 at 06:13:28PM -0400, Tom Lane wrote: > > Is that sufficient? The clients probably are not affected by quite as > many config options as the server, but they still have a nontrivial > list. (Multibyte, SSL, Kerberos come to mind at once.) I'd not like > to see us assume that a one-line output format will do the job. > > A way to interrogate the libpq being used by psql might be good too. I like the way perl handles this, for example perl -MExtUtils::Embed -e ccopts for options to cc used when compiling(-I and stuff) and perl -MExtUtils::Embed -e ldopts for options to ld used when compiling(-L and stuff). I think it would be really nice if we could have psql to ask its libpq to spew out something similiar. Then you could do stuff like cc -o ex ex.c `psql -ccopts -ldopts` and not having to worry about where the libraries are. -- Magnus