Hi Adis,
Thanks for this patch, it is a very useful class.
Some remark about the documentation and the formats:
----------------------------------------------------
js:function:: StickTable.info()
js:function:: StickTable.lookup(key)
Maybe the specification and an example of the returnes values
will be welcome, because I guess that the table keys are hardcoded.
Something like :
{
type = <string> -- list of given string,
length = <int>,
...
}
or
{
expire = <integer> -- Specifiy the unit (ms or ?)
}
js:function:: StickTable.dump([filter])
The exact list of allowed operators will helps the user to use
your class. It seems that string or regexes operators are not
allowed, only the integer operator are taken in account. This
list is "eq", "ne", "le", "lt", "ge", "gt".
Same remarqk for allowed data type. Maybe a link to the HAProxy
documentation will be sufficient for the data type.
I see in the code that the filters can exceed 4 entries. This
limitation must be written in the doc.
I miss also the relation between oprators and between the content
of operators. I mean AND or OR. How I understand your example:
+ local filter = {
+ lt={{"gpc0", 1}, {"gpc1", 2}},
+ gt={{"conn_rate", 3}},
+ eq={{"conn_cur", 4}}
+ }
( gpc0 >= 1 AND gpc1 >= 2 ) OR (conn_rate > 3) OR (conn_cur = 4)
or
( gpc0 >= 1 OR gpc1 >= 2 ) OR (conn_rate > 3) OR (conn_cur = 4)
or
( gpc0 >= 1 AND gpc1 >= 2 ) AND (conn_rate > 3) AND (conn_cur = 4)
or
( gpc0 >= 1 OR gpc1 >= 2 ) AND (conn_rate > 3) AND (conn_cur = 4)
Are you sure that the syntax <operator>=<list of left, right operands>
is a good format ? Maybe something like the following, with the operator
as argument between the two operands. lines are implicitly OR, and columns
are AND:
+ local filter = {
+ {{"gpc0", "lt", 1}, {"gpc1", "lt", 2}},
+ {{"conn_rate", "gt", 3}},
+ {{"conn_cur", "eq", 4}}
+ }
It is read:
( gpc0 >= 1 OR gpc1 >= 2 ) AND (conn_rate > 3) AND (conn_cur = 4)
You can also implement a parser for natural language, in order to understand
directly the previous string ? This algoritm is very simple to implement:
https://en.wikipedia.org/wiki/Shunting-yard_algorithm
Idea of extension for the future: Maybe it will be safe to compile
sticktable filter during the initialisation of the Lua code, to avoid
runtime errors ?
Other point with the doc of this function, you must specify that the
execution of this function can be very long for millions of entry, even
if a filter is specified because the stick table is entirely scanned.
some remarks about the code and the logs:
-----------------------------------------
The line 182 of the patch contains space in place of tab.
Line 274 of your patch, I don't see any HA_SPIN_LOCK(STK_TABLE_LOCK
I don't known very well the thread, so maybe there are useles, maybe no.
Line 311: I see the decrement of the refcount without ATOMIC function
and whitout lock. Once again, I don't known very well the thread but
I send a warning. Maybe locks are useles, maybe no.
Line 286 of your patch. It seems that smp.flags is used uninitialized.
Maybe you should apply this change:
- smp.flags |= SMP_F_CONST;
+ smp.flags = SMP_F_CONST;
l.365, 369: The user doesn't have context about the error. there are the
first entry of the table, the second ? Which operator doesn't exists ?
L.380, 384: Which line is wrong ?
L.431: Your release the lock, so the next element relative to the current
"n", can disappear and the ebmb_next() can return wrong memory.
That's all.
Thierry
Hi guys,
I've attached a patch to add stick table support to Lua. Operations are
- Show basic table info
- Key lookup
- Table dump, optionally using data/column filter
One side note, the code provides support for multiple filters
(4 by default) while CLI doesn't support that, nor it complains about
multiple "<data.type> <op> <val>" clauses.
Also, if this patch is accepted, maybe we can use provided helper
functions in other places in the code.
It's always funny to reply to self, right sending email to public I've
spotted a bug. New patch attached.
SMT_T_SINT should be treated as ordinary signed integer, and shoud use
lua_pushinteger() on it?
On many places in the code it is noted as "64" bit integer, but in
struct stktable_type stktable_types[SMP_TYPES] = {
[SMP_T_SINT] = { "integer", 0, 4 },
Best regards,
Adis
<0001-MEDIUM-lua-Add-stick-table-support-for-Lua-read-only.patch>