virajjasani commented on issue #754: HBASE-22978 : Online slow response log URL: https://github.com/apache/hbase/pull/754#issuecomment-585829397 > Thanks for addressing all my concerns so far @virajjasani . I've tried to resolve all the inline discussions. Here's a summary of what I think remains outstanding. > > * log large responses (follow-on task)? > > * don’t assume “tooLarge” and “tooSlow” are mutually exclusive. > * replicate log entries into a file on HDFS or a new system table (follow-on task)? > * expose slow/large responses as a metric (follow-on task)? > * do we need to ship with thrift api parity? > * push filter and limit logic into the server, so all clients have the same experience > > * I think this requires adding a `TooSlowRequest` protobuf message with filter and limit parameters > * Maybe then `SlowLogPayload` is renamed to `SlowLogRecord` or something like that > * need separation of concerns between SlowLog.toString and formatting of output in the shell > * avoid adding to the surface area of HConstants > * proper interrupt handling within SlowLogEventHandler > * update configuration adoc to reference the feature adoc > * test cleanup > > * avoid sleeping for fixed time units, instead use waitFor > * maybe combine some methods containing repeated logic, introduce a helper method or two Thanks @ndimiduk for all your review. I have tried to cover these points in my recent commit except for `need separation of concerns between SlowLog.toString and formatting of output in the shell`. The reason being shell is not able to use Gson as efficiently as keeping `Gson.toJson()` in toString of SlowLogRecord because we have some serialization filters (neither does shell have any good pretty print library that works with our POJO - AFAIK). shell or any client would receive `List<SlowLogRecord>` as output of `Admin.getSlowLogResponses()`. In a way, keeping `Gson.toJson()` within `toString()` would ensure all clients can print output in pretty print format. Do you think this is good to maintain uniform print format across all clients? I have covered other points provided by you, please review. Thanks
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
