httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe Jr." <wmr...@gmail.com>
Subject h2 request processing score behavior...
Date Wed, 27 Apr 2016 18:25:59 GMT
Within h2_task.c, we have

static apr_status_t h2_task_process_request(h2_task *task, conn_rec *c)
{
    const h2_request *req = task->request;
    conn_state_t *cs = c->cs;
    request_rec *r;

    ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
                  "h2_task(%s): create request_rec", task->id);
    r = h2_request_create_rec(req, c);
    if (r && (r->status == HTTP_OK)) {
        ap_update_child_status(c->sbh, SERVER_BUSY_READ, r);

        if (cs) {
            cs->state = CONN_STATE_HANDLER;
        }
        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
                      "h2_task(%s): start process_request", task->id);
        ap_process_request(r);

Contrast this to http_core.c...

    ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ, c);
    while ((r = ap_read_request(c)) != NULL) {
        apr_interval_time_t keep_alive_timeout =
r->server->keep_alive_timeout;

        /* To preserve legacy behaviour, use the keepalive timeout from the
         * base server (first on this IP:port) when none is explicitly
         * configured on this server.
         */
        if (!r->server->keep_alive_timeout_set) {
            keep_alive_timeout = c->base_server->keep_alive_timeout;
        }

        c->keepalive = AP_CONN_UNKNOWN;
        /* process the request if it was read without error */

        ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
        if (r->status == HTTP_OK) {
            if (cs)
                cs->state = CONN_STATE_HANDLER;
            ap_process_request(r);

It certainly seems this slave worker needs to be in
SERVER_BUSY_WRITE at the time we run the
process_request logic to correspond to the expected
behavior of status analysis tools, so will update this
logic accordingly.

The next issue I do not understand, this seems broken
whether it is happening in the master or slave worker...

static apr_status_t pass_out(apr_bucket_brigade *bb, void *ctx)
{
    pass_out_ctx *pctx = ctx;
    conn_rec *c = pctx->c;
    apr_status_t status;
    apr_off_t bblen;

    if (APR_BRIGADE_EMPTY(bb)) {
        return APR_SUCCESS;
    }

    ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_WRITE, c);
    apr_brigade_length(bb, 0, &bblen);
    h2_conn_io_bb_log(c, 0, APLOG_TRACE2, "master conn pass", bb);
    status = ap_pass_brigade(c->output_filters, bb);

This does whack the request line so I've corrected the logic
accordingly by not passing the conn_rec, but if this is only
invoked in the slave worker, correcting the first issue above
seems to make this redundant (and offers a small optimization
to simply remove it.)

The only other issue I see is in h2_session.c...

                ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_READ,
c);
                if (!h2_is_acceptable_connection(c, 1)) {
                    update_child_status(session, SERVER_BUSY_READ,
"inadequate security");
                    h2_session_shutdown(session,
NGHTTP2_INADEQUATE_SECURITY, NULL, 1);
                }
                else {
                    update_child_status(session, SERVER_BUSY_READ, "init");

Can h2_is_acceptable_connection update the connection's
virtual host?  If it does, we want to repeat the _from_conn
status refresh to correct the vhost before setting the status
to "init".  I have not touched this code.

Mime
View raw message