Hi Mark,

Thanks for the patch. I don’t like usage of floating point, but the
luasocket documentation says that the settimeout() function accept only
second. In this case, the usage of floating point seems be to be a good
way.

Can you split in a second commit the fix of comments from the effective
patch, and avoid this kind of changes:

   -    int tmout;
   +    int    tmout;

Just because, this kind of changes are useless, and it add noisy
information in the patch.

A last point: could you explain int the message of the patch the
goal of these patch. To avoid a search, this is the link of the official
luasocket setimeout function:

    http://w3.impa.br/~diego/software/luasocket/tcp.html#settimeout

Thanks
Thierry


> On 7 Mar 2018, at 18:16, Mark Lakes <mla...@signalsciences.com> wrote:
> 
> In regards to earlier conversation, herein is a patch attached for the 
> feature.
> From the mail archive:
> https://www.mail-archive.com/haproxy@formilux.org/msg27806.html
> https://www.mail-archive.com/haproxy@formilux.org/msg27807.html
> 
> Mark Lakes 
> Signal Sciences | www.signalsciences.com |
> 
> conversation participants:
> Willy Tarreau
> Adis Nezirovic
> Nick Galbreath
> 
> --------- Last conversation and decision agreement ----------
> Nick Galbreath Thu, 09 Nov 2017 20:44:28 -0800
> 
> 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
> >
> 
> 
> ----------
> 
> 
> 
> <0001-MINOR-lua-allow-socket-api-settimeout-to-accept-inte.patch>


Reply via email to