Don't retain abstract req in response

[seglo]

I picked up ticket 5859 because it was marked with the newbie flag and it seemed like a good opportunity to learn more about about the request and response lifecycle in the Kafka broker.

We currently store AbstractRequest in RequestChannel.Request.bodyAndSize. RequestChannel.Request is, in turn, stored in RequestChannel.Response. We keep the latter until the response is sent to the client.

However, after KafkaApis.handle, we no longer need AbstractRequest apart from its string representation for logging. We could potentially replace AbstractRequest with a String representation (if the relevant logging is enabled). The String representation is generally small while some AbstractRequest subclasses can be pretty large. The largest one is ProduceRequest and we clear the underlying ByteBuffer explicitly in KafkaApis.handleProduceRequest. We could potentially remove that special case if AbstractRequest subclasses were not retained.

This was originally suggested by Jason Gustafson in the following PR apache#3801 (comment)

I removed the parsed AbstractRequest that was stored in RequestChannel.Request in the private bodyAndSize: RequestAndSize variable. Ismael points out that the AbstractRequest is mainly used by KafkaApi, so we should only need a reference there and nowhere else. However when I investigated I found several other usages within RequestChannel.Request itself. I've summarized these usages below:

• updateRequestMetrics - Checks to see if the parsed request is a FetchRequest to access the isFromFollower value. Ismail mentioned in the ticket 5859 that "it needs the ApiKeys instance", but I don't know what he means by that
• updateRequestMetrics - Requires the parsed size of the request in bytes to update the requestBytesHist metric
• updateRequestMetrics - Calls requestDesc(true) or requestDesc(false) depending on whether trace logging enabled. It looks like it's used when request logging is enabled.
• Request constructor has a trace logging line that calls requestDesc(true)

I made some decisions on my own. Below is a summary for review.

• Parse the request in each handler in KafkaApi. Ideally this would be done a higher level and passed into each handler, but I was hesitant about changing all the handler method signatures. Effectively the response is only parsed once since only one handler can be called per request AFAIK.
• I generate a detailed request description in the default constructor of RequestChannel.Request. I generate a detailed description because the codebase isn't consistent about requiring details or not. For example, there's a usage that's called for every instance of RequestChannel.Request (https://github.com/seglo/kafka/blob/trunk/core/src/main/scala/kafka/network/RequestChannel.scala#L107), so I thought it would be acceptable to parse the request once for details and retain it for use in other logging RequestChannel.Request.requestDesc.
• I handle a special case while I have a local reference of the parsed request to determine if it's a FetchRequest and cache its isFromFollower property for use in updateRequestMetrics. I'm not sure how else to gain this information.
• This implementation has the consequence of parsing the AbstractRequest twice, once for KafkaApi and once for the RequestChannel.Request constructor. I would be interested in finding a non-invasive way to avoid this, while maintaining immutability as requested by Ismael in the original ticket.

[seglo]

Closed in favour of the implementation in #2