Welcome! Log In Create A New Profile

Advanced

lua socket api settimeout in seconds vs. milliseconds

Posted by Nick Galbreath 
Nick Galbreath
lua socket api settimeout in seconds vs. milliseconds
November 07, 2017 09:00PM
Hello,

Thanks for a great Lua API!

I have a question regarding the socket:settimeout function.

It appears to only accept an integer value for seconds, but internally
converts to milliseconds (see below).

Is there a reason to enforce whole seconds, or could this be relaxed to
accept a lua number (float/double).?

Happy to provide a patch, but just checking first since I'm new to HAProxy.

Thanks for your time,

regards

nickg

around line 2442 in hluc.c:

__LJMP static int hlua_socket_settimeout(struct lua_State *L)
{
struct hlua_socket *socket;
int tmout;
struct xref *peer;
struct appctx *appctx;
struct stream_interface *si;
struct stream *s;

MAY_LJMP(check_args(L, 2, "settimeout"));

socket = MAY_LJMP(hlua_checksocket(L, 1));
tmout = MAY_LJMP(luaL_checkinteger(L, 2)) * 1000;
Adis Nezirovic
Re: lua socket api settimeout in seconds vs. milliseconds
November 08, 2017 10:00AM
On 11/07/2017 08:49 PM, Nick Galbreath wrote:
> I have a question regarding the socket:settimeout function.
>
> It appears to only accept an integer value for seconds, but internally
> converts to milliseconds (see below).
>
> Is there a reason to enforce whole seconds, or could this be relaxed to
> accept a lua number (float/double).?

Hi Nick,

HAProxy uses Lua 5.3 which has proper integers. There is already a
similar situation where we have both, seconds and milliseconds
(hlua_sleep/hlua_msleep), so maybe that's the best solution here too,
just add another variant using milliseconds.

Best regards,
Adis
Nick Galbreath
Re: lua socket api settimeout in seconds vs. milliseconds
November 10, 2017 05:40AM
Hello Adis,

We could certainly add another API/Lua function but it might be easier to
change

luaL_checkinteger(L, 2) in

tmout = MAY_LJMP(luaL_checkinteger(L, 2)) * 1000;

to luaL_checknumber(L, 2), along with appropriate cast to int.

Then we have backwards compatibility, less documentation to write, and get
millisecond timeouts.

If people want a separate API, I'm happy to do that too, just more work.


Please advise, and I'll make a patch either way. I'm unfamiliar with the
HAProxy development process, so any tips or pointers are welcome,


thanks all!


nickg

On Wed, Nov 8, 2017 at 12:45 AM, Adis Nezirovic <anezirovic@haproxy.com>
wrote:

> On 11/07/2017 08:49 PM, Nick Galbreath wrote:
> > I have a question regarding the socket:settimeout function.
> >
> > It appears to only accept an integer value for seconds, but internally
> > converts to milliseconds (see below).
> >
> > Is there a reason to enforce whole seconds, or could this be relaxed to
> > accept a lua number (float/double).?
>
> Hi Nick,
>
> HAProxy uses Lua 5.3 which has proper integers. There is already a
> similar situation where we have both, seconds and milliseconds
> (hlua_sleep/hlua_msleep), so maybe that's the best solution here too,
> just add another variant using milliseconds.
>
> Best regards,
> Adis
>



--
Nick Galbreath | Founder / CTO | @ngalbreath |
Signal Sciences | www.signalsciences.com | @signalsciences |
Willy Tarreau
Re: lua socket api settimeout in seconds vs. milliseconds
November 10, 2017 05:50AM
Hi Nick,

On Thu, Nov 09, 2017 at 08:27:29PM -0800, Nick Galbreath wrote:
> Hello Adis,
>
> We could certainly add another API/Lua function but it might be easier to
> change
>
> luaL_checkinteger(L, 2) in
>
> tmout = MAY_LJMP(luaL_checkinteger(L, 2)) * 1000;
>
> to luaL_checknumber(L, 2), along with appropriate cast to int.
>
> Then we have backwards compatibility, less documentation to write, and get
> millisecond timeouts.

At least it seems important to round up non-null values to the next
millisecond, otherwise we may observe busy loops when users specify
sleep delays smaller than the millisecond, as haproxy's internal
clock is millisecond-based (poll()'s resolution).

> If people want a separate API, I'm happy to do that too, just more work.

I think it should work as you propose it, more or less the round up of
course.

> Please advise, and I'll make a patch either way. I'm unfamiliar with the
> HAProxy development process, so any tips or pointers are welcome,

It's important to CC the subsystem maintainer when submitting a change,
since they are supposed to have the last word on submissions in their
area. This is done here since Thierry maintains the Lua area. Please
carefully read CONTRIBUTING in the sources directory, it's not very
long and will help you ensure that all your patches are easily merged.
And you're welcome to propose changes to this file if something is
unclear :-)

Thanks,
Willy
Nick Galbreath
Re: lua socket api settimeout in seconds vs. milliseconds
November 10, 2017 05:50AM
thanks wily.

re: " CONTRIBUTING in the sources directory," -

yes, that is what I was looking for! thanks for the tip.

re: least it seems important to round up non-null values to the next
millisecond.

Definitely, we can and should add some checks for invalid values, etc.

I'll read CONTRIBUTING, and set up my dev env, try a patch, and report
back appropriately.

regards,

n






On Thu, Nov 9, 2017 at 8:37 PM, Willy Tarreau <w@1wt.eu> wrote:

> Hi Nick,
>
> On Thu, Nov 09, 2017 at 08:27:29PM -0800, Nick Galbreath wrote:
> > Hello Adis,
> >
> > We could certainly add another API/Lua function but it might be easier to
> > change
> >
> > luaL_checkinteger(L, 2) in
> >
> > tmout = MAY_LJMP(luaL_checkinteger(L, 2)) * 1000;
> >
> > to luaL_checknumber(L, 2), along with appropriate cast to int.
> >
> > Then we have backwards compatibility, less documentation to write, and
> get
> > millisecond timeouts.
>
> At least it seems important to round up non-null values to the next
> millisecond, otherwise we may observe busy loops when users specify
> sleep delays smaller than the millisecond, as haproxy's internal
> clock is millisecond-based (poll()'s resolution).
>
> > If people want a separate API, I'm happy to do that too, just more work.
>
> I think it should work as you propose it, more or less the round up of
> course.
>
> > Please advise, and I'll make a patch either way. I'm unfamiliar with the
> > HAProxy development process, so any tips or pointers are welcome,
>
> It's important to CC the subsystem maintainer when submitting a change,
> since they are supposed to have the last word on submissions in their
> area. This is done here since Thierry maintains the Lua area. Please
> carefully read CONTRIBUTING in the sources directory, it's not very
> long and will help you ensure that all your patches are easily merged.
> And you're welcome to propose changes to this file if something is
> unclear :-)
>
> Thanks,
> Willy
>



--
Nick Galbreath | Founder / CTO | @ngalbreath |
Signal Sciences | www.signalsciences.com | @signalsciences |
Sorry, only registered users may post in this forum.

Click here to login