port kong's ring-balancer to orange #138

zhjwpku · 2017-08-02T10:02:08Z

This is premature pull , hope to acquire some advice

Signed-off-by: Zhao Junwang zhjwpku@gmail.com

sumory

good job!

sumory · 2017-08-02T14:47:34Z

orange/plugins/balancer/handler.lua

+    current_try.balancer_start = nil
+
+    -- record overall latency
+    ngx.ctx.KONG_BALANCER_TIME = (ngx.ctx.KONG_BALANCER_TIME or 0) + try_latency


KONG_BALANCER_TIME?

sorry about this typo

if one peer down why not change another peer(rule)

I am sure i understand your mean, if one peer down, the balancer() will be called again due to the set_more_tries() methods, actually the whole balancer_by_lua will be called again, see https://groups.google.com/d/msg/openresty-en/iCUSw-dxOek/jX6rh2ecBQAJ

sumory · 2017-08-02T14:55:20Z

orange/orange.lua

+
+    -- only care about upstreams stored in db
+    if utils.hostnameType(hostname) == "name" then
+        local upstreams = orange_db.get_json("balancer.selectors")


where is balancer.selectors initialized?

balancer.selectors will be initialized in Orange.init like other plugins. I want the balancer to reuse the selector and rules structure(upstream => selector, hosts => rules), this will make life easier.

you can see the balancer data structure here: https://gist.github.com/zhjwpku/ea6c9036897217abe1f50532b415f93c

sumory · 2017-08-02T14:58:05Z

orange/orange.lua

+            name, port = hostname, 80
+        end
+
+        if upstreams and type(upstreams) == "table" then


it's better that we replace these logic into plugin balancer's access method.

wow, definitely yes!

sumory · 2017-08-02T14:58:56Z

orange/plugins/balancer/api.lua

+local common_api = require("orange.plugins.common_api")
+
+local api = BaseAPI:new("balancer-api", 2)
+api:merge_apis(common_api("balancer"))


it's probably not compatible with the common_api

not sure about this, working on this

sumory · 2017-08-02T15:00:53Z

orange/utils/utils.lua

+-- @usage hostname_type("123.123.123.123")  -->  "ipv4"
+-- hostname_type("::1")              -->  "ipv6"
+-- hostname_type("some::thing")      -->  "ipv6", but invalid...
+function _M.hostnameType(name)


hostnameType -> hostname_type

sure, I will change the style

another style is you need indent 4 spaces......:)

we use editorconfig to control this style problems......

sure, will change that.

btw, I use vim (:%s/ / /g) to do the trick ;)

oh, the white space is striped, :%s/__/____/g, take _ as whitespace ;)

welcome to join the qq group 522410959 :)

another question, do you prefer to use git commit --amend to address the PR edit, or just another commit?

sumory · 2017-08-02T15:13:29Z

dynamic upstream is a plan in progressing for Orange v0.7. your pr could be a reference for this feature.

in my plan, dynamic upstream should be implemented as a plugin which would not break any current feature or usages such as divide upstream plugin.

users should consider whether they should use divide upstream or dynamic upstream by themselves. we'd better not restrict this point.

anyway, feel free to give any advice or implementation for this feature.

noname007 · 2017-08-03T02:24:21Z

how to use ?

noname007 · 2017-08-03T02:31:08Z

if could combine this and divid upstream may be more better job ! 👍

zhjwpku · 2017-08-03T02:48:55Z

@noname007 this feature uses mashape's ring-balancer, https://github.com/Mashape/lua-resty-dns-client, so that should be installed first.

$ luarocks install lua-resty-dns-client
$ luarocks install penlight

zhjwpku · 2017-08-03T03:02:03Z

@sumory in my current design, I am doing the dynamic upstream at the end of access context, at this time, the upstream_url has been already set. I handled with the upstream_url to see whether this is a upstream stored in the database, if it is, proxy_pass to the orange_upstream, otherwise it will remain its original target.

I like your proposal about implements this in a separate plugin, just one concern: what if we have same rules in both plugins? Not sure about this, just think it might be a little complicated to make sure the user won't add same rule in both plugins.

noname007 · 2017-08-03T03:19:46Z

i think no one will add same server to the same upstream :) your design good ，i can set the upstream url by divid upstream plug 。 good!

…

----- http://blog.soul11201.com 2017-08-03 11:02 GMT+08:00 Zhao Junwang <notifications@github.com>:

@sumory <https://github.com/sumory> in my current design, I am doing the dynamic upstream at the end of access context, at this time, the upstream_url has been already set. I handled with the upstream_url to see whether this is a upstream stored in the database, if it is, proxy_pass to the orange_upstream, otherwise it will remain its original target. I like your proposal about implements this in a separate plugin, just one concern: what if we have same rules in both plugins? Not sure about this, just think it might be a little complicated to make sure the user won't add same rule in both plugins. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#138 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIHtAOd-iKevG5kywVOtDTD9huIkzfGZks5sUTgsgaJpZM4Oq3ly> .

noname007 · 2017-08-03T07:11:43Z

i more like another commit !

…

----- http://blog.soul11201.com 2017-08-03 15:06 GMT+08:00 Zhao Junwang <notifications@github.com>:

***@***.**** commented on this pull request. ------------------------------ In orange/utils/utils.lua <#138 (comment)>: > @@ -193,6 +193,25 @@ function _M.load_module_if_exists(module_name) end end +--- checks the hostname type; ipv4, ipv6, or name. +-- Type is determined by exclusion, not by validation. So if it returns 'ipv6' then +-- it can only be an ipv6, but it is not necessarily a valid ipv6 address. +-- @param name the string to check (this may contain a portnumber) +-- @return string either; 'ipv4', 'ipv6', or 'name' +-- @Usage hostname_type("123.123.123.123") --> "ipv4" +-- hostname_type("::1") --> "ipv6" +-- hostname_type("some::thing") --> "ipv6", but invalid... +function _M.hostnameType(name) another question, do you prefer to use git commit --amend to address the PR edit, or just another commit? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#138 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIHtALjZxmLJXwPkmPZ14rjQ692DuC8vks5sUXF8gaJpZM4Oq3ly> .

noname007 · 2017-08-03T08:48:51Z

orange/utils/balancer.lua

+    if not balancer then
+        -- no balancer yet (or invalidated) so create a new one
+        balancer, err = ring_balancer.new({
+            wheelsize = upstream.slots,


wheelSize
http://mashape.github.io/lua-resty-dns-client/modules/resty.dns.balancer.html#new

zhjwpku · 2017-08-09T10:33:15Z

@sumory @noname007 Hi guys, please review the UI part, I am not good at JavaScript, sorry if I made some mistakes :)

noname007 · 2017-08-27T09:10:12Z

orange/utils/balancer.lua

+-- modified by zhjwpku@github
+
+local orange_db = require "orange.store.orange_db"
+local pl_tablex = require "pl.tablex"


"pl.tablex" what's this? please give a way to solve this kind of deps

https://github.com/stevedonovan/Penlight#installation

we'd better not import such dependencies like penlight for now

orangle · 2017-09-01T02:30:42Z

good job

noname007 · 2017-11-06T14:54:05Z

when i visit uri: /balancer by pull this code, i got some exception ,and i saw 404 at the dashboard error.log/

zhjwpku · 2017-11-07T00:53:00Z

@noname007 can you paste(make a gist or pastebin) the error, so i can have a look?

noname007 · 2017-11-07T01:56:23Z

@zhjwpku

2017/11/06 22:52:23 [error] 62058#0: *38 [lua] server.lua:59: func(): 404! not found., client: 127.0.0.1, server: , request: "GET /balancer/selectors?_=1509979943518 HTTP/1.1", host: "127.0.0.1:9999", referrer: "http://127.0.0.1:9999/balancer"

zhjwpku · 2017-11-07T02:50:55Z

@noname007 pls double check the file orange/plugins/balancer/api.lua is there. I am not sure what your codebase is, can you push your working space code to a git branch so we can discuss the code. Sorry about the trouble.

noname007 · 2017-11-07T09:35:40Z

@zhjwpku
github上 pull request的代码实际上是有一个分支的，把git配置好后，就能获取到你pr的代码。因此和你pr的代码是一致的。检查到api.lua 文件也是存在的。

这个是在dashboard中打开balancer 图标时候发生的的。
看这个错误，可能是是没有路由到。不知道是不是你有什么配置。

zhjwpku · 2017-11-07T14:21:37Z

@noname007 这个插件我们已经在用了，目前还没有遇到过你说的问题。我抽时间部署一台新的机器上试一下，之后给你结果~

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

thanks https://github.com/noname007 to point this ref: http://mashape.github.io/lua-resty-dns-client/modules/resty.dns.balancer.html#new Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

This bug cause the divde plugin not work, since for every none balancer request, the request has an addition `http://` Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

zhjwpku · 2017-11-16T08:30:45Z

@noname007 我把代码调整了一下，另外写了一篇部署的文档 Orange Balancer 安装及使用，你看下是否能够解决你之前遇到的问题

sumory · 2017-11-24T14:24:51Z

would you please pull this pr to branch v0.7.0-dev?

zhjwpku · 2017-11-24T15:02:51Z

@sumory sorry but as far as i can see, the destination branch is already v0.7.0-dev, teach me how if i am wrong ;)

sumory · 2017-11-24T16:06:06Z

this feature has been merged into v0.7.0-dev branch. hope you will take more effort for v0.7.0 release. 😃

sumory reviewed Aug 2, 2017

View reviewed changes

noname007 changed the base branch from master to v0.7.0-dev August 3, 2017 07:04

noname007 added the v0.7.0 label Aug 3, 2017

noname007 reviewed Aug 3, 2017

View reviewed changes

noname007 reviewed Aug 27, 2017

View reviewed changes

salt must be 8 characters or nil to respect lua-resty-string v0.10+

fc0d03e

zhjwpku added 8 commits November 14, 2017 16:47

port kong's ring-balancer to orange

1c46822

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

fix some typo and coding style

3e85861

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

wheelsize => wheelSize

f64bd8d

thanks https://github.com/noname007 to point this ref: http://mashape.github.io/lua-resty-dns-client/modules/resty.dns.balancer.html#new Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

take host's enable field into account

5432ad1

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

add Balancer UI

18938d2

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

fix bug

dae8961

This bug cause the divde plugin not work, since for every none balancer request, the request has an addition `http://` Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

fix bad log level

6144035

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

do some refactor

d93aea8

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>

sumory merged commit 2b67d35 into orlabs:v0.7.0-dev Nov 24, 2017

port kong's ring-balancer to orange #138

port kong's ring-balancer to orange #138

Conversation

zhjwpku commented Aug 2, 2017

sumory left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sumory commented Aug 2, 2017

noname007 commented Aug 3, 2017

noname007 commented Aug 3, 2017 • edited

zhjwpku commented Aug 3, 2017

zhjwpku commented Aug 3, 2017

noname007 commented Aug 3, 2017 via email

noname007 commented Aug 3, 2017 via email

Choose a reason for hiding this comment

zhjwpku commented Aug 9, 2017

noname007 Aug 27, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orangle commented Sep 1, 2017

noname007 commented Nov 6, 2017 • edited

zhjwpku commented Nov 7, 2017

noname007 commented Nov 7, 2017

zhjwpku commented Nov 7, 2017

noname007 commented Nov 7, 2017 • edited

zhjwpku commented Nov 7, 2017

zhjwpku commented Nov 16, 2017

sumory commented Nov 24, 2017

zhjwpku commented Nov 24, 2017

sumory commented Nov 24, 2017

noname007 commented Aug 3, 2017 •

edited

noname007 Aug 27, 2017 •

edited

noname007 commented Nov 6, 2017 •

edited

noname007 commented Nov 7, 2017 •

edited