You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Ames <gr...@remulak.net> on 2001/12/20 21:22:33 UTC

related config directives

...are more painful to deal with than you might think, if the user is
allowed to code them in any order.  

I'd like the comparisons between MaxClients and ServerLimit and the
equivalent thread directives to be coding order insensitive.  So I
created a post-config hook to do the comparing & adjusting.  The code
was very simple, but the log messages went to the error log file twice,
rather than to the console once which is what people are accustomed to
for config errors.

worker already has the same situation with MaxClients and
ThreadsPerChild.  It has a clever but kind of scary solution:  a
pre-config hook which swaps nodes in the config tree if it doesn't like
the order.  Then the comparisons, adjusting, and logging are done when
the second directive is parsed.  

While I think it's a very cool hack, mucking with the innards of the
config tree bothers me some.  config.c could provide an API to do order
checking and swapping, but I can't see this as a general solution. 
Think about cases where one directive is in container X and the other
isn't - yuck!

What if the very first ap_run_post_config hook was moved to before the
logs files were opened?  Then we could have simple logic to compare or
otherwise process multiple directives.  Any error messages would go
directly to the console.  If you wanted them to also go to the error log
file, that would just happen during the second post-config call;  if you
don't want that, it's easy to avoid.

Thoughts?

Greg

Re: related config directives

Posted by Jeff Trawick <tr...@attglobal.net>.
Ryan Bloom <rb...@covalent.net> writes:

> > I'm inclined to agree with Ryan. Let's just make a function to do
> > the swapping.

I had the same opinion...  a swap function owned by the config tree code.

> I meant to mention that if you really want to fix this problem, we should move to
> an XML based config language.

First awk, now XML.  !*&^%$%$## 

(just kidding; XML and related tools are worth being comfortable with)

-- 
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...

Re: related config directives

Posted by Aaron Bannert <aa...@clove.org>.
On Thu, Dec 20, 2001 at 01:15:25PM -0800, Ryan Bloom wrote:

> > I'm inclined to agree with Ryan. Let's just make a function to do the swapping.
> 
> I meant to mention that if you really want to fix this problem, we should move to
> an XML based config language.

If we want to do it now, we need to move post_config before open_logs,
(or create another hook that happens before open_logs). The better
long-term solution, I agree, is to do this kind of checking in XML.
At least I'm told that XML can do this kind of self checking at not
only the syntatical level but also at the semantical level.

-aaron

Re: related config directives

Posted by Ryan Bloom <rb...@covalent.net>.
On Thursday 20 December 2001 01:01 pm, Bill Stoddard wrote:
> > On Thursday 20 December 2001 12:22 pm, Greg Ames wrote:
> > > ...are more painful to deal with than you might think, if the user is
> > > allowed to code them in any order.  
> > > 
> > > I'd like the comparisons between MaxClients and ServerLimit and the
> > > equivalent thread directives to be coding order insensitive.  So I
> > > created a post-config hook to do the comparing & adjusting.  The code
> > > was very simple, but the log messages went to the error log file twice,
> > > rather than to the console once which is what people are accustomed to
> > > for config errors.
> > > 
> > > worker already has the same situation with MaxClients and
> > > ThreadsPerChild.  It has a clever but kind of scary solution:  a
> > > pre-config hook which swaps nodes in the config tree if it doesn't like
> > > the order.  Then the comparisons, adjusting, and logging are done when
> > > the second directive is parsed.  
> > >
> > > While I think it's a very cool hack, mucking with the innards of the
> > > config tree bothers me some.  config.c could provide an API to do order
> > > checking and swapping, but I can't see this as a general solution. 
> > > Think about cases where one directive is in container X and the other
> > > isn't - yuck!
> > 
> > This is the point of the config tree, it allows you to modify the config before
> > we actually run the configuration, so that you can find these dependancies
> > and fix them quickly.  Putting the logic in the directive handlers doesn't
> > really work, because you still have an order dependancy.
> > 
> > Think of it this way, these are good configs:
> > 
> > MaxClients 500
> > ServerLimit 525
> > 
> > ServerLimit 20
> > MaxClients 5
> > 
> > This is a bad config:
> > 
> > MaxClients 525
> > ServerLimit 500
> > 
> > Where do you catch it though?  In ServerLimit, then you
> > will also catch the second good config above, which is a good config
> > If you try to catch it in MaxClients, you will catch the first config above,
> > which is a good config.
> > 
> > The only solution is to choose default values that are invalid, so that you
> > can make sure that you always check in the last directive handler, but that
> > is incredibly ugly.  We can easily create a function to do the swapping,
> > which is more generic, but the swapping is the only 100% correct way
> > to do this.
> > 
> > Ryan
> 
> I'm inclined to agree with Ryan. Let's just make a function to do the swapping.

I meant to mention that if you really want to fix this problem, we should move to
an XML based config language.

Ryan

______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: related config directives

Posted by Bill Stoddard <bi...@wstoddard.com>.
> On Thursday 20 December 2001 12:22 pm, Greg Ames wrote:
> > ...are more painful to deal with than you might think, if the user is
> > allowed to code them in any order.  
> > 
> > I'd like the comparisons between MaxClients and ServerLimit and the
> > equivalent thread directives to be coding order insensitive.  So I
> > created a post-config hook to do the comparing & adjusting.  The code
> > was very simple, but the log messages went to the error log file twice,
> > rather than to the console once which is what people are accustomed to
> > for config errors.
> > 
> > worker already has the same situation with MaxClients and
> > ThreadsPerChild.  It has a clever but kind of scary solution:  a
> > pre-config hook which swaps nodes in the config tree if it doesn't like
> > the order.  Then the comparisons, adjusting, and logging are done when
> > the second directive is parsed.  
> >
> > While I think it's a very cool hack, mucking with the innards of the
> > config tree bothers me some.  config.c could provide an API to do order
> > checking and swapping, but I can't see this as a general solution. 
> > Think about cases where one directive is in container X and the other
> > isn't - yuck!
> 
> This is the point of the config tree, it allows you to modify the config before
> we actually run the configuration, so that you can find these dependancies
> and fix them quickly.  Putting the logic in the directive handlers doesn't
> really work, because you still have an order dependancy.
> 
> Think of it this way, these are good configs:
> 
> MaxClients 500
> ServerLimit 525
> 
> ServerLimit 20
> MaxClients 5
> 
> This is a bad config:
> 
> MaxClients 525
> ServerLimit 500
> 
> Where do you catch it though?  In ServerLimit, then you
> will also catch the second good config above, which is a good config
> If you try to catch it in MaxClients, you will catch the first config above,
> which is a good config.
> 
> The only solution is to choose default values that are invalid, so that you
> can make sure that you always check in the last directive handler, but that
> is incredibly ugly.  We can easily create a function to do the swapping,
> which is more generic, but the swapping is the only 100% correct way
> to do this.
> 
> Ryan

I'm inclined to agree with Ryan. Let's just make a function to do the swapping.

Bill


Re: related config directives

Posted by Ryan Bloom <rb...@covalent.net>.
On Thursday 20 December 2001 12:22 pm, Greg Ames wrote:
> ...are more painful to deal with than you might think, if the user is
> allowed to code them in any order.  
> 
> I'd like the comparisons between MaxClients and ServerLimit and the
> equivalent thread directives to be coding order insensitive.  So I
> created a post-config hook to do the comparing & adjusting.  The code
> was very simple, but the log messages went to the error log file twice,
> rather than to the console once which is what people are accustomed to
> for config errors.
> 
> worker already has the same situation with MaxClients and
> ThreadsPerChild.  It has a clever but kind of scary solution:  a
> pre-config hook which swaps nodes in the config tree if it doesn't like
> the order.  Then the comparisons, adjusting, and logging are done when
> the second directive is parsed.  
>
> While I think it's a very cool hack, mucking with the innards of the
> config tree bothers me some.  config.c could provide an API to do order
> checking and swapping, but I can't see this as a general solution. 
> Think about cases where one directive is in container X and the other
> isn't - yuck!

This is the point of the config tree, it allows you to modify the config before
we actually run the configuration, so that you can find these dependancies
and fix them quickly.  Putting the logic in the directive handlers doesn't
really work, because you still have an order dependancy.

Think of it this way, these are good configs:

	MaxClients 500
	ServerLimit 525

	ServerLimit 20
	MaxClients 5

This is a bad config:

	MaxClients 525
	ServerLimit 500

Where do you catch it though?  In ServerLimit, then you
will also catch the second good config above, which is a good config
If you try to catch it in MaxClients, you will catch the first config above,
which is a good config.

The only solution is to choose default values that are invalid, so that you
can make sure that you always check in the last directive handler, but that
is incredibly ugly.  We can easily create a function to do the swapping,
which is more generic, but the swapping is the only 100% correct way
to do this.

Ryan

______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: related config directives

Posted by Aaron Bannert <aa...@clove.org>.
On Thu, Dec 20, 2001 at 03:22:33PM -0500, Greg Ames wrote:
> ...are more painful to deal with than you might think, if the user is
> allowed to code them in any order.  
> 
> I'd like the comparisons between MaxClients and ServerLimit and the
> equivalent thread directives to be coding order insensitive.  So I
> created a post-config hook to do the comparing & adjusting.  The code
> was very simple, but the log messages went to the error log file twice,
> rather than to the console once which is what people are accustomed to
> for config errors.
> 
> worker already has the same situation with MaxClients and
> ThreadsPerChild.  It has a clever but kind of scary solution:  a
> pre-config hook which swaps nodes in the config tree if it doesn't like
> the order.  Then the comparisons, adjusting, and logging are done when
> the second directive is parsed.  
> 
> While I think it's a very cool hack, mucking with the innards of the
> config tree bothers me some.  config.c could provide an API to do order
> checking and swapping, but I can't see this as a general solution. 
> Think about cases where one directive is in container X and the other
> isn't - yuck!

I agree (and I'm the one who wrote it).

> What if the very first ap_run_post_config hook was moved to before the
> logs files were opened?  Then we could have simple logic to compare or
> otherwise process multiple directives.  Any error messages would go
> directly to the console.  If you wanted them to also go to the error log
> file, that would just happen during the second post-config call;  if you
> don't want that, it's easy to avoid.

I was thinking the same thing. My only concern was that something has
already been written that assumes post_config already has the log opened
and stderr redirected. Unless that is a problem or someone else can
come up with another reason not to do this, I'm +1.

Another possibility that would avoid the whole issue altogether would
be to simply add another hook before open_logs. I started doing this
yesterday on the plane:


Index: server/main.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/main.c,v
retrieving revision 1.111
diff -u -u -r1.111 main.c
--- server/main.c	2001/12/18 20:26:15	1.111
+++ server/main.c	2001/12/20 20:31:52
@@ -412,6 +412,10 @@
 	destroy_and_exit_process(process, 0);
     }
     apr_pool_clear(plog);
+    if ( ap_run_validate_config(pconf, plog, ptemp, server_conf) != OK) {
+        ap_log_error(APLOG_MARK, APLOG_STARTUP |APLOG_ERR| APLOG_NOERRNO, 0, NULL, "Unable to validate the config\n");
+        destroy_and_exit_process(process, 1);
+    }
     if ( ap_run_open_logs(pconf, plog, ptemp, server_conf) != OK) {
         ap_log_error(APLOG_MARK, APLOG_STARTUP |APLOG_ERR| APLOG_NOERRNO, 0, NULL, "Unable to open logs\n");
         destroy_and_exit_process(process, 1);
Index: include/http_config.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_config.h,v
retrieving revision 1.92
diff -u -u -r1.92 http_config.h
--- include/http_config.h	2001/11/24 00:08:29	1.92
+++ include/http_config.h	2001/12/20 20:31:55
@@ -974,6 +974,18 @@
 AP_DECLARE_HOOK(void,pre_config,(apr_pool_t *pconf,apr_pool_t *plog,apr_pool_t *ptemp))
 
 /**
+ * Run the validate_config function for each module. This function should
+ * validate the config parameters accepted by the module, and return
+ * OK or non-OK to signify success.
+ * @param pconf The config pool
+ * @param plog The logging streams pool
+ * @param ptemp The temporary pool
+ * @param s The list of server_recs
+ * @return OK or DECLINED on success anything else is a error
+ */
+AP_DECLARE_HOOK(int,validate_config,(apr_pool_t *pconf,apr_pool_t *plog,apr_pool_t *ptemp,server_rec *s))
+
+/**
  * Run the post_config function for each module
  * @param pconf The config pool
  * @param plog The logging streams pool


-aaron

Re: related config directives

Posted by Aaron Bannert <aa...@clove.org>.
On Mon, Dec 24, 2001 at 09:40:03AM -0800, Ryan Bloom wrote:
> I thought of another reason to do this in the tree itself.  The point of the config
> tree is to keep the current config in memory.  If you are going to modify the
> config, you need to modify it in the tree.  By doing to work on the tree, you
> get that for free.

I'm confused (it may just be the eggnog :), but aren't we talking about
two different things? There are fatal and non-fatal errors in the config.
We want to give a meaningful message when we discover a fatal error,
but we just want to issue a warning and try to adjust something sane
for non-fatal errors. I can see how fidgeting with absurd settings by
rearranging the order in the tree will make it easier to deal with some
of the non-fatal errors, but doesn't it seem easiest to deal with the
fatal errors all at once after we've got a stable config tree with all
the defaults in place in the tree?

-aaron

Re: related config directives

Posted by Ryan Bloom <rb...@covalent.net>.
On Sunday 23 December 2001 09:38 am, Ryan Bloom wrote:
> On Sunday 23 December 2001 06:27 am, Jeff Trawick wrote:
> > Greg Ames <gr...@remulak.net> writes:
> > 
> > > ...are more painful to deal with than you might think, if the user is
> > > allowed to code them in any order.  
> > 
> > another thing is that the user can code any number of them, possibly
> > with different values...  swapping items in the tree doesn't
> > necessarily deal with that properly while doing stuff in a post-config
> > hook wouldn't run into problems (it could just use the setting from
> > the last time the directive occurred)
> 
> Yeah, but that can be handled by searching from the end of the list instead
> of the front.  The problem with doing it in post-config is that it isn't generic 
> enough.  It will work for any directive that is only allowed in the main server
> config, but you can't really use it for anything in a different scope.  Since
> we already know that we need this general ability for some of the auth
> directives (see John Sterling's posts about some of the problems with AuthType
> and AuthName), I would much rather have a generic function that will 
> manipulate the tree itself.

I thought of another reason to do this in the tree itself.  The point of the config
tree is to keep the current config in memory.  If you are going to modify the
config, you need to modify it in the tree.  By doing to work on the tree, you
get that for free.

Ryan

______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: related config directives

Posted by Ryan Bloom <rb...@covalent.net>.
On Wednesday 26 December 2001 08:10 am, Greg Ames wrote:
> Ryan Bloom wrote:
> > 
> >       Since
> > we already know that we need this general ability for some of the auth
> > directives (see John Sterling's posts about some of the problems with AuthType
> > and AuthName), I would much rather have a generic function that will
> > manipulate the tree itself.
> 
> Swapping things in the config tree breaks if the nodes involved are in
> different scopes:
> 
> AuthType basic
> 
> <Directory /docroot/flintstone >
> AuthName Fred
> </Directory >
> 
> <Directory /docroot/rubble >
> AuthName Barney 
> </Directory >
> 
> I think that's a legitimate config.  But if we swapped AuthName &
> AuthType, we would destroy it.

That shouldn't be a legitimate config IMHO.  However, I would suggest that
the swap function should only be used to swap things on the same level of
the config tree.

Ryan

______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: related config directives

Posted by Greg Ames <gr...@remulak.net>.
Ryan Bloom wrote:
> 
> On Sunday 23 December 2001 06:27 am, Jeff Trawick wrote:
> > Greg Ames <gr...@remulak.net> writes:
> >
> > > ...are more painful to deal with than you might think, if the user is
> > > allowed to code them in any order.
> >
> > another thing is that the user can code any number of them, possibly
> > with different values...  swapping items in the tree doesn't
> > necessarily deal with that properly while doing stuff in a post-config
> > hook wouldn't run into problems (it could just use the setting from
> > the last time the directive occurred)
> 
>             The problem with doing it in post-config is that it isn't generic
> enough.  It will work for any directive that is only allowed in the main server
> config, but you can't really use it for anything in a different scope.  

You're right.  Good point.

>       Since
> we already know that we need this general ability for some of the auth
> directives (see John Sterling's posts about some of the problems with AuthType
> and AuthName), I would much rather have a generic function that will
> manipulate the tree itself.

Swapping things in the config tree breaks if the nodes involved are in
different scopes:

AuthType basic

<Directory /docroot/flintstone >
AuthName Fred
</Directory >

<Directory /docroot/rubble >
AuthName Barney 
</Directory >

I think that's a legitimate config.  But if we swapped AuthName &
AuthType, we would destroy it.

Greg

Re: related config directives

Posted by Ryan Bloom <rb...@covalent.net>.
On Sunday 23 December 2001 06:27 am, Jeff Trawick wrote:
> Greg Ames <gr...@remulak.net> writes:
> 
> > ...are more painful to deal with than you might think, if the user is
> > allowed to code them in any order.  
> 
> another thing is that the user can code any number of them, possibly
> with different values...  swapping items in the tree doesn't
> necessarily deal with that properly while doing stuff in a post-config
> hook wouldn't run into problems (it could just use the setting from
> the last time the directive occurred)

Yeah, but that can be handled by searching from the end of the list instead
of the front.  The problem with doing it in post-config is that it isn't generic 
enough.  It will work for any directive that is only allowed in the main server
config, but you can't really use it for anything in a different scope.  Since
we already know that we need this general ability for some of the auth
directives (see John Sterling's posts about some of the problems with AuthType
and AuthName), I would much rather have a generic function that will 
manipulate the tree itself.

Ryan
______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: related config directives

Posted by Jeff Trawick <tr...@attglobal.net>.
Greg Ames <gr...@remulak.net> writes:

> ...are more painful to deal with than you might think, if the user is
> allowed to code them in any order.  

another thing is that the user can code any number of them, possibly
with different values...  swapping items in the tree doesn't
necessarily deal with that properly while doing stuff in a post-config
hook wouldn't run into problems (it could just use the setting from
the last time the directive occurred)

-- 
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...