Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Willy Tarreau]

On Mon, Jul 24, 2017 at 02:38:41PM +0200, Willy Tarreau wrote:
> On Mon, Jul 24, 2017 at 02:04:16PM +0200, Thierry FOURNIER wrote:
> > On Thu, 20 Jul 2017 15:26:52 +0200
> > You will found in attchement a patch which add the proxy name as member
> > of the proxy object.
> > 
> > Willy, can you apply it ?
> 
> I'd like to but there's no attachment, so even trying hard I'm failing to :-)

Now merged, thanks!
Willy

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Willy Tarreau]

On Mon, Jul 24, 2017 at 02:04:16PM +0200, Thierry FOURNIER wrote:
> On Thu, 20 Jul 2017 15:26:52 +0200
> You will found in attchement a patch which add the proxy name as member
> of the proxy object.
> 
> Willy, can you apply it ?

I'd like to but there's no attachment, so even trying hard I'm failing to :-)

Willy

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Willy Tarreau]

On Mon, Jul 24, 2017 at 02:27:03PM +0200, Thierry FOURNIER wrote:
> an other case pop in my mind: with this solution, the "listen" proxies
> will be declared in both lists. I think that it is the expected behaviour,
> but I have some doubt about the usage.

Yes I think it's desirable.

> I can add a "listens" list.

I'd rather avoid this, which would add further confusion. The initial
purpose of allowing the same name in different proxies was to make it
seamless to break a full proxy ("listen") into a frontend and backend.
By having a separate list, it would become a problem again.

Willy

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Thierry FOURNIER]

On Mon, 24 Jul 2017 14:03:30 +0200
Willy Tarreau  wrote:

> Hi Thierry,
> 
> On Mon, Jul 24, 2017 at 01:30:23PM +0200, Thierry FOURNIER wrote:
> > Ok. After brainstorm, I think that the it will be netter to keep the
> > current behaviour to avoid breaking existing Lua implementations.
> > 
> > Adding other entries with prefix "@f:" and "@b:" in the same list that
> > the existing will disturb existing Lua which browse the lists.
> > Returning a list (point 1 & 2) also break it.
> > 
> > I think that the most reliable way is adding anoter tree. We keep the
> > "proxies" tree base with existing, and we add two trees "frontends" and
> > "backends" which contains respecticely the list of frontends and
> > backends.
> 
> Initially I wasn't much fond of this one but after some thinking I
> convinced myself that it's probably the best solution. In fact you
> never need to dereference a proxy by its name, unless :
>   - you know its type (eg: for use_backend or to list servers)
>   - you want to reference its table, which is already forbidden in
> duplicate names.
> 
> So in the end, being able to spot a proxy by its type+name, or being
> able to lookup its name regardless of the type in case of a table seems
> the best solution.  Hence frontends+backends+proxies.


an other case pop in my mind: with this solution, the "listen" proxies
will be declared in both lists. I think that it is the expected behaviour,
but I have some doubt about the usage.

I can add a "listens" list.

Thierry


> 
> > Or another tree called "all_proxies" which contains names and a list of
> > frontend/backend with the same name.
> 
> It would add more confusion.
> 
> > > >> Also, apply the same logic to the 'Listener' and 'Server' classes.
> > > > 
> > > > These ones do not support duplicate names so we should not need to 
> > > > impact
> > > > them.
> > > 
> > > This is just for convenience and uniformity, Proxy.servers/listeners
> > > returns a table/hash of objects with names as keys, but for example when
> > > I want to pass such object to some other Lua function I have to manually
> > > copy the name (or wrap the object), since the object itself doesn't
> > > expose name info.
> > 
> > 
> > You're right, it will be better adding the name in the object. I will
> > do this.
>  
> Thanks,
> Willy

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Adis Nezirovic]

On 07/24/2017 01:30 PM, Thierry FOURNIER wrote:
> I think that the most reliable way is adding anoter tree. We keep the
> "proxies" tree base with existing, and we add two trees "frontends" and
> "backends" which contains respecticely the list of frontends and
> backends.

This would work for me too, maybe just add a note in docs regarding that
bug in core.proxies (masking frontends/backends with the same name).

>> This is just for convenience and uniformity, Proxy.servers/listeners
>> returns a table/hash of objects with names as keys, but for example when
>> I want to pass such object to some other Lua function I have to manually
>> copy the name (or wrap the object), since the object itself doesn't
>> expose name info.
> 
> 
> You're right, it will be better adding the name in the object. I will
> do this.

Great, that would be convenient, thanks.


Best regards,
Adis

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Thierry FOURNIER]

On Thu, 20 Jul 2017 15:26:52 +0200
Adis Nezirovic  wrote:

> On 07/20/2017 02:55 PM, Willy Tarreau wrote:
> > So you can have :
> >   0 or 1 "listen"
> >   0 or 1 "frontend" + 0 or 1 "backend"
> > 
> > Just a few ideas come to my mind :
> >   - is it possible to store arrays into arrays ? I mean, could we have
> > for example core.proxies["foo"].side[FRONT|BACK] where each side is
> > set only when the two differ ?
> > 
> >   - or is it to return a list (1 or 2 elements) ? That would be even more
> > convenient to use. If you have "p = core.proxies[foo]" and can use p
> > then next(p), you're sure to always scan everything.
> > 
> >   - otherwise I like the principle of being able to force the type using
> > a prefix "@f:" or "@b:" in front of the name. But we must be careful
> > not to insert duplicates. So probably by default only "listen" instances
> > would be created without the "@" prefix. My initial idea was to be able
> > to always return the plain form and point to whichever exists, but for
> > enumeration it would require to keep only the "@" form, which might be
> > more complicated.
> 
> We currently use a variant of second proposal, when working internally
> with backend/frontends with same name, type/side should still be
> indicated. Simple list/array might be easiest thing to implement.
> 
> >> Also, apply the same logic to the 'Listener' and 'Server' classes.
> > 
> > These ones do not support duplicate names so we should not need to impact
> > them.
> 
> This is just for convenience and uniformity, Proxy.servers/listeners
> returns a table/hash of objects with names as keys, but for example when
> I want to pass such object to some other Lua function I have to manually
> copy the name (or wrap the object), since the object itself doesn't
> expose name info.


You will found in attchement a patch which add the proxy name as member
of the proxy object.

Willy, can you apply it ?


> Best regards,
> Adis

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Willy Tarreau]

Hi Thierry,

On Mon, Jul 24, 2017 at 01:30:23PM +0200, Thierry FOURNIER wrote:
> Ok. After brainstorm, I think that the it will be netter to keep the
> current behaviour to avoid breaking existing Lua implementations.
> 
> Adding other entries with prefix "@f:" and "@b:" in the same list that
> the existing will disturb existing Lua which browse the lists.
> Returning a list (point 1 & 2) also break it.
> 
> I think that the most reliable way is adding anoter tree. We keep the
> "proxies" tree base with existing, and we add two trees "frontends" and
> "backends" which contains respecticely the list of frontends and
> backends.

Initially I wasn't much fond of this one but after some thinking I
convinced myself that it's probably the best solution. In fact you
never need to dereference a proxy by its name, unless :
  - you know its type (eg: for use_backend or to list servers)
  - you want to reference its table, which is already forbidden in
duplicate names.

So in the end, being able to spot a proxy by its type+name, or being
able to lookup its name regardless of the type in case of a table seems
the best solution.  Hence frontends+backends+proxies.

> Or another tree called "all_proxies" which contains names and a list of
> frontend/backend with the same name.

It would add more confusion.

> > >> Also, apply the same logic to the 'Listener' and 'Server' classes.
> > > 
> > > These ones do not support duplicate names so we should not need to impact
> > > them.
> > 
> > This is just for convenience and uniformity, Proxy.servers/listeners
> > returns a table/hash of objects with names as keys, but for example when
> > I want to pass such object to some other Lua function I have to manually
> > copy the name (or wrap the object), since the object itself doesn't
> > expose name info.
> 
> 
> You're right, it will be better adding the name in the object. I will
> do this.
 
Thanks,
Willy

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Thierry FOURNIER]

On Thu, 20 Jul 2017 15:26:52 +0200
Adis Nezirovic  wrote:

> On 07/20/2017 02:55 PM, Willy Tarreau wrote:
> > So you can have :
> >   0 or 1 "listen"
> >   0 or 1 "frontend" + 0 or 1 "backend"
> > 
> > Just a few ideas come to my mind :
> >   - is it possible to store arrays into arrays ? I mean, could we have
> > for example core.proxies["foo"].side[FRONT|BACK] where each side is
> > set only when the two differ ?
> > 
> >   - or is it to return a list (1 or 2 elements) ? That would be even more
> > convenient to use. If you have "p = core.proxies[foo]" and can use p
> > then next(p), you're sure to always scan everything.
> > 
> >   - otherwise I like the principle of being able to force the type using
> > a prefix "@f:" or "@b:" in front of the name. But we must be careful
> > not to insert duplicates. So probably by default only "listen" instances
> > would be created without the "@" prefix. My initial idea was to be able
> > to always return the plain form and point to whichever exists, but for
> > enumeration it would require to keep only the "@" form, which might be
> > more complicated.
> 
> We currently use a variant of second proposal, when working internally
> with backend/frontends with same name, type/side should still be
> indicated. Simple list/array might be easiest thing to implement.


Ok. After brainstorm, I think that the it will be netter to keep the
current behaviour to avoid breaking existing Lua implementations.

Adding other entries with prefix "@f:" and "@b:" in the same list that
the existing will disturb existing Lua which browse the lists.
Returning a list (point 1 & 2) also break it.

I think that the most reliable way is adding anoter tree. We keep the
"proxies" tree base with existing, and we add two trees "frontends" and
"backends" which contains respecticely the list of frontends and
backends.

Or another tree called "all_proxies" which contains names and a list of
frontend/backend with the same name.


> >> Also, apply the same logic to the 'Listener' and 'Server' classes.
> > 
> > These ones do not support duplicate names so we should not need to impact
> > them.
> 
> This is just for convenience and uniformity, Proxy.servers/listeners
> returns a table/hash of objects with names as keys, but for example when
> I want to pass such object to some other Lua function I have to manually
> copy the name (or wrap the object), since the object itself doesn't
> expose name info.


You're right, it will be better adding the name in the object. I will
do this.

Thierry

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Adis Nezirovic]

On 07/20/2017 02:55 PM, Willy Tarreau wrote:
> So you can have :
>   0 or 1 "listen"
>   0 or 1 "frontend" + 0 or 1 "backend"
> 
> Just a few ideas come to my mind :
>   - is it possible to store arrays into arrays ? I mean, could we have
> for example core.proxies["foo"].side[FRONT|BACK] where each side is
> set only when the two differ ?
> 
>   - or is it to return a list (1 or 2 elements) ? That would be even more
> convenient to use. If you have "p = core.proxies[foo]" and can use p
> then next(p), you're sure to always scan everything.
> 
>   - otherwise I like the principle of being able to force the type using
> a prefix "@f:" or "@b:" in front of the name. But we must be careful
> not to insert duplicates. So probably by default only "listen" instances
> would be created without the "@" prefix. My initial idea was to be able
> to always return the plain form and point to whichever exists, but for
> enumeration it would require to keep only the "@" form, which might be
> more complicated.

We currently use a variant of second proposal, when working internally
with backend/frontends with same name, type/side should still be
indicated. Simple list/array might be easiest thing to implement.

>> Also, apply the same logic to the 'Listener' and 'Server' classes.
> 
> These ones do not support duplicate names so we should not need to impact
> them.

This is just for convenience and uniformity, Proxy.servers/listeners
returns a table/hash of objects with names as keys, but for example when
I want to pass such object to some other Lua function I have to manually
copy the name (or wrap the object), since the object itself doesn't
expose name info.


Best regards,
Adis

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Willy Tarreau]

Hi guys,

On Thu, Jul 20, 2017 at 02:35:22PM +0200, Adis Nezirovic wrote:
> On 07/20/2017 12:17 PM, Thierry FOURNIER wrote:
> > I understand the problem, but I can't accept this patch because it makes
> > the proxies list unusable. Your patch remove the proxies names, and the
> > user cannot have solution for knowning the real name of the proxies
> > now called 1, 2, 3, ...
> > 
> > I propose 2 solutions:
> > 
> >  - Two new lists called "frontend" and "backend" with no name collision
> >by design
> > 
> >  - a prefix "@f:" for fontend and "@b:" for backends and keep the name
> >without prefix for the compatibility.
> > 
> > I prefer the two new lists.
> 
> Hello Thierry,
> 
> Yeah, dropping the name might be inconvenient, we were traversing the
> proxy list and calling get_stats() on each object, and fetched "pxname"
> attribute.
> 
> How about different approach, can we set "name" and "type" attribute for
> Proxy objects? Maybe expose some other attributes (id, ...)

The difficulty is that it is allowed to have both a frontend and a backend
with the same name. This is so that configs using a "listen" section can
easily be split into "frontend" and "backend" and continue to work (think
about "use_backend", "track bkname/srvname", tables and so on).

The per-proxy lookup is necessary in order to be able to find a proxy by
its name (think about the Lua equivalent of "use_backend"). However what
is certain is that for a given name, there cannot be more than two proxies
because you cannot have overlapping capabilities (frontend and backend).
So you can have :
  0 or 1 "listen"
  0 or 1 "frontend" + 0 or 1 "backend"

Just a few ideas come to my mind :
  - is it possible to store arrays into arrays ? I mean, could we have
for example core.proxies["foo"].side[FRONT|BACK] where each side is
set only when the two differ ?

  - or is it to return a list (1 or 2 elements) ? That would be even more
convenient to use. If you have "p = core.proxies[foo]" and can use p
then next(p), you're sure to always scan everything.

  - otherwise I like the principle of being able to force the type using
a prefix "@f:" or "@b:" in front of the name. But we must be careful
not to insert duplicates. So probably by default only "listen" instances
would be created without the "@" prefix. My initial idea was to be able
to always return the plain form and point to whichever exists, but for
enumeration it would require to keep only the "@" form, which might be
more complicated.

> Also, apply the same logic to the 'Listener' and 'Server' classes.

These ones do not support duplicate names so we should not need to impact
them.

Just my two cents,
Willy

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Adis Nezirovic]

On 07/20/2017 12:17 PM, Thierry FOURNIER wrote:
> I understand the problem, but I can't accept this patch because it makes
> the proxies list unusable. Your patch remove the proxies names, and the
> user cannot have solution for knowning the real name of the proxies
> now called 1, 2, 3, ...
> 
> I propose 2 solutions:
> 
>  - Two new lists called "frontend" and "backend" with no name collision
>by design
> 
>  - a prefix "@f:" for fontend and "@b:" for backends and keep the name
>without prefix for the compatibility.
> 
> I prefer the two new lists.

Hello Thierry,

Yeah, dropping the name might be inconvenient, we were traversing the
proxy list and calling get_stats() on each object, and fetched "pxname"
attribute.

How about different approach, can we set "name" and "type" attribute for
Proxy objects? Maybe expose some other attributes (id, ...)

Also, apply the same logic to the 'Listener' and 'Server' classes.


Best regards,
Adis

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Thierry FOURNIER]

Hi Adis,

Sorry, I dont saw this patch proposal.

I understand the problem, but I can't accept this patch because it makes
the proxies list unusable. Your patch remove the proxies names, and the
user cannot have solution for knowning the real name of the proxies
now called 1, 2, 3, ...

I propose 2 solutions:

 - Two new lists called "frontend" and "backend" with no name collision
   by design

 - a prefix "@f:" for fontend and "@b:" for backends and keep the name
   without prefix for the compatibility.

I prefer the two new lists.

Thierry



On Wed, 19 Jul 2017 14:34:16 +0200
Willy Tarreau  wrote:

> Hi Adis,
> 
> I missed this one. Thierry, I *think* it's OK, are you OK with me merging
> it ?
> 
> Willy
> 
> On Mon, Jun 05, 2017 at 05:37:23PM +0200, Adis Nezirovic wrote:
> > Hi guys,
> > 
> > While playing with Lua API I've noticed that core.proxies attribute
> > doesn't return all the proxies, more precisely the ones with same names
> > (e.g. for frontend and backend with the same name it would only return
> > the latter one).
> > 
> > Here is a patch which tries to fix that, avoiding using the proxy name
> > as a key for Lua table.
> > We use a plain integer offsets (starting from 1) to create numerical
> > array/table iterable by both pairs() and ipairs() in Lua.
> > 
> > The patch also applies cleanly to 1.7 branch.
> > 
> > Best regards,
> > Adis
> 
> > >From f201afcd5ec9790a40907ab24e089db28a9cb6f1 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Adis=20Nezirovi=C4=87?= 
> > Date: Mon, 5 Jun 2017 17:10:51 +0200
> > Subject: [PATCH] BUG/MEDIUM: lua: Support proxies with identical names in
> >  core.proxies
> > 
> > When we have frontends/backends with identical names, core.proxies
> > returns only the last one, using proxy->id (i.e. proxy name) to insert
> > the value in Lua table. We should add them to numerical array/table, to
> > be able to access all proxies.
> > ---
> >  src/hlua_fcn.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
> > index 8406bfe5..89bf0b3f 100644
> > --- a/src/hlua_fcn.c
> > +++ b/src/hlua_fcn.c
> > @@ -893,6 +893,7 @@ int hlua_proxy_shut_bcksess(lua_State *L)
> >  int hlua_fcn_post_init(lua_State *L)
> >  {
> > struct proxy *px;
> > +   int index;
> >  
> > /* get core array. */
> > if (lua_getglobal(L, "core") != LUA_TTABLE)
> > @@ -903,8 +904,9 @@ int hlua_fcn_post_init(lua_State *L)
> > lua_newtable(L);
> >  
> > /* List all proxies. */
> > +   index = 1;
> > for (px = proxy; px; px = px->next) {
> > -   lua_pushstring(L, px->id);
> > +   lua_pushinteger(L, index++);
> > hlua_fcn_new_proxy(L, px);
> > lua_settable(L, -3);
> > }
> > -- 
> > 2.13.0
> > 
> 
> 

Re: [PATCH] Support proxies with identical names in Lua core.proxies

[Willy Tarreau]

Hi Adis,

I missed this one. Thierry, I *think* it's OK, are you OK with me merging
it ?

Willy

On Mon, Jun 05, 2017 at 05:37:23PM +0200, Adis Nezirovic wrote:
> Hi guys,
> 
> While playing with Lua API I've noticed that core.proxies attribute
> doesn't return all the proxies, more precisely the ones with same names
> (e.g. for frontend and backend with the same name it would only return
> the latter one).
> 
> Here is a patch which tries to fix that, avoiding using the proxy name
> as a key for Lua table.
> We use a plain integer offsets (starting from 1) to create numerical
> array/table iterable by both pairs() and ipairs() in Lua.
> 
> The patch also applies cleanly to 1.7 branch.
> 
> Best regards,
> Adis

> >From f201afcd5ec9790a40907ab24e089db28a9cb6f1 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Adis=20Nezirovi=C4=87?= 
> Date: Mon, 5 Jun 2017 17:10:51 +0200
> Subject: [PATCH] BUG/MEDIUM: lua: Support proxies with identical names in
>  core.proxies
> 
> When we have frontends/backends with identical names, core.proxies
> returns only the last one, using proxy->id (i.e. proxy name) to insert
> the value in Lua table. We should add them to numerical array/table, to
> be able to access all proxies.
> ---
>  src/hlua_fcn.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
> index 8406bfe5..89bf0b3f 100644
> --- a/src/hlua_fcn.c
> +++ b/src/hlua_fcn.c
> @@ -893,6 +893,7 @@ int hlua_proxy_shut_bcksess(lua_State *L)
>  int hlua_fcn_post_init(lua_State *L)
>  {
>   struct proxy *px;
> + int index;
>  
>   /* get core array. */
>   if (lua_getglobal(L, "core") != LUA_TTABLE)
> @@ -903,8 +904,9 @@ int hlua_fcn_post_init(lua_State *L)
>   lua_newtable(L);
>  
>   /* List all proxies. */
> + index = 1;
>   for (px = proxy; px; px = px->next) {
> - lua_pushstring(L, px->id);
> + lua_pushinteger(L, index++);
>   hlua_fcn_new_proxy(L, px);
>   lua_settable(L, -3);
>   }
> -- 
> 2.13.0
> 

[PATCH] Support proxies with identical names in Lua core.proxies

[Adis Nezirovic]

Hi guys,

While playing with Lua API I've noticed that core.proxies attribute
doesn't return all the proxies, more precisely the ones with same names
(e.g. for frontend and backend with the same name it would only return
the latter one).

Here is a patch which tries to fix that, avoiding using the proxy name
as a key for Lua table.
We use a plain integer offsets (starting from 1) to create numerical
array/table iterable by both pairs() and ipairs() in Lua.

The patch also applies cleanly to 1.7 branch.

Best regards,
Adis
>From f201afcd5ec9790a40907ab24e089db28a9cb6f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adis=20Nezirovi=C4=87?= 
Date: Mon, 5 Jun 2017 17:10:51 +0200
Subject: [PATCH] BUG/MEDIUM: lua: Support proxies with identical names in
 core.proxies

When we have frontends/backends with identical names, core.proxies
returns only the last one, using proxy->id (i.e. proxy name) to insert
the value in Lua table. We should add them to numerical array/table, to
be able to access all proxies.
---
 src/hlua_fcn.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
index 8406bfe5..89bf0b3f 100644
--- a/src/hlua_fcn.c
+++ b/src/hlua_fcn.c
@@ -893,6 +893,7 @@ int hlua_proxy_shut_bcksess(lua_State *L)
 int hlua_fcn_post_init(lua_State *L)
 {
 	struct proxy *px;
+	int index;
 
 	/* get core array. */
 	if (lua_getglobal(L, "core") != LUA_TTABLE)
@@ -903,8 +904,9 @@ int hlua_fcn_post_init(lua_State *L)
 	lua_newtable(L);
 
 	/* List all proxies. */
+	index = 1;
 	for (px = proxy; px; px = px->next) {
-		lua_pushstring(L, px->id);
+		lua_pushinteger(L, index++);
 		hlua_fcn_new_proxy(L, px);
 		lua_settable(L, -3);
 	}
-- 
2.13.0