https://bz.apache.org/bugzilla/show_bug.cgi?id=64338
--- Comment #5 from Ruediger Pluem <[email protected]> --- (In reply to Christopher Schultz from comment #4) > (In reply to Ruediger Pluem from comment #3) > > > > 1. Why not setting this environment variable in the same block as > > BALANCER_WORKER_ROUTE (line 622) but only when a session route was supplied? > > This is my ignorance showing. I figured that if there was no route, then the > request wouldn't actually be delivered anywhere and it would be wasted > effort. What does the condition on line 632 actually check? It checks if a route was supplied by the client, but I think the worker state might be interesting even in the case that the client did not supply a route, e.g. because it is a freshly opened browser which has no session cookie yet. A worker will be chosen according to the LB policy then. > > /* Rewrite the url from 'balancer://url' > * to the 'worker_scheme://worker_hostname[:worker_port]/url' > * This replaces the balancers fictional name with the > * real hostname of the elected worker. > */ > access_status = rewrite_url(r, *worker, url); > /* Add the session route to request notes if present */ > if (route) { // <=================== > line 632 > apr_table_setn(r->notes, "session-sticky", sticky); > apr_table_setn(r->notes, "session-route", route); > > /* Add session info to env. */ > apr_table_setn(r->subprocess_env, > "BALANCER_SESSION_STICKY", sticky); > apr_table_setn(r->subprocess_env, > "BALANCER_SESSION_ROUTE", route); > apr_table_setn(r->subprocess_env, > "BALANCER_WORKER_STATUS", > apr_itoa(r->pool, (*worker)->s->status)); > > I'm happy to move this to a more appropriate location. > > > 2. I haven't checked it we could run into issues as apr_itoa takes an int > > while worker->s->status is an unsigned int. At least this could cause > > compiler warnings. > > I got no compiler warnings using gcc, though I didn't specifically enable > anything during the build. I just assumed warnings were enabled during > compile. > > But I do take your point: status is unsigned int and I'm using atoi. All the > current statuses are non-negative and don't run higher than 0x800 so we're > safe for now, but it's definitely not future-proof. > > I didn't see apr_uitoa, but of course there is still apr_ltoa. I just wanted > to be as minimally impactful on performance as possible. I suppose I could > check the magnitude of 'status' and use apr_ltoa if necessary after a cast. > > What is your recommendation, here? So far I have none. I am waiting for my fellow developers :-). If no one speaks up here, we probably just commit and discuss later on the development list. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
