Hi Simon, On Thu, Jun 16, 2011 at 09:10:37AM +0900, Simon Horman wrote: > Hi, > > I believed this addresses all of the issues raised with the previous > version of this change. I have also verified that connections are shutdown > a backend is placed in maintenance mode.
That's really cool, thank you. I've quickly checked your patches an see nothing obviously wrong, so we'll have to merge them to detect any possible issue. One question though : what flags are emitted in the logs when a session is closed that way ? I'm wondering if we should not add a new flag such as "D" (for "down") and/or "K" (for "killed") on the first character to indicate why the session was terminated. It would also make room for the ability to kill sessions from the CLI later. But this new flag could impact more code, we have to check this. One comment about the "struct list server" in the session. I'm afraid this name could cause confusion with the "srv" field, especially since it appears at the beginning. Maybe we should find another name such as "by_srv" or something like this to more clearly indicate it's the node used to chain the session from the server's list. Last point, I'm amazed by the number of functions you managed to turn to static. A few years ago this would not have been possible at all, and when I read your changes, for each of them I remember what we changed which made that possible. This means to me that the code is becoming much more modular and that's a very good news ! I'll try to be careful in the future about what functions can be turned to static after changes (provided it makes sense of course). Thanks, Willy
