Thread: [WIP] GUC for temp_tablespaces
On 3/17/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Jaime Casanova" <systemguards@gmail.com> writes:
> > On 3/5/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> In the second place, it's a serious violation of what little modularity
> >> and layering we have for fd.c to be calling into commands/tablespace.c.
> >> This is not merely cosmetic but has real consequences: one being that
> >> it's now unsafe to call OpenTemporaryFile outside a transaction.
>
> > ok, you are right... what do you suggest?
> > maybe move the GetTempTablespace function to somewhere in src/backend/utils?
>
> You missed the point entirely. Relocating the code to some other file
> wouldn't change the objection: the problem is that fd.c mustn't invoke
> any transactional facilities such as catalog lookups. It's too low
> level for that.
>
> You could perhaps do it the other way around: some transactional
> code (eg the assign hook for a GUC variable) tells fd.c to save
> some private state controlling future temp file creations.
>
ok. i have done that.
I know this is not the time i told you but i was busy at job.
i haven't did anything about RemovePgTempFiles() yet, because i want
to know of the posibility of getting this on 8.3
--
regards,
Jaime Casanova
"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook
Attachment
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Jaime Casanova wrote:
> On 3/17/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > "Jaime Casanova" <systemguards@gmail.com> writes:
> > > On 3/5/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >> In the second place, it's a serious violation of what little modularity
> > >> and layering we have for fd.c to be calling into commands/tablespace.c.
> > >> This is not merely cosmetic but has real consequences: one being that
> > >> it's now unsafe to call OpenTemporaryFile outside a transaction.
> >
> > > ok, you are right... what do you suggest?
> > > maybe move the GetTempTablespace function to somewhere in src/backend/utils?
> >
> > You missed the point entirely. Relocating the code to some other file
> > wouldn't change the objection: the problem is that fd.c mustn't invoke
> > any transactional facilities such as catalog lookups. It's too low
> > level for that.
> >
> > You could perhaps do it the other way around: some transactional
> > code (eg the assign hook for a GUC variable) tells fd.c to save
> > some private state controlling future temp file creations.
> >
>
> ok. i have done that.
> I know this is not the time i told you but i was busy at job.
>
> i haven't did anything about RemovePgTempFiles() yet, because i want
> to know of the posibility of getting this on 8.3
>
> --
> regards,
> Jaime Casanova
>
> "Programming today is a race between software engineers striving to
> build bigger and better idiot-proof programs and the universe trying
> to produce bigger and better idiots.
> So far, the universe is winning."
> Richard Cook
[ Attachment, skipping... ]
>
> ---------------------------(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
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
On 5/3/07, Bruce Momjian <bruce@momjian.us> wrote:
>
> Your patch has been added to the PostgreSQL unapplied patches list at:
>
This is an updated version of the patch.
Tom objections:
- fd.c is too low level for calling code from commands/tablespace.c.
This was fixed adding a second parameter to BufFileCreateTemp() to send
the tblspcOid (this function is called from executor/nodeHashJoin.c,
utils/sort/logtape.c and utils/sort/tuplestore.c). Are these places ok?
- RemovePgTempFilesInDir() has no support for removing temp files from
strange locations.
Per Tom suggestion temp files are now created in: base/pgsql_tmp and
pg_tblspc/$oid_tblspc/pgsql_tmp. So i just refactor RemovePgTempFiles()
to call RemovePgTempFilesInDir() with base and pg_tblspc/$oid_tblspc's
pgsql_tmp
Other changes in code:
fd.c:
functions make_database_relative() and FileNameOpenFile() were marked
as NOT_USED. objections to simply delete them?
also added OpenTempFileInTblspc() to create the tempfilepath and call
to PathNameOpenFile()
buffile.c:
also added a new tblspcOid field to BufFile struct to use it in extendBufFile()
Problems:
While the patch passes all the regression tests i still have a problem
when doin this:
sgerp=# set temp_tablespaces = '';
ERROR: tablespace "" does not exist
note that setting temp_tablespaces = '' from postgresql.conf works well.
maybe this is silly but it's too late for me... i will keep trying
tomorrow unless someone else has fixed it.
comments?
--
regards,
Jaime Casanova
"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook
Attachment
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Jaime Casanova wrote:
> On 5/3/07, Bruce Momjian <bruce@momjian.us> wrote:
> >
> > Your patch has been added to the PostgreSQL unapplied patches list at:
> >
>
> This is an updated version of the patch.
>
> Tom objections:
> - fd.c is too low level for calling code from commands/tablespace.c.
> This was fixed adding a second parameter to BufFileCreateTemp() to send
> the tblspcOid (this function is called from executor/nodeHashJoin.c,
> utils/sort/logtape.c and utils/sort/tuplestore.c). Are these places ok?
>
> - RemovePgTempFilesInDir() has no support for removing temp files from
> strange locations.
> Per Tom suggestion temp files are now created in: base/pgsql_tmp and
> pg_tblspc/$oid_tblspc/pgsql_tmp. So i just refactor RemovePgTempFiles()
> to call RemovePgTempFilesInDir() with base and pg_tblspc/$oid_tblspc's
> pgsql_tmp
>
> Other changes in code:
> fd.c:
> functions make_database_relative() and FileNameOpenFile() were marked
> as NOT_USED. objections to simply delete them?
> also added OpenTempFileInTblspc() to create the tempfilepath and call
> to PathNameOpenFile()
> buffile.c:
> also added a new tblspcOid field to BufFile struct to use it in extendBufFile()
>
>
> Problems:
> While the patch passes all the regression tests i still have a problem
> when doin this:
>
> sgerp=# set temp_tablespaces = '';
> ERROR: tablespace "" does not exist
>
> note that setting temp_tablespaces = '' from postgresql.conf works well.
>
> maybe this is silly but it's too late for me... i will keep trying
> tomorrow unless someone else has fixed it.
>
> comments?
>
> --
> regards,
> Jaime Casanova
>
> "Programming today is a race between software engineers striving to
> build bigger and better idiot-proof programs and the universe trying
> to produce bigger and better idiots.
> So far, the universe is winning."
> Richard Cook
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
On 5/5/07, Bruce Momjian <bruce@momjian.us> wrote:
>
> Your patch has been added to the PostgreSQL unapplied patches list at:
This is final version of the patch (i hope), at least it fixes the
problem i had yesterday.
--
regards,
Jaime Casanova
"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook
Attachment
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Jaime Casanova wrote:
> On 5/5/07, Bruce Momjian <bruce@momjian.us> wrote:
> >
> > Your patch has been added to the PostgreSQL unapplied patches list at:
>
> This is final version of the patch (i hope), at least it fixes the
> problem i had yesterday.
>
> --
> regards,
> Jaime Casanova
>
> "Programming today is a race between software engineers striving to
> build bigger and better idiot-proof programs and the universe trying
> to produce bigger and better idiots.
> So far, the universe is winning."
> Richard Cook
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Am Samstag, 5. Mai 2007 16:40 schrieb Jaime Casanova: > On 5/5/07, Bruce Momjian <bruce@momjian.us> wrote: > > Your patch has been added to the PostgreSQL unapplied patches list at: > > This is final version of the patch (i hope), at least it fixes the > problem i had yesterday. What I have been missing all along in these patches is an explanation for what it means to list multiple temporary tablespaces. Are they used in order, or the first one that exists, or what? -- Peter Eisentraut http://developer.postgresql.org/~petere/
On 5/8/07, Peter Eisentraut <peter_e@gmx.net> wrote: > Am Samstag, 5. Mai 2007 16:40 schrieb Jaime Casanova: > > On 5/5/07, Bruce Momjian <bruce@momjian.us> wrote: > > > Your patch has been added to the PostgreSQL unapplied patches list at: > > > > This is final version of the patch (i hope), at least it fixes the > > problem i had yesterday. > > What I have been missing all along in these patches is an explanation for what > it means to list multiple temporary tablespaces. Are they used in order, or > the first one that exists, or what? > http://archives.postgresql.org/pgsql-hackers/2007-01/msg00531.php http://archives.postgresql.org/pgsql-patches/2007-01/msg00282.php in src/backend/commands/tablespace.c:assign_temp_tablespaces(): /* * Select the first tablespace to use */ Assert(num_temp_tablespaces >= 0); if (num_temp_tablespaces != 0) next_temp_tablespace = MyProcPid % num_temp_tablespaces; -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
Am Mittwoch, 9. Mai 2007 02:21 schrieb Jaime Casanova: > > What I have been missing all along in these patches is an explanation for > > what it means to list multiple temporary tablespaces. Are they used in > > order, or the first one that exists, or what? > > http://archives.postgresql.org/pgsql-hackers/2007-01/msg00531.php > http://archives.postgresql.org/pgsql-patches/2007-01/msg00282.php Those are discussions of possible ideas, not an acceptable documentation of this feature. > in src/backend/commands/tablespace.c:assign_temp_tablespaces(): That also isn't an acceptable place to put feature documentation. > /* > * Select the first tablespace to use > */ > Assert(num_temp_tablespaces >= 0); > if (num_temp_tablespaces != 0) > next_temp_tablespace = MyProcPid % num_temp_tablespaces; What does this mean? Is there code that selects the second tablespace to use? -- Peter Eisentraut http://developer.postgresql.org/~petere/
On 5/9/07, Peter Eisentraut <peter_e@gmx.net> wrote: > Am Mittwoch, 9. Mai 2007 02:21 schrieb Jaime Casanova: > > > What I have been missing all along in these patches is an explanation for > > > what it means to list multiple temporary tablespaces. Are they used in > > > order, or the first one that exists, or what? > > > > http://archives.postgresql.org/pgsql-hackers/2007-01/msg00531.php > > http://archives.postgresql.org/pgsql-patches/2007-01/msg00282.php > > Those are discussions of possible ideas, not an acceptable documentation of > this feature. > ahh... ok, obviously a misunderstood you... what you were asking for is user visible documentation, isn't it? what the patch does is to select the first tablespace from the list pseudo-randomicaly (MyProcPid % num_temp_tablespaces) and then cycle in order through the list every time we call GetTempTablespace(). Every backend will start (hopefully) in a different tablespace and will keep its own iterator for the list. A BufFile will use the same tablespace for every file it has. If we can't create the file in the selected tablespace we fall into $PGDATA/base/pgsql_tmp, now that i think on it we should be sending a warning that the file couldn't be created. About the docs, what about something along the lines, in config.sgml: "The first tablespace that will be used is choosen randomly from the list, starting from that with cycle through the list in order. -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook