Hi Thierry, thanks for feedback. Addressed concerns in the new attached patch.
http://w3.impa.br/~diego/software/luasocket/tcp.html#settimeout Description: instead of hlua_socket_settimeout() accepting only integers, allow user to specify float and double as well. Convert to milliseconds much like cli_parse_set_timeout but also sanity check the value. -mark On Wed, Mar 7, 2018 at 9:55 AM, Thierry Fournier <tfourn...@arpalert.org> wrote: > 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> > >
0001-MINOR-lua-allow-socket-api-settimeout-to-accept-inte.patch
Description: Binary data
