You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2009/03/29 12:05:32 UTC
Re: svn commit: r759386 - in /httpd/httpd/trunk/modules/proxy: mod_serf.c
mod_serf.h
On 03/28/2009 12:10 AM, pquerna@apache.org wrote:
> Author: pquerna
> Date: Fri Mar 27 23:10:21 2009
> New Revision: 759386
>
> URL: http://svn.apache.org/viewvc?rev=759386&view=rev
> Log:
> Work in Progress.
>
> Add Clustered proxying support to mod_serf, by using the heartbeats system.
>
> No preconfiguration of cluster members is needed.
>
> Just a config like this:
> SerfCluster sweet heartbeat file=/var/cache/apache/hb.dat
> SerfCluster sour heartbeat file=/var/cache/apache/cluster2.dat
> <Location "/">
> SerfPass cluster://sweet
> </Location>
> <Location "/different_cluster">
> SerfPass cluster://sour
> </Location>
>
> The location of all possible destination servers is provided by a new
> providers interface, that includes configuration checking of the arguments to
> the SerfCluster command, solving one of the worst problems with the mod_proxy
> load balancer subsystem.
>
> Added:
> httpd/httpd/trunk/modules/proxy/mod_serf.h (with props)
> Modified:
> httpd/httpd/trunk/modules/proxy/mod_serf.c
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_serf.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_serf.c?rev=759386&r1=759385&r2=759386&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_serf.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_serf.c Fri Mar 27 23:10:21 2009
> @@ -408,32 +486,356 @@
> return NULL;
> }
>
> -static void *create_config(apr_pool_t *p, char *dummy)
> +/* SerfCluster <name> <provider> <key=value_params_to_provider> ... */
> +
> +static const char *add_cluster(cmd_parms *cmd, void *d,
> + int argc, char *const argv[])
> {
> - serf_config_rec *new = (serf_config_rec *) apr_pcalloc(p, sizeof(serf_config_rec));
> + const char *rv;
> + ap_serf_cluster_provider_t *backend;
> + int i;
> + serf_cluster_t *cluster = NULL;
> + serf_server_config_t *ctx =
> + (serf_server_config_t *)ap_get_module_config(cmd->server->module_config,
> + &serf_module);
> +
> + const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
> +
> + if (err != NULL) {
> + return err;
> + }
> +
> + if (argc < 2) {
> + return "SerfCluster must have at least a name and provider.";
> + }
> +
> + cluster = apr_palloc(cmd->pool, sizeof(serf_cluster_t));
> + cluster->name = apr_pstrdup(cmd->pool, argv[0]);
> + cluster->provider = apr_pstrdup(cmd->pool, argv[1]);
> + cluster->params = apr_table_make(cmd->pool, 6);
> +
> + backend = ap_lookup_provider(AP_SERF_CLUSTER_PROVIDER, cluster->provider, "0");
> +
> + if (backend == NULL) {
> + return apr_psprintf(cmd->pool, "SerfCluster: unable to find "
> + "provider '%s'", cluster->provider);
> + }
> +
> + for (i = 2; i < argc; i++) {
> + const char *p = argv[i];
> + const char *x = ap_strchr(p, '=');
> +
> + if (x && strlen(p) > 1) {
> + apr_table_addn(cluster->params,
> + apr_pstrndup(cmd->pool, p, x-p),
> + x+1);
> + }
> + else {
> + apr_table_addn(cluster->params,
> + apr_pstrdup(cmd->pool, p),
> + "");
> + }
> + }
> +
> + if (backend->check_config == NULL) {
> + return apr_psprintf(cmd->pool, "SerfCluster: Provider '%s' failed to "
> + "provider a configuration checker",
> + cluster->provider);
> + }
> +
> + rv = backend->check_config(backend->baton, cmd, cluster->params);
> +
> + if (rv) {
> + return rv;
> + }
> +
> + apr_hash_set(ctx->clusters, cluster->name, APR_HASH_KEY_STRING, cluster);
> +
> + return NULL;
> +}
> +
> +static void *create_dir_config(apr_pool_t *p, char *dummy)
> +{
> + serf_config_t *new = (serf_config_t *) apr_pcalloc(p, sizeof(serf_config_t));
> new->on = 0;
> return new;
> }
>
> +static void *create_server_config(apr_pool_t *p, server_rec *s)
> +{
> + serf_server_config_t *ctx =
> + (serf_server_config_t *) apr_pcalloc(p, sizeof(serf_server_config_t));
> +
> + ctx->clusters = apr_hash_make(p);
> +
> + return ctx;
> +}
> +
> +static void * merge_server_config(apr_pool_t *p, void *basev, void *overridesv)
> +{
> + serf_server_config_t *ctx = apr_pcalloc(p, sizeof(serf_server_config_t));
> + serf_server_config_t *base = (serf_server_config_t *) basev;
> + serf_server_config_t *overrides = (serf_server_config_t *) overridesv;
> +
> + ctx->clusters = apr_hash_overlay(p, base->clusters, overrides->clusters);
> + return ctx;
> +}
Why do we need merging when the directive is global only?
> +
> static const command_rec serf_cmds[] =
> {
> - AP_INIT_TAKE1("SerfPass", add_pass, NULL, OR_INDEXES/*making shit up*/,
> - "A prefix and destination"),
> + AP_INIT_TAKE_ARGV("SerfCluster", add_cluster, NULL, RSRC_CONF,
> + "Configure a cluster backend"),
> + AP_INIT_TAKE1("SerfPass", add_pass, NULL, OR_INDEXES,
> + "URL to reverse proxy to"),
> {NULL}
> };
>
> +typedef struct hb_table_baton_t {
> + apr_pool_t *p;
> + const char *msg;
> +} hb_table_baton_t;
> +
> +static int hb_table_check(void *rec, const char *key, const char *value)
> +{
> + hb_table_baton_t *b = (hb_table_baton_t*)rec;
> + if (strcmp(key, "path") != 0) {
> + b->msg = apr_psprintf(b->p,
> + "SerfCluster Heartbeat Invalid parameter '%s'",
> + key);
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static const char* hb_config_check(void *baton,
> + cmd_parms *cmd,
> + apr_table_t *params)
> +{
> + hb_table_baton_t b;
> +
> + if (apr_is_empty_table(params)) {
> + return "SerfCluster Heartbeat requires a path to the heartbat information.";
> + }
> +
> + b.p = cmd->pool;
> + b.msg = NULL;
> +
> + apr_table_do(hb_table_check, &b, params, NULL);
> +
> + if (b.msg) {
> + return b.msg;
> + }
> +
> + return NULL;
> +}
> +
> +typedef struct hb_server_t {
> + const char *ip;
> + int busy;
> + int ready;
> + int seen;
> +} hb_server_t;
> +
> +static void
> +argstr_to_table(apr_pool_t *p, char *str, apr_table_t *parms)
> +{
> + char *key;
> + char *value;
> + char *strtok_state;
> +
> + key = apr_strtok(str, "&", &strtok_state);
> + while (key) {
> + value = strchr(key, '=');
> + if (value) {
> + *value = '\0'; /* Split the string in two */
> + value++; /* Skip passed the = */
> + }
> + else {
> + value = "1";
> + }
> + ap_unescape_url(key);
> + ap_unescape_url(value);
> + apr_table_set(parms, key, value);
> + key = apr_strtok(NULL, "&", &strtok_state);
> + }
> +}
> +
> +static apr_status_t read_heartbeats(const char *path,
> + apr_array_header_t *servers,
> + apr_pool_t *pool)
> +{
> + apr_finfo_t fi;
> + apr_status_t rv;
> + apr_file_t *fp;
> +
> + if (!path) {
> + return APR_SUCCESS;
> + }
> +
> + rv = apr_file_open(&fp, path, APR_READ|APR_BINARY|APR_BUFFERED,
> + APR_OS_DEFAULT, pool);
> +
> + if (rv) {
> + return rv;
> + }
> +
> + rv = apr_file_info_get(&fi, APR_FINFO_SIZE, fp);
> +
> + if (rv) {
> + return rv;
> + }
> +
> + {
> + char *t;
> + int lineno = 0;
> + apr_table_t *hbt = apr_table_make(pool, 10);
> + char buf[4096];
> +
> + while (apr_file_gets(buf, sizeof(buf), fp) == APR_SUCCESS) {
> + hb_server_t *server;
> + const char *ip;
> + lineno++;
> +
> + /* comment */
> + if (buf[0] == '#') {
> + continue;
> + }
> +
> +
> + /* line format: <IP> <query_string>\n */
> + t = strchr(buf, ' ');
> + if (!t) {
> + continue;
> + }
> +
> + ip = apr_pstrndup(pool, buf, t - buf);
> + t++;
> + server = apr_pcalloc(pool, sizeof(hb_server_t));
> + server->ip = ip;
> + server->seen = -1;
> + apr_table_clear(hbt);
> +
> + argstr_to_table(pool, apr_pstrdup(pool, t), hbt);
> +
> + if (apr_table_get(hbt, "busy")) {
> + server->busy = atoi(apr_table_get(hbt, "busy"));
> + }
> +
> + if (apr_table_get(hbt, "ready")) {
> + server->ready = atoi(apr_table_get(hbt, "ready"));
> + }
> +
> + if (apr_table_get(hbt, "lastseen")) {
> + server->seen = atoi(apr_table_get(hbt, "lastseen"));
> + }
> +
> + if (server->busy == 0 && server->ready != 0) {
> + /* Server has zero threads active, but lots of them ready,
> + * it likely just started up, so lets /4 the number ready,
> + * to prevent us from completely flooding it with all new
> + * requests.
> + */
> + server->ready = server->ready / 4;
Nitpick: server->ready = server->ready >> 2 seems to be better.
> + }
> +
> + APR_ARRAY_PUSH(servers, hb_server_t *) = server;
> + }
> + }
> +
> + return APR_SUCCESS;
> +}
> +
> +static int hb_server_sort(const void *a_, const void *b_)
> +{
> + hb_server_t *a = (hb_server_t*)a;
> + hb_server_t *b = (hb_server_t*)b;
Guess that should be a_ and b_ above.
> + if (a->ready == b->ready) {
> + return 0;
> + }
> + else if (a->ready > b->ready) {
> + return -1;
> + }
> + else {
> + return 1;
> + }
> +}
Regards
Rüdiger
Re: svn commit: r759386 - in /httpd/httpd/trunk/modules/proxy:
mod_serf.c mod_serf.h
Posted by Paul Querna <pa...@querna.org>.
On Sun, Mar 29, 2009 at 11:05 AM, Ruediger Pluem <rp...@apache.org> wrote:
> On 03/28/2009 12:10 AM, pquerna@apache.org wrote:
>> Author: pquerna
>> Date: Fri Mar 27 23:10:21 2009
>> New Revision: 759386
>>
>> URL: http://svn.apache.org/viewvc?rev=759386&view=rev
....
>> +static void * merge_server_config(apr_pool_t *p, void *basev, void *overridesv)
>> +{
>> + serf_server_config_t *ctx = apr_pcalloc(p, sizeof(serf_server_config_t));
>> + serf_server_config_t *base = (serf_server_config_t *) basev;
>> + serf_server_config_t *overrides = (serf_server_config_t *) overridesv;
>> +
>> + ctx->clusters = apr_hash_overlay(p, base->clusters, overrides->clusters);
>> + return ctx;
>> +}
>
>
> Why do we need merging when the directive is global only?
I intend to change it to not be global only.... There is no reason
SerfCluster can't be server/vhost scoped.
>> +
>> static const command_rec serf_cmds[] =
>> {
>> - AP_INIT_TAKE1("SerfPass", add_pass, NULL, OR_INDEXES/*making shit up*/,
>> - "A prefix and destination"),
>> + AP_INIT_TAKE_ARGV("SerfCluster", add_cluster, NULL, RSRC_CONF,
>> + "Configure a cluster backend"),
>> + AP_INIT_TAKE1("SerfPass", add_pass, NULL, OR_INDEXES,
>> + "URL to reverse proxy to"),
>> {NULL}
>> };
>>
>> +typedef struct hb_table_baton_t {
>> + apr_pool_t *p;
>> + const char *msg;
>> +} hb_table_baton_t;
>> +
>> +static int hb_table_check(void *rec, const char *key, const char *value)
>> +{
>> + hb_table_baton_t *b = (hb_table_baton_t*)rec;
>> + if (strcmp(key, "path") != 0) {
>> + b->msg = apr_psprintf(b->p,
>> + "SerfCluster Heartbeat Invalid parameter '%s'",
>> + key);
>> + return 1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const char* hb_config_check(void *baton,
>> + cmd_parms *cmd,
>> + apr_table_t *params)
>> +{
>> + hb_table_baton_t b;
>> +
>> + if (apr_is_empty_table(params)) {
>> + return "SerfCluster Heartbeat requires a path to the heartbat information.";
>> + }
>> +
>> + b.p = cmd->pool;
>> + b.msg = NULL;
>> +
>> + apr_table_do(hb_table_check, &b, params, NULL);
>> +
>> + if (b.msg) {
>> + return b.msg;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +typedef struct hb_server_t {
>> + const char *ip;
>> + int busy;
>> + int ready;
>> + int seen;
>> +} hb_server_t;
>> +
>> +static void
>> +argstr_to_table(apr_pool_t *p, char *str, apr_table_t *parms)
>> +{
>> + char *key;
>> + char *value;
>> + char *strtok_state;
>> +
>> + key = apr_strtok(str, "&", &strtok_state);
>> + while (key) {
>> + value = strchr(key, '=');
>> + if (value) {
>> + *value = '\0'; /* Split the string in two */
>> + value++; /* Skip passed the = */
>> + }
>> + else {
>> + value = "1";
>> + }
>> + ap_unescape_url(key);
>> + ap_unescape_url(value);
>> + apr_table_set(parms, key, value);
>> + key = apr_strtok(NULL, "&", &strtok_state);
>> + }
>> +}
>> +
>> +static apr_status_t read_heartbeats(const char *path,
>> + apr_array_header_t *servers,
>> + apr_pool_t *pool)
>> +{
>> + apr_finfo_t fi;
>> + apr_status_t rv;
>> + apr_file_t *fp;
>> +
>> + if (!path) {
>> + return APR_SUCCESS;
>> + }
>> +
>> + rv = apr_file_open(&fp, path, APR_READ|APR_BINARY|APR_BUFFERED,
>> + APR_OS_DEFAULT, pool);
>> +
>> + if (rv) {
>> + return rv;
>> + }
>> +
>> + rv = apr_file_info_get(&fi, APR_FINFO_SIZE, fp);
>> +
>> + if (rv) {
>> + return rv;
>> + }
>> +
>> + {
>> + char *t;
>> + int lineno = 0;
>> + apr_table_t *hbt = apr_table_make(pool, 10);
>> + char buf[4096];
>> +
>> + while (apr_file_gets(buf, sizeof(buf), fp) == APR_SUCCESS) {
>> + hb_server_t *server;
>> + const char *ip;
>> + lineno++;
>> +
>> + /* comment */
>> + if (buf[0] == '#') {
>> + continue;
>> + }
>> +
>> +
>> + /* line format: <IP> <query_string>\n */
>> + t = strchr(buf, ' ');
>> + if (!t) {
>> + continue;
>> + }
>> +
>> + ip = apr_pstrndup(pool, buf, t - buf);
>> + t++;
>> + server = apr_pcalloc(pool, sizeof(hb_server_t));
>> + server->ip = ip;
>> + server->seen = -1;
>> + apr_table_clear(hbt);
>> +
>> + argstr_to_table(pool, apr_pstrdup(pool, t), hbt);
>> +
>> + if (apr_table_get(hbt, "busy")) {
>> + server->busy = atoi(apr_table_get(hbt, "busy"));
>> + }
>> +
>> + if (apr_table_get(hbt, "ready")) {
>> + server->ready = atoi(apr_table_get(hbt, "ready"));
>> + }
>> +
>> + if (apr_table_get(hbt, "lastseen")) {
>> + server->seen = atoi(apr_table_get(hbt, "lastseen"));
>> + }
>> +
>> + if (server->busy == 0 && server->ready != 0) {
>> + /* Server has zero threads active, but lots of them ready,
>> + * it likely just started up, so lets /4 the number ready,
>> + * to prevent us from completely flooding it with all new
>> + * requests.
>> + */
>> + server->ready = server->ready / 4;
>
> Nitpick: server->ready = server->ready >> 2 seems to be better.
>
>> + }
>> +
>> + APR_ARRAY_PUSH(servers, hb_server_t *) = server;
>> + }
>> + }
>> +
>> + return APR_SUCCESS;
>> +}
>> +
>> +static int hb_server_sort(const void *a_, const void *b_)
>> +{
>> + hb_server_t *a = (hb_server_t*)a;
>> + hb_server_t *b = (hb_server_t*)b;
>
> Guess that should be a_ and b_ above.
>
oh duh. fixed in r759673.
>> + if (a->ready == b->ready) {
>> + return 0;
>> + }
>> + else if (a->ready > b->ready) {
>> + return -1;
>> + }
>> + else {
>> + return 1;
>> + }
>> +}
>
>
> Regards
>
> Rüdiger
Thanks for reviewing,
Paul