[jira] [Commented] (KAFKA-97) SocketServer.scala refers to Handler-specific variables

["Colin B. (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KAFKA-97?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13592405#comment-13592405 ] 

Colin B. commented on KAFKA-97:
-------------------------------

The code referenced no longer exists in 0.8. On quick inspection the only Kafka reference in SocketServer.scala is on line 185-186:
      case e: SocketException =>
        throw new KafkaException("Socket server failed to bind to %s:%d: %s.".format(socketAddress.getHostName, port, e.getMessage), e)

This bug should be closed. Perhaps a new bug should be opened to fix the reference to KafkaException.
                
> SocketServer.scala refers to Handler-specific variables
> -------------------------------------------------------
>
>                 Key: KAFKA-97
>                 URL: https://issues.apache.org/jira/browse/KAFKA-97
>             Project: Kafka
>          Issue Type: Bug
>            Reporter: Jay Kreps
>            Assignee: Jun Rao
>            Priority: Minor
>
> There is meant to be a separation between the socket server and the kafka server. The socket server doesn't know anything about kafka, it is a set of classes that take a request handler implementation, reads requests and writes responses. It is like a 300 line version of netty. Some logging slipped in that is specific to the KafkaHandler implementation. This is a violation of concerns and should be fixed, it is kind of like the map/reduce framework referencing stuff in a particular map/reduce job. If we want to refactor the handlers to better be able to give the name to a particular request that is good, but nothing in kafka.network should depend on anything but kafka.utils. Having worked on projects where we screwed this up i am a little sensitive--the network stuff is complex enough even if we don't mix it in with kafka logic, we want it to be testable on its own.
> Here is the code (beginning on SocketServer.scala, line 267):
>     if(requestLogger.isTraceEnabled) {
>       requestTypeId match {
>         case RequestKeys.Produce =>
>           requestLogger.trace("Handling produce request from " + channelFor(key).socket.getRemoteSocketAddress())
>         case RequestKeys.Fetch =>
>           requestLogger.trace("Handling fetch request from " + channelFor(key).socket.getRemoteSocketAddress())
>         case RequestKeys.MultiFetch =>
>           requestLogger.trace("Handling multi-fetch request from " + channelFor(key).socket.getRemoteSocketAddress())
>         case RequestKeys.MultiProduce =>
>           requestLogger.trace("Handling multi-produce request from " + channelFor(key).socket.getRemoteSocketAddress())
>         case RequestKeys.Offsets =>
>           requestLogger.trace("Handling offset request from " + channelFor(key).socket.getRemoteSocketAddress())
>         case _ => throw new InvalidRequestException("No mapping found for handler id " + requestTypeId)
>       }
>     }

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira