{"action":"created","issue":{"url":"https://api.github.com/repos/natsys/tempesta/issues/78","labels_url":"https://api.github.com/repos/natsys/tempesta/issues/78/labels{/name}","comments_url":"https://api.github.com/repos/natsys/tempesta/issues/78/comments","events_url":"https://api.github.com/repos/natsys/tempesta/issues/78/events","html_url":"https://github.com/natsys/tempesta/issues/78","id":61097466,"number":78,"title":"lb: improve design of load balancing interfaces","user":{"login":"vdmit11","id":5739502,"avatar_url":"https://avatars.githubusercontent.com/u/5739502?v=3","gravatar_id":"","url":"https://api.github.com/users/vdmit11","html_url":"https://github.com/vdmit11","followers_url":"https://api.github.com/users/vdmit11/followers","following_url":"https://api.github.com/users/vdmit11/following{/other_user}","gists_url":"https://api.github.com/users/vdmit11/gists{/gist_id}","starred_url":"https://api.github.com/users/vdmit11/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vdmit11/subscriptions","organizations_url":"https://api.github.com/users/vdmit11/orgs","repos_url":"https://api.github.com/users/vdmit11/repos","events_url":"https://api.github.com/users/vdmit11/events{/privacy}","received_events_url":"https://api.github.com/users/vdmit11/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/natsys/tempesta/labels/enhancement","name":"enhancement","color":"84b6eb"},{"url":"https://api.github.com/repos/natsys/tempesta/labels/review","name":"review","color":"009800"}],"state":"open","locked":false,"assignee":{"login":"krizhanovsky","id":884694,"avatar_url":"https://avatars.githubusercontent.com/u/884694?v=3","gravatar_id":"","url":"https://api.github.com/users/krizhanovsky","html_url":"https://github.com/krizhanovsky","followers_url":"https://api.github.com/users/krizhanovsky/followers","following_url":"https://api.github.com/users/krizhanovsky/following{/other_user}","gists_url":"https://api.github.com/users/krizhanovsky/gists{/gist_id}","starred_url":"https://api.github.com/users/krizhanovsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/krizhanovsky/subscriptions","organizations_url":"https://api.github.com/users/krizhanovsky/orgs","repos_url":"https://api.github.com/users/krizhanovsky/repos","events_url":"https://api.github.com/users/krizhanovsky/events{/privacy}","received_events_url":"https://api.github.com/users/krizhanovsky/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/natsys/tempesta/milestones/1","html_url":"https://github.com/natsys/tempesta/milestones/0.3.0%20Proxy%20Filter","labels_url":"https://api.github.com/repos/natsys/tempesta/milestones/1/labels","id":1015680,"number":1,"title":"0.3.0 Proxy Filter","description":"","creator":{"login":"krizhanovsky","id":884694,"avatar_url":"https://avatars.githubusercontent.com/u/884694?v=3","gravatar_id":"","url":"https://api.github.com/users/krizhanovsky","html_url":"https://github.com/krizhanovsky","followers_url":"https://api.github.com/users/krizhanovsky/followers","following_url":"https://api.github.com/users/krizhanovsky/following{/other_user}","gists_url":"https://api.github.com/users/krizhanovsky/gists{/gist_id}","starred_url":"https://api.github.com/users/krizhanovsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/krizhanovsky/subscriptions","organizations_url":"https://api.github.com/users/krizhanovsky/orgs","repos_url":"https://api.github.com/users/krizhanovsky/repos","events_url":"https://api.github.com/users/krizhanovsky/events{/privacy}","received_events_url":"https://api.github.com/users/krizhanovsky/received_events","type":"User","site_admin":false},"open_issues":20,"closed_issues":2,"state":"open","created_at":"2015-03-10T23:30:34Z","updated_at":"2015-03-20T20:17:55Z","due_on":"2015-03-22T21:00:00Z","closed_at":null},"comments":2,"created_at":"2015-03-13T15:25:23Z","updated_at":"2015-03-23T12:03:03Z","closed_at":null,"body":"**1.  `TfwScheduler`**\r\n\r\nThe interface is currently defined as follows:\r\n\r\n```\r\ntypedef struct {\r\n\tconst char *name;\r\n\tTfwServer *\t(*get_srv)(TfwMsg *msg);\r\n\tint\t\t(*add_srv)(TfwServer *srv);\r\n\tint\t\t(*del_srv)(TfwServer *srv);\r\n} TfwScheduler;\r\n```\r\n\r\nAnd it has two problems with the interface:\r\n\r\n1. It is stateful. Therefore:\r\n    - All schedulers have to maintain the global list of all servers, and the implementation is pretty much same in every scheduler. Tempesta FW maintains its own list of servers (to manage connections to backend servers). All operations on these lists should be synchronized which is tricky, because you have to update two lists atomically (otherwise a scheduler may return a server which is offline).\r\n    - Multiple schedulers cannot be used simultaneously, and a single scheduler cannot be used with multiple group of servers. There has to be one scheduler and one global list of servers.\r\n\r\n2. There is no failovering interface:\r\n    - A scheduler manages only online servers. When the server goes offline, it is deleted from the scheduler until a connection is re-established. That is done by `add_srv()`/`del_srv()` that delete an element from the global list of servers (implemented in every scheduler). The deletion should be synchronized with reads, hence there is extra locking/RCU protection in every scheduler. That has some overhead and makes the code more complex. It would be better to just mark a server offline instead of deleting it via `del_srv()`.\r\n    - The `get_srv()` returns a `TfwServer`, but we can have multiple parallel connections to the same server. A particular connection is chosen by round-robin/hash, so there is duplicate logic in scheduler modules and Tempesta FW.\r\n    - There is two levels of failovering: we have to switch between online/offline servers, but also we have to switch over open/closed connections within a server. The implementation turns out to be complex because it is spread across several units/modules.\r\n    - When a server goes offline, it is deleted from the scheduler via `del_srv()`. The `del_srv()` usually uses RCU to protect the deletion. Therefore, there is a relatively long period of time (an interval between grace periods) when a scheduler may return an offline server.\r\n\r\nI suggest the new set of interfaces for the schedulers:\r\n\r\n```\r\ntypedef struct TfwLbGroup TfwLbGroup;\r\ntypedef struct TfwLbSched TfwLbSched;\r\ntypedef struct TfwLbSrv TfwLbSrv;\r\n\r\n/**\r\n * A backend server representation containing a set of parallel connections.\r\n * It corresponds to one \"backend\" entry in the configuration file.\r\n */\r\nstruct TfwLbSrv {\r\n\t/* The @srv is the \"superclass\" field. It is exported to Tempesta FW.\r\n\t * other fields are private to the load balancing code. */\r\n\tTfwServer srv;\r\n\r\n\t/* The pool of parallel connections. Schedulers may access it. */\r\n\tstruct socket socks[];\r\n\tsize_t        socks_n;\r\n};\r\n\r\n/**\r\n * A group of TfwLbSrv objects. A bunch of \"backend\" entries can be grouped\r\n * together to a \"backend_group\" in the configuration file, so this structure\r\n * corresponds to the \"backend_group\".\r\n *\r\n * Schedulers made stateless. They don't maintain a list of servers anymore.\r\n * Instead, the list of servers is stored in the TfwLbGroup and managed by\r\n * the Tempesta FW (it has to do it anyway because it connects to servers).\r\n *\r\n * All writes to the TfwLbGroup are done via RCU, so schedulers can safely\r\n * assume that the data is immutable and don't do any locking or implement\r\n * complicated lock-free algorithms.\r\n */\r\nstruct TfwLbGroup {\r\n\t/* The list of servers belonging to the group. */\r\n\tTfwLbSrv         srvs[];\r\n\r\n\t/* The scheduling algorithm that chooses a server/connection.\r\n\t * @sched_data contains some private implementation-specific data:\r\n\t * round-robin counters, hashes, weights, etc. */\r\n\tTfwLbSched       *sched;\r\n\tvoid             *sched_data;\r\n};\r\n\r\n/**\r\n * The impotant point is the TfwLbGroup and TfwLbSrv objects are managed\r\n * by the Tempesta FW, but not the scheduler. A scheduler module is not allowed\r\n * to modify these structures. A scheduler is only allowed to write @sched_data.\r\n *\r\n * Functions in this structure are hooks rather than methods. These hooks allow\r\n * the scheduler to receive events: when a group or server is created/deleted,\r\n * or when a connection is established or closed.\r\n */\r\nstruct TfwLbSched {\r\n\t/* These hooks are invoked called Tempesta FW creates/deletes a TwLbGroup.\r\n\t * The scheduler can allocate/free @sched_data there. */\r\n\tint  create_group(TfwLbGroup *group);\r\n\tvoid delete_grpup(TfwLbGroup *group);\r\n\r\n\t/* These hooks are called when a TfwLb is added/deleted to/from a TfwLbGroup\r\n\t * to let the scheduler update its @sched_data accordingly. */\r\n\tint add_srv(TfwLbGroup *group, TfwLbSrv *srv);\r\n\tint del_srv(TfwLbGroup *group, TfwLbSrv *srv);\r\n\r\n\t/* The hook is called when the Tempesta FW detects and re-establishes\r\n\t * the closed connection. The scheduler must update @sched_data.\r\n\t *\r\n\t * The replace() is used instead of a add()/del() pair because it is\r\n\t * much easier to implement: Tempesta FW should protect any update of\r\n\t * TfwLbGroup with RCU (hence the replace()), so the scheduler doesn't\r\n\t * need to synchronize with get_conn() and implement complicated\r\n\t * lock-free structures. All it has to do is to replace a pointer. */\r\n\tint replace_conn(TfwLbGroup *group, TfwLbSrv *srv,\r\n\t\t\t struct socket *old, struct socket *new);\r\n\r\n\t/* The only \"non-hook\" function that implements the actual scheduling\r\n\t * algorithm (chooses a server/connection from the group). */\r\n\tint get_conn(TfwLbGroup *group, TfwServer **out_srv, struct sock **out_sk);\r\n};\r\n```\r\n\r\nThe main points are:\r\n\r\n1. Schedulers made more stateless. Now they don't have to maintain the global list of servers. Schedulers now use (but don't create or delete) `TfwLbGroup`/`TfwLbSrv` objects managed by tempesta. The Tempesta FW had to maintain the global state anyway to manage backend connections and configuration, so now we have just one global state instead of two, hence many synchronization problems go away.\r\n\r\n2. Multiple scheduler modules may be loaded at the same time, and each `TfwLbGroup` may choose its own scheduler. In the configuration file it looks like:\r\n```\r\nbackend_group sched=hash {\r\n         backend 10.0.0.1;\r\n         backend 10.0.0.2;\r\n         backend 10.0.0.3;\r\n         ...\r\n}\r\n```\r\n\r\n3. Scheduler modules now are aware of sockets. They listen for connect/disconnect events and implement some failovering logic. That may seem awkward and bloated, but actually that makes the implementation much simpler. There is a [prototype implementation in the `failover` branch](https://github.com/natsys/tempesta/blob/failover/tempesta_fw/lb/tfw_lb_http.c#L137), and a scheduler implementation fits to the 100 lines of code or so. Without sockets, the implementation would consist of much more code across different modules/units. As a bonus, the prototype implementation always returns an alive socket if there is at least one alive connection in a group. That makes failover switching trivial.\r\n\r\n**2.  `tfw_sched_http`**\r\n\r\nThe problems with the scheduler are:\r\n\r\n1. It requires backend groups, while the Tempesta FW doesn't implement them. The scheduler  already implements a kind of `TfwLbSrv`/`TfwLbGroup` objects, see [`TfwSchedHttpCfgBeGroup`/`TfwBeGroupAddrSet`/`TfwRrPtrSet`](https://github.com/natsys/tempesta/blob/master/tempesta_fw/sched/tfw_sched_http.c#L116). That leads to duplicate logic again: the Tempesta FW implements the global list of servers, and the `tfw_sched_http` implements the list of servers (and also groups servers together).\r\n\r\n2. Backend connections are managed by Tempesta FW, but groups of servers are managed by the `tfw_sched_http`. Hence, a system administrator has to duplicate each `backend` entry in the configuration file when he uses the `tfw_sched_http` (see [tfw_sched_http.conf](https://github.com/natsys/tempesta/blob/master/etc/tfw_sched_http.conf)).\r\n\r\n3. The `tfw_sched_http` uses the round-robin approach to pick a server from a group, so again, we have the same logic as in `tfw_sched_rr`.\r\n\r\nIt seems the `tfw_sched_http` should not be a scheduler. Usually schedulers are much more generic, they deal with resources, collections, queues, weights, etc. The `tfw_sched_http` routes HTTP requests based on a static set of `URI`/`Host` values. That seems more like a request dispatcher than a scheduler.\r\n\r\nI suggest to move the `tfw_sched_http` to a separate entity, I will call it \"dispatcher\". The dispatcher's responsibility is to choose a `TfwLbGroup` for the HTTP request, while the scheduler's responsibility is to choose a particular server within the group.\r\n\r\nThere is a [prototype implementation in the `failover` branch](https://github.com/natsys/tempesta/blob/failover/tempesta_fw/lb/tfw_lb_http.c#L237).\r\n\r\nThe whole `tfw_sched_http`'s job is done by the single function in the prototype implementation, but that will change: the #76 imples large amounts of sites proxied by the Tempesta FW, so the dispatcher presumably will implement some efficient search algorithms (search trees, hash tables, etc). From this point of view, the separate dispatcher concept is even more benefitcial: the dispatcher will work with the large set of groups, and the scheduler will select a server from the small group, hence different optimization techniques may be used."},"comment":{"url":"https://api.github.com/repos/natsys/tempesta/issues/comments/84965275","html_url":"https://github.com/natsys/tempesta/issues/78#issuecomment-84965275","issue_url":"https://api.github.com/repos/natsys/tempesta/issues/78","id":84965275,"user":{"login":"vdmit11","id":5739502,"avatar_url":"https://avatars.githubusercontent.com/u/5739502?v=3","gravatar_id":"","url":"https://api.github.com/users/vdmit11","html_url":"https://github.com/vdmit11","followers_url":"https://api.github.com/users/vdmit11/followers","following_url":"https://api.github.com/users/vdmit11/following{/other_user}","gists_url":"https://api.github.com/users/vdmit11/gists{/gist_id}","starred_url":"https://api.github.com/users/vdmit11/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vdmit11/subscriptions","organizations_url":"https://api.github.com/users/vdmit11/orgs","repos_url":"https://api.github.com/users/vdmit11/repos","events_url":"https://api.github.com/users/vdmit11/events{/privacy}","received_events_url":"https://api.github.com/users/vdmit11/received_events","type":"User","site_admin":false},"created_at":"2015-03-23T12:03:03Z","updated_at":"2015-03-23T12:03:03Z","body":"> should do their work as early as possible\r\n> parts of the request can be sent to server immediately\r\nThat may not be beneficial in some cases: during a DoS attack, the most of incoming HTTP requests are malicious. Such requests are dropped at certain stages of parsing, but before that happens, we send some data to backend servers and do the scheduling job. That is waste of resources, especially for the backend server.\r\n"}}