pgsql/src/backend/postmaster (postmaster.c)

PGPORT environment variable is ignored again.
PostPortName seems to be reset in ResetAllOptions().

Regards.

Hiroshi Inoue
Inoue@tpf.co.jp

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:

PGPORT environment variable is ignored again.
PostPortName seems to be reset in ResetAllOptions().

Think you want to complain to Peter, not me. I haven't been touching
any options-handling code...

regards, tom lane

-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:

PGPORT environment variable is ignored again.
PostPortName seems to be reset in ResetAllOptions().

Think you want to complain to Peter, not me. I haven't been touching
any options-handling code...

Seems you have changed the position of ResetAllOptions() as follows.

@@ -378,15 +378,38 @@
*/
umask((mode_t) 0077);

-	ResetAllOptions();
-
 	MyProcPid = getpid();
+
+	/*
+	 * Fire up essential subsystems: error and memory management
+	 */
+	EnableExceptionHandling(true);
+	MemoryContextInit();
+
+	/*
+	 * By default, palloc() requests in the postmaster will be allocated
+	 * in the PostmasterContext, which is space that can be recycled by
+	 * backends.  Allocated data that needs to be available to backends
+	 * should be allocated in TopMemoryContext.
+	 */
+	PostmasterContext = AllocSetContextCreate(TopMemoryContext,
+											  "Postmaster",
+											  ALLOCSET_DEFAULT_MINSIZE,
+											  ALLOCSET_DEFAULT_INITSIZE,
+											  ALLOCSET_DEFAULT_MAXSIZE);
+	MemoryContextSwitchTo(PostmasterContext);
+
+	/*
+	 * Options setup
+	 */
 	if (getenv("PGDATA"))
 		DataDir = strdup(getenv("PGDATA")); /* default value */

if (getenv("PGPORT"))
PostPortName = atoi(getenv("PGPORT"));

+ ResetAllOptions();
+

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:

Seems you have changed the position of ResetAllOptions() as follows.

+	/*
+	 * Options setup
+	 */
if (getenv("PGDATA"))
DataDir = strdup(getenv("PGDATA")); /* default value */

if (getenv("PGPORT"))
PostPortName = atoi(getenv("PGPORT"));

+ ResetAllOptions();

Hmm, actually I was trying to move it to after the memory-context
creation code, but I guess your point is that ResetAllOptions() is
ignoring these env-var-based settings. Seems like the correct fix
is that guc.c ought to be handling this logic, not postmaster.c.
Peter, what do you think?

regards, tom lane

On Mon, 3 Jul 2000, Tom Lane wrote:

Hmm, actually I was trying to move it to after the memory-context
creation code, but I guess your point is that ResetAllOptions() is
ignoring these env-var-based settings. Seems like the correct fix
is that guc.c ought to be handling this logic, not postmaster.c.
Peter, what do you think?

I concur.

--
Peter Eisentraut Sernanders vaeg 10:115
peter_e@gmx.net 75262 Uppsala
http://yi.org/peter-e/ Sweden

eisentrp@csis.gvsu.edu writes:

... Seems like the correct fix
is that guc.c ought to be handling this logic, not postmaster.c.
Peter, what do you think?

I concur.

OK, do you want to fix it, or shall I?

BTW, it is now possible for guc.c to use palloc in TopMemoryContext,
if you want to do that. There's no real functional difference between
that and malloc --- except that MemoryContextStats() could include
the space used by guc.c --- but if it's just irritating you to use
malloc then you could change it.

regards, tom lane

Tom Lane writes:

OK, do you want to fix it, or shall I?

I will. I got some half-finished code lying around that touches that.

BTW, it is now possible for guc.c to use palloc in TopMemoryContext,
if you want to do that. There's no real functional difference between
that and malloc --- except that MemoryContextStats() could include
the space used by guc.c --- but if it's just irritating you to use
malloc then you could change it.

Well, I would really like a way to register an entire memory context to be
swept away by elog(ERROR). But that's probably too specialized. But yes, I
will look into that.

--
Peter Eisentraut Sernanders v�g 10:115
peter_e@gmx.net 75262 Uppsala
http://yi.org/peter-e/ Sweden

Peter Eisentraut <peter_e@gmx.net> writes:

Well, I would really like a way to register an entire memory context to be
swept away by elog(ERROR). But that's probably too specialized.

Be careful what you ask for, you might get it ;-).

If you really truly want a context that's destroyed by elog(ERROR),
and not by any other event, then create it as a child of ErrorContext,
and that's what will happen. But I suspect what you really want is a
context that's a child of TopTransactionContext and will go away at
either transaction commit or transaction abort.

A difficulty I have been noticing with some of these special-purpose
contexts is that you'd like to keep a pointer to them in some global
variable, but there's no easy way to determine whether that pointer is
currently valid (eg, whether the context has yet been created during
the current transaction). This would be particularly nasty with
something like a child of ErrorContext --- has an error occurred since
you last used the context?

An example of how we currently have to get around this is
backend/commands/trigger.c's DeferredTriggerAbortXact() routine, which
xact.c has to know to call at transaction abort. All it does is delete
trigger.c's private per-transaction context and reset the global pointer
to same. Deleting the context is useless because that'd happen
automatically anyway when TopTransactionContext gets zapped. So,
resetting the pointer is the only reason for this function to exist.

I've been toying with the notion of adding an optional "notify" callback
function to memory contexts. If specified, it'd be called immediately
before resetting or deleting the context. This'd give you a clean way
to reset pointers etc.

Comments anyone?

regards, tom lane

Tom Lane writes:

If you really truly want a context that's destroyed by elog(ERROR),
and not by any other event, then create it as a child of ErrorContext,
and that's what will happen. But I suspect what you really want is a
context that's a child of TopTransactionContext and will go away at
either transaction commit or transaction abort.

No, actually I wanted the former. Notice how the configuration file is
first read in completely as a linked list. When something goes wrong
(garbage in the file) I need to free the list (and other things) before
calling elog. The alternative would be to create a separate memory context
for this processing, but that's more of a purity issue, it wouldn't make
it work better, AFAICT.

A difficulty I have been noticing with some of these special-purpose
contexts is that you'd like to keep a pointer to them in some global
variable, but there's no easy way to determine whether that pointer is
currently valid

Shouldn't it be NULL if it's not valid?

--
Peter Eisentraut Sernanders v�g 10:115
peter_e@gmx.net 75262 Uppsala
http://yi.org/peter-e/ Sweden

Peter Eisentraut <peter_e@gmx.net> writes:

Tom Lane writes:

If you really truly want a context that's destroyed by elog(ERROR),
and not by any other event, then create it as a child of ErrorContext,
and that's what will happen. But I suspect what you really want is a
context that's a child of TopTransactionContext and will go away at
either transaction commit or transaction abort.

No, actually I wanted the former. Notice how the configuration file is
first read in completely as a linked list. When something goes wrong
(garbage in the file) I need to free the list (and other things) before
calling elog.

Ah, I see: normal exit from the routine would be to copy the validated
list to permanent storage and free the temporary junk, but in case of
error exit you'd like to have the list thrown away automatically.

As a general thing I'd still recommend a child of a transaction context,
because that guarantees the temp storage will be cleaned up either way;
you can't accidentally forget about it. However, for guc.c's purposes
a child of ErrorContext is probably better, because (I think) guc.c
needs to be able to run outside of any transaction. You'll still leak
memory if you forget to free the context during normal exit, but at
least this technique handles the error-case cleanup for you.

The alternative would be to create a separate memory context
for this processing, but that's more of a purity issue, it wouldn't make
it work better, AFAICT.

If you leak permanent storage upon error, that's a bug that needs to be
fixed, IMHO, not just a "purity" issue.

A difficulty I have been noticing with some of these special-purpose
contexts is that you'd like to keep a pointer to them in some global
variable, but there's no easy way to determine whether that pointer is
currently valid

Shouldn't it be NULL if it's not valid?

Exactly. The problem is how to make sure that that's true, if the
context gets discarded by general-purpose memory cleanup code that has
no idea you've got a pointer stashed away somewhere. You need a hook
that can clear your pointer to NULL at the right time. Basically this
mechanism could take the place of (some of the) hard-wired calls in
xact.c's abort processing, which would be a Good Thing --- consider
dynamically loaded code that has no prospect of being directly known
by xact.c.

regards, tom lane