Thread: new patches
Two small patches:
1) make default NBuffers = DEF_MAXBACKENDS*2 as required by check inPostmasterMain().
2) check for QueryCancel in the copy command. Maybe we should do thesame in vacuum command (Vadim?).
*** src/include/miscadmin.h.orig Wed May 26 09:06:39 1999
--- src/include/miscadmin.h Sat Jun 12 20:01:10 1999
***************
*** 106,112 **** * default number of buffers in buffer pool * */
! #define NDBUFS 64 /***************************************************************************** * pdir.h --
*
--- 106,112 ---- * default number of buffers in buffer pool * */
! #define NDBUFS (2*DEF_MAXBACKENDS) /***************************************************************************** *
pdir.h -- *
*** src/backend/commands/copy.c.orig Sun May 30 09:01:01 1999
--- src/backend/commands/copy.c Sat Jun 12 20:23:51 1999
***************
*** 18,23 ****
--- 18,24 ---- #include <access/heapam.h> #include <tcop/dest.h>
+ #include "tcop/tcopprot.h" #include <fmgr.h> #include <miscadmin.h> #include <utils/builtins.h>
***************
*** 253,259 **** */ if (file_opened) {
! FreeFile(fp); file_opened = false; }
--- 254,265 ---- */ if (file_opened) {
! /*
! * This is unnecessary: files are closed automatically by
! * AtEOXact_Files() at transaction abort. -- dz
! */
!
! /* FreeFile(fp); */ file_opened = false; }
***************
*** 419,424 ****
--- 425,432 ---- while (HeapTupleIsValid(tuple = heap_getnext(scandesc, 0))) {
+ if (QueryCancel)
+ CancelQuery(); if (oids && !binary) {
***************
*** 691,696 ****
--- 699,707 ---- lineno = 0; while (!done) {
+ if (QueryCancel)
+ CancelQuery();
+ if (!binary) { #ifdef COPY_PATCH
--
Massimo Dal Zotto
+----------------------------------------------------------------------+
| Massimo Dal Zotto email: dz@cs.unitn.it |
| Via Marconi, 141 phone: ++39-0461534251 |
| 38057 Pergine Valsugana (TN) www: http://www.cs.unitn.it/~dz/ |
| Italy pgp: finger dz@tango.cs.unitn.it |
+----------------------------------------------------------------------+
Massimo Dal Zotto <dz@cs.unitn.it> writes:
> Two small patches:
> 1) make default NBuffers = DEF_MAXBACKENDS*2 as required by check in
> PostmasterMain().
I had proposed moving NDBUFS into config.h and fixing the default a few
days ago, but then forgot to do it. As things stand, if you increase
DEF_MAXBACKENDS at configure time, you'll get a postmaster that won't
start unless you give it a -B setting larger than default. This is bad,
and I agree with Massimo that we ought to make sure the default NBuffers
is one that will work with the default MaxBackends.
This patch is not quite right though, since it doesn't account for the
other part of PostmasterMain's condition (NBuffers >= 16). Will fix.
> 2) check for QueryCancel in the copy command. Maybe we should do the
> same in vacuum command (Vadim?).
I'm not too excited about adding QueryCancel support so soon before the
release, but the part of your patch that you didn't mention (diking out
the "file_opened" hack) is really a critical fix --- as the code stood
it would try to fclose() the same stdio file twice, which is disastrous
in most stdio libraries. I applied that part of it... good catch!
regards, tom lane
Massimo Dal Zotto wrote: > > 2) check for QueryCancel in the copy command. Maybe we should do the > same in vacuum command (Vadim?). Yes, as well as in other places - QueryCancel doesn't work when backend waits for lock. But not now... Vadim
> Two small patches:
>
> 1) make default NBuffers = DEF_MAXBACKENDS*2 as required by check in
> PostmasterMain().
Seems this line is gone in the current sources.
>
> 2) check for QueryCancel in the copy command. Maybe we should do the
> same in vacuum command (Vadim?).
Applied.
>
>
> *** src/include/miscadmin.h.orig Wed May 26 09:06:39 1999
> --- src/include/miscadmin.h Sat Jun 12 20:01:10 1999
> ***************
> *** 106,112 ****
> * default number of buffers in buffer pool
> *
> */
> ! #define NDBUFS 64
>
> /*****************************************************************************
> * pdir.h -- *
> --- 106,112 ----
> * default number of buffers in buffer pool
> *
> */
> ! #define NDBUFS (2*DEF_MAXBACKENDS)
>
> /*****************************************************************************
> * pdir.h -- *
> *** src/backend/commands/copy.c.orig Sun May 30 09:01:01 1999
> --- src/backend/commands/copy.c Sat Jun 12 20:23:51 1999
> ***************
> *** 18,23 ****
> --- 18,24 ----
>
> #include <access/heapam.h>
> #include <tcop/dest.h>
> + #include "tcop/tcopprot.h"
> #include <fmgr.h>
> #include <miscadmin.h>
> #include <utils/builtins.h>
> ***************
> *** 253,259 ****
> */
> if (file_opened)
> {
> ! FreeFile(fp);
> file_opened = false;
> }
>
> --- 254,265 ----
> */
> if (file_opened)
> {
> ! /*
> ! * This is unnecessary: files are closed automatically by
> ! * AtEOXact_Files() at transaction abort. -- dz
> ! */
> !
> ! /* FreeFile(fp); */
> file_opened = false;
> }
>
> ***************
> *** 419,424 ****
> --- 425,432 ----
>
> while (HeapTupleIsValid(tuple = heap_getnext(scandesc, 0)))
> {
> + if (QueryCancel)
> + CancelQuery();
>
> if (oids && !binary)
> {
> ***************
> *** 691,696 ****
> --- 699,707 ----
> lineno = 0;
> while (!done)
> {
> + if (QueryCancel)
> + CancelQuery();
> +
> if (!binary)
> {
> #ifdef COPY_PATCH
>
>
> --
> Massimo Dal Zotto
>
> +----------------------------------------------------------------------+
> | Massimo Dal Zotto email: dz@cs.unitn.it |
> | Via Marconi, 141 phone: ++39-0461534251 |
> | 38057 Pergine Valsugana (TN) www: http://www.cs.unitn.it/~dz/ |
> | Italy pgp: finger dz@tango.cs.unitn.it |
> +----------------------------------------------------------------------+
>
--
Bruce Momjian | http://www.op.net/~candle
maillist@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
Tom, any comment on this? > Massimo Dal Zotto <dz@cs.unitn.it> writes: > > Two small patches: > > 1) make default NBuffers = DEF_MAXBACKENDS*2 as required by check in > > PostmasterMain(). > > I had proposed moving NDBUFS into config.h and fixing the default a few > days ago, but then forgot to do it. As things stand, if you increase > DEF_MAXBACKENDS at configure time, you'll get a postmaster that won't > start unless you give it a -B setting larger than default. This is bad, > and I agree with Massimo that we ought to make sure the default NBuffers > is one that will work with the default MaxBackends. > > This patch is not quite right though, since it doesn't account for the > other part of PostmasterMain's condition (NBuffers >= 16). Will fix. > > > 2) check for QueryCancel in the copy command. Maybe we should do the > > same in vacuum command (Vadim?). > > I'm not too excited about adding QueryCancel support so soon before the > release, but the part of your patch that you didn't mention (diking out > the "file_opened" hack) is really a critical fix --- as the code stood > it would try to fclose() the same stdio file twice, which is disastrous > in most stdio libraries. I applied that part of it... good catch! > > regards, tom lane > > -- Bruce Momjian | http://www.op.net/~candle maillist@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, any comment on this?
I believe all those patches are applied long since in current sources
(Massimo might want to check though).
I even did something about QueryCancel in vacuum yesterday...
regards, tom lane
>> Massimo Dal Zotto <dz@cs.unitn.it> writes:
>>>> Two small patches:
>>>> 1) make default NBuffers = DEF_MAXBACKENDS*2 as required by check in
>>>> PostmasterMain().
>>
>> I had proposed moving NDBUFS into config.h and fixing the default a few
>> days ago, but then forgot to do it. As things stand, if you increase
>> DEF_MAXBACKENDS at configure time, you'll get a postmaster that won't
>> start unless you give it a -B setting larger than default. This is bad,
>> and I agree with Massimo that we ought to make sure the default NBuffers
>> is one that will work with the default MaxBackends.
>>
>> This patch is not quite right though, since it doesn't account for the
>> other part of PostmasterMain's condition (NBuffers >= 16). Will fix.
>>
>>>> 2) check for QueryCancel in the copy command. Maybe we should do the
>>>> same in vacuum command (Vadim?).
>>
>> I'm not too excited about adding QueryCancel support so soon before the
>> release, but the part of your patch that you didn't mention (diking out
>> the "file_opened" hack) is really a critical fix --- as the code stood
>> it would try to fclose() the same stdio file twice, which is disastrous
>> in most stdio libraries. I applied that part of it... good catch!