Re: cvs commit: httpd-2.0 STATUS
On Thursday, July 29, 2004, at 05:58 AM, André Malo wrote:
* "Mladen Turk" <[EMAIL PROTECTED]> wrote:
William A. Rowe, Jr. wrote:
/* Scoreboard file, if there is one */
#ifndef DEFAULT_SCOREBOARD
@@ -118,6 +119,7 @@
typedef struct {
int server_limit;
int thread_limit;
+int lb_limit;
ap_scoreboard_e sb_type;
ap_generation_t running_generation; /* the
generation of children which
* should still
be serving
requests. */
This definitely breaks binary compatibility.
Moving the lb_limit to the end of the struct will not break the binary
compatibility. Correct?
Not Correct. It *may* be the case. Depending on who allocates the
stuff.
Then the question to ask is whether any independent modules
(those that are not installed when the server is installed)
are likely to use that structure, and how they are expected
to use it.
I'd be surprised if it were even possible for an independent
module to allocate a scoreboard struct, but it has been a while
since I looked at that code.
Roy
Ideas for Smart Filtering
The filter architecture periodically gets discussed here, and I've been meaning to write up my thoughts for some time. I'm using a module that implements a slightly different filter API, primarily for filtering in a proxy context. I've now written a brief discussion document on the subject. It's mostly an abstraction of what I'm currently using, although it does propose some additional improvements, primarily with regard to protocol handling (reflecting the recent byteranges thread here). It generated an interesting discussion, including some interesting alternative ideas, last night on IRC. Perhaps it can lead to a general-purpose module for 2.0 and an architecture update for 2.2? http://www.apachetutor.org/dev/smart-filter -- Nick Kew
[PATCH] scoreboard WAS RE: cvs commit: httpd-2.0 STATUS
Hi all,
Since there was some concerns regarding binary compatibility, here is the
patch that uses different approach.
1. Revert the patch with changes to scoreboard structures
2. Add an extra hook that is run during ap_reopen_scoreboard with detached
param.
We will use our own shm, opening by hooking pre_mpm, and attaching in child
on reopen_scoreboard.
This will resolve any binary compatibility concerns.
Regards,
MT.
Index: scoreboard.h
===
RCS file: /home/cvspublic/httpd-2.0/include/scoreboard.h,v
retrieving revision 1.53
diff -u -r1.53 scoreboard.h
--- scoreboard.h29 Jul 2004 15:18:40 - 1.53
+++ scoreboard.h29 Jul 2004 18:28:21 -
@@ -124,7 +124,6 @@
* should still be serving requests.
*/
apr_time_t restart_time;
-int lb_limit;
} global_score;
/* stuff which the parent generally writes and the children rarely read */
@@ -138,13 +137,6 @@
*/
};
-/* stuff which is lb specific */
-typedef struct lb_score lb_score;
-struct lb_score{
-/* TODO: make a real stuct from this */
-unsigned char data[1024];
-};
-
/* Scoreboard is now in 'local' memory, since it isn't updated once created,
* even in forked architectures. Child created-processes (non-fork) will
* set up these indicies into the (possibly relocated) shmem records.
@@ -153,7 +145,6 @@
global_score *global;
process_score *parent;
worker_score **servers;
-lb_score **balancers;
} scoreboard;
typedef struct ap_sb_handle_t ap_sb_handle_t;
@@ -179,7 +170,6 @@
AP_DECLARE(worker_score *) ap_get_scoreboard_worker(int x, int y);
AP_DECLARE(process_score *) ap_get_scoreboard_process(int x);
AP_DECLARE(global_score *) ap_get_scoreboard_global(void);
-AP_DECLARE(lb_score *) ap_get_scoreboard_lb(int child_num, int lb_num);
AP_DECLARE_DATA extern scoreboard *ap_scoreboard_image;
AP_DECLARE_DATA extern const char *ap_scoreboard_fname;
@@ -198,11 +188,13 @@
AP_DECLARE_HOOK(int, pre_mpm, (apr_pool_t *p, ap_scoreboard_e sb_type))
/**
- * proxy load balancer
- * @return the number of load balancer workers.
+ * Hook for reopening scoreboard, reopen scoreboard.
+ * @param detached Non-zero if this is a seperate child process.
+ * @ingroup hooks
+ * @return OK or DECLINE on success; anything else is a error
*/
-APR_DECLARE_OPTIONAL_FN(int, ap_proxy_lb_workers,
-(void));
+AP_DECLARE_HOOK(int, reopen_scoreboard, (int detached))
+
/* for time_process_request() in http_main.c */
#define START_PREQUEST 1
Index: scoreboard.c
===
RCS file: /home/cvspublic/httpd-2.0/server/scoreboard.c,v
retrieving revision 1.75
diff -u -r1.75 scoreboard.c
--- scoreboard.c28 Jul 2004 22:50:54 - 1.75
+++ scoreboard.c29 Jul 2004 18:33:01 -
@@ -53,21 +53,23 @@
APR_HOOK_STRUCT(
APR_HOOK_LINK(pre_mpm)
+APR_HOOK_LINK(reopen_scoreboard)
)
AP_IMPLEMENT_HOOK_RUN_ALL(int,pre_mpm,
(apr_pool_t *p, ap_scoreboard_e sb_type),
(p, sb_type),OK,DECLINED)
-static APR_OPTIONAL_FN_TYPE(ap_proxy_lb_workers)
-*proxy_lb_workers;
+AP_IMPLEMENT_HOOK_RUN_ALL(int,reopen_scoreboard,
+ (int detached),
+ (detached),OK,DECLINED)
struct ap_sb_handle_t {
int child_num;
int thread_num;
};
-static int server_limit, thread_limit, lb_limit;
+static int server_limit, thread_limit;
static apr_size_t scoreboard_size;
/*
@@ -93,18 +95,9 @@
ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);
ap_mpm_query(AP_MPMQ_HARD_LIMIT_DAEMONS, &server_limit);
-if (!proxy_lb_workers)
-proxy_lb_workers = APR_RETRIEVE_OPTIONAL_FN(ap_proxy_lb_workers);
-if (proxy_lb_workers)
-lb_limit = proxy_lb_workers();
-else
-lb_limit = 0;
-
scoreboard_size = sizeof(global_score);
scoreboard_size += sizeof(process_score) * server_limit;
scoreboard_size += sizeof(worker_score) * server_limit * thread_limit;
-if (lb_limit)
-scoreboard_size += sizeof(lb_score) * server_limit * lb_limit;
return scoreboard_size;
}
@@ -116,8 +109,7 @@
ap_calc_scoreboard_size();
ap_scoreboard_image =
-calloc(1, sizeof(scoreboard) + server_limit * sizeof(worker_score *) +
- server_limit * lb_limit * sizeof(lb_score *));
+calloc(1, sizeof(scoreboard) + server_limit * sizeof(worker_score *));
more_storage = shared_score;
ap_scoreboard_image->global = (global_score *)more_storage;
more_storage += sizeof(global_score);
@@ -129,19 +121,9 @@
ap_scoreboard_image->servers[i] = (worker_score *)more_storage;
more_storage += thread_limit * sizeof(worker_score);
}
-if
RE: cvs commit: httpd-2.0 STATUS
William A. Rowe, Jr. wrote: > Although > we create the pointer-pointer logic in the children to avoid > this, it's still possible that the code would break some modules. > > I can't envision a case where any of the scoreboard entries > are allocated outside of our scoreboard.c code. > OK, then. I'll revert the patch and for 2.1 too, and we'll use our own shm for loadbalancer. Can we then create two extra hooks aka on_scoreboard_create and on_scoreboard_attach with ap_scoreboard_e, thread and server limit params. It will also help other modules like mod_ssl (currently using pool tag to supress double creation on post_config). MT. smime.p7s Description: S/MIME cryptographic signature
RE: cvs commit: httpd-2.0 STATUS
At 10:26 AM 7/29/2004, William A. Rowe, Jr. wrote: >If you changed worker_score, it would still have broken (or if you change >balancers in the future.) Agents reviewing the scoreboard are presuming >scoreboard_entry *psb can be accessed as psb[0..n] and that -will- be >broken with any size/alignment change to the struct. > >E.g. [...?] we have individual element accessors, but older style code presumes it can loop through the array of individual worker threads/procs. Adding another element to worker_score would break the starting offset of the members psb[1..n]. Although we create the pointer-pointer logic in the children to avoid this, it's still possible that the code would break some modules. I can't envision a case where any of the scoreboard entries are allocated outside of our scoreboard.c code. At 11:01 AM 7/29/2004, Mladen Turk wrote: > I was afraid you've gonna said that. Those 3rd party... :). > How about adding extra data to the end of the entire scoreboard withouth > touching any existing structs or data? > It will require an extra copy on child_init, but there will be no > compatibility issues, thought. You aren't changing worker_score so this isn't an issue. The patch already creates (new) a fourth lb_score array. Bill
RE: cvs commit: httpd-2.0 STATUS
William A. Rowe, Jr. wrote: > > > >Moving the lb_limit to the end of the struct will not break > the binary > >compatibility. Correct? > > Yes, in the case of global_score, that would be safer. It > seems that the additional lb_score 's element point was better placed. > > If you changed worker_score, it would still have broken (or > if you change balancers in the future.) Agents reviewing the > scoreboard are presuming scoreboard_entry *psb can be > accessed as psb[0..n] and that -will- be broken with any > size/alignment change to the struct. > I was afraid you've gonna said that. Those 3rd party... :). How about adding extra data to the end of the entire scoreboard withouth touching any existing structs or data? It will require an extra copy on child_init, but there will be no compatibility issues, thought. Will that rise any objections? MT. smime.p7s Description: S/MIME cryptographic signature
RE: cvs commit: httpd-2.0 STATUS
At 02:24 AM 7/29/2004, Mladen Turk wrote:
>
>William A. Rowe, Jr. wrote:
>> >
>> > /* Scoreboard file, if there is one */
>> > #ifndef DEFAULT_SCOREBOARD
>> > @@ -118,6 +119,7 @@
>> > typedef struct {
>> > int server_limit;
>> > int thread_limit;
>> > +int lb_limit;
>> > ap_scoreboard_e sb_type;
>> > ap_generation_t running_generation; /* the
>> generation of children which
>> >* should still
>> be serving
>> > requests. */
>>
>> This definitely breaks binary compatibility.
>
>Moving the lb_limit to the end of the struct will not break the binary
>compatibility. Correct?
Yes, in the case of global_score, that would be safer. It seems that
the additional lb_score 's element point was better placed.
If you changed worker_score, it would still have broken (or if you change
balancers in the future.) Agents reviewing the scoreboard are presuming
scoreboard_entry *psb can be accessed as psb[0..n] and that -will- be
broken with any size/alignment change to the struct.
E.g.
Bill
RE: cvs commit: httpd-2.0 STATUS
André Malo wrote: > > > William A. Rowe, Jr. wrote: > > > > > > This definitely breaks binary compatibility. > > > > > Moving the lb_limit to the end of the struct will not break > the binary > > compatibility. Correct? > > Not Correct. It *may* be the case. Depending on who allocates > the stuff. > Can you explain that a bit. Seems to me that there is a single ap_scoreboard_image->global, and it is allocated when the scoreboard gets created. IMO if the lb_limit is moved to the end of the struct it will not affect the existing binaries. Cannot speak on all builds but FreeBSD 4.1.10 and WIN32 are binary compatible (mod_status is the only that directly uses global). The only way where it might create problems is ap_scoreboard_image::servers not global, for some 3rd party module that directly uses this pointer array instead using ap_get_scoreboard_worker. Scoreboard file format is not binary compatible for sure, and this is the only part that might rise concerns, but only if that compatibility is required after all. MT. smime.p7s Description: S/MIME cryptographic signature
[PATCH] Bug #9037 reminder
Hi, I've received yet another mail from someone asking me about the patch for bug #9037 at http://nagoya.apache.org/bugzilla/show_bug.cgi?id=9037 The patch is attacched to the bug report. Also many distributions (e.g. Debian) ship apache with a patch for this issue. So far it seems to me that there is enough interest to have this problem fixed. I'm interested to bring the patch to an appliable state if it isn't ok as it is. Regards. Note: probably the severity (set to minor) of this problem ins't appropriate because when you hit it in a productive environment it causes a major pain. -- Federico Mennite
Hi
Important document! Norton AntiVirus Deleted1.txt Description: plain/text
Re: cvs commit: httpd-2.0 STATUS
* "Mladen Turk" <[EMAIL PROTECTED]> wrote:
> William A. Rowe, Jr. wrote:
> > >
> > > /* Scoreboard file, if there is one */
> > > #ifndef DEFAULT_SCOREBOARD
> > > @@ -118,6 +119,7 @@
> > > typedef struct {
> > > int server_limit;
> > > int thread_limit;
> > > +int lb_limit;
> > > ap_scoreboard_e sb_type;
> > > ap_generation_t running_generation; /* the
> > generation of children which
> > >* should still
> > be serving
> > > requests. */
> >
> > This definitely breaks binary compatibility.
> >
>
> Moving the lb_limit to the end of the struct will not break the binary
> compatibility. Correct?
Not Correct. It *may* be the case. Depending on who allocates the stuff.
nd
--
"Das Verhalten von Gates hatte mir bewiesen, dass ich auf ihn und seine
beiden Gefährten nicht zu zählen brauchte" -- Karl May, "Winnetou III"
Im Westen was neues:
RE: cvs commit: httpd-2.0 STATUS
William A. Rowe, Jr. wrote:
> >
> > /* Scoreboard file, if there is one */
> > #ifndef DEFAULT_SCOREBOARD
> > @@ -118,6 +119,7 @@
> > typedef struct {
> > int server_limit;
> > int thread_limit;
> > +int lb_limit;
> > ap_scoreboard_e sb_type;
> > ap_generation_t running_generation; /* the
> generation of children which
> >* should still
> be serving
> > requests. */
>
> This definitely breaks binary compatibility.
>
Moving the lb_limit to the end of the struct will not break the binary
compatibility. Correct?
MT.
smime.p7s
Description: S/MIME cryptographic signature
Re: cvs commit: httpd-2.0 STATUS
At 05:55 PM 7/28/2004, [EMAIL PROTECTED] wrote:
>minfrin 2004/07/28 15:55:15
>
> Modified:.Tag: APACHE_2_0_BRANCH STATUS
> Log:
> Propose a backport
>
> +*) Add load balancer support to the scoreboard in preparation for
> + load balancing support in mod_proxy.
> + include/scoreboard.h: 1.52
> + server/scoreboard.c: 1.75
> + +1: minfrin
Rather than vote... afraid I have to veto (for 2.0)...
> --- scoreboard.h 9 Feb 2004 20:38:21 - 1.51
> +++ scoreboard.h 28 Jul 2004 22:50:54 - 1.52
> @@ -32,6 +32,7 @@
> #include "apr_thread_proc.h"
> #include "apr_portable.h"
> #include "apr_shm.h"
> +#include "apr_optional.h"
>
> /* Scoreboard file, if there is one */
> #ifndef DEFAULT_SCOREBOARD
> @@ -118,6 +119,7 @@
> typedef struct {
> int server_limit;
> int thread_limit;
> +int lb_limit;
> ap_scoreboard_e sb_type;
> ap_generation_t running_generation; /* the generation of children which
>* should still be serving requests. */
This definitely breaks binary compatibility.
Bill
