Hi Simon, On Mon, Nov 28, 2016 at 04:18:52PM +0100, Simon Horman wrote: > Hi, > > this short series is an RFC implementation of adding JSON format > output to show (info|stat). It also adds a new show schema json > stats command to allow retreival of the schema which describes > the JSON output of show (info|stat).
Thanks! > Some areas for possible discussion: > * Use of STAT_STARTED in first patch I don't see how you can get rid of it. You can't rely on the field number to know that you've already dumped something since you can start by dumping empty fields. The only solution would be to systematically append a comma after a value like we do in C enums, but I'm not sure most JSON parsers will like it. > * Possible automatic generation of (part) of schema in 2nd patch I think this schema dump is very useful, I just don't know what level of maintenance it will require (not much I think). I'm not convinced that the amount of work needed to make it more automatic would make it more maintainable to be honnest. Probably we just have to add a comment next to the type definitions, mentionning that anyone adding a type there must also update the dump functions and the json schema. I noticed various typos there, that I'll comment on in a reply to the patch. > * Improved documentation > > Some discussion of the size of JSON output is included as an appendix > to the changelog of the first patch. OK so the size should be around 45kB for show stats on a listener with two servers, meaning roughly 10kB per line (I think the first "show info" output was in fact "show stat"). For now we cannot dump less than a line at once so we must be cautious, because many of us run with 8kB buffers, and even those running at 16kB might hit the limit once we increase the number of fields and they contain large values. In theory we could have a sub-level corresponding to the field being dumped, but it could cause some inconsistencies on a line and is not desirable (at least for now). I'm just realizing that we could very well replace the buffer contents with "output buffer too short (%d), needs %d". Thanks, Willy
