Kafka
  1. Kafka
  2. KAFKA-745

Remove getShutdownReceive() and other kafka specific code from the RequestChannel

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 0.8.0
    • Fix Version/s: None
    • Component/s: network
    • Labels:
      None

      Description

      Jay's suggestion from KAFKA-736 is to get rid of getShutdownReceive() and kafka request specific code from the generic requestchannel

      1. KAFKA-745-v1.patch
        31 kB
        Sriram Subramanian

        Activity

        Hide
        Sriram Subramanian added a comment -

        This is a first version.

        1. Removes all the metric stuff,requestObj etc from Request.
        2.Adds Network metrics to track network level parameters
        3. Any form of delayed request now uses a trimmed version of the Request object.
        4. Added Request metrics to the kafka layer to track kafka specific parameters

        TODO - Need to add logging.

        • Some more cleanup
        • We will not get all the different times for a given request as part of one log statement anymore. I think this is fine for the ability to maintain clean code.
        • W.r.t the logging in the network layer, we would get info from the Request object (need to add a toString here to ignore the buffer) and nothing from the actual request. I think this is fine since knowing the request type is not useful at this layer based on the explanation above.
        Show
        Sriram Subramanian added a comment - This is a first version. 1. Removes all the metric stuff,requestObj etc from Request. 2.Adds Network metrics to track network level parameters 3. Any form of delayed request now uses a trimmed version of the Request object. 4. Added Request metrics to the kafka layer to track kafka specific parameters TODO - Need to add logging. Some more cleanup We will not get all the different times for a given request as part of one log statement anymore. I think this is fine for the ability to maintain clean code. W.r.t the logging in the network layer, we would get info from the Request object (need to add a toString here to ignore the buffer) and nothing from the actual request. I think this is fine since knowing the request type is not useful at this layer based on the explanation above.
        Hide
        Sriram Subramanian added a comment -

        Took a look at this. The main issue is that we seem to be tracking a bunch of metrics per request type and they all end up being tracked in the network layer. This has really made the network layer ugly. These metrics are really of two types Network level metrics and Request level metrics.

        1. Metrics that can be tracked at the network level are those that do not have any large difference based on the request type. For example, queueTime is a metric that is supposed to be the amount of time a request spends in the request queue. Tracking this for each request type does not really make sense. This is a network level property and should be tracked at that level.

        2. Metrics that can be tracked at the request level have a large difference based on the request type. For example, local time is the amount of time the KafkaApi handle method takes to complete. This largely depends on the request type and should be tracked at the request level.

        To summarize, with the decoupling specified above we have the following metrics at the two levels

        Network metrics
        -----------------------
        // time a request spent in a request queue
        val queueTimeHist = newHistogram(name + "-QueueTimeMs")

        // time to send the response to the requester from the response queue
        val responseSendTimeHist = newHistogram(name + "-ResponseSendTimeMs")

        // total time taken for the request to be served
        val totalTimeHist = newHistogram(name + "-TotalTimeMs")

        Request metrics
        ------------------------

        // request rate by type
        val requestRate = newMeter(name + "-RequestsPerSec", "requests", TimeUnit.SECONDS)

        // time a request takes to be processed at the local broker
        val localTimeHist = newHistogram(name + "-LocalTimeMs")

        // time a request takes to wait on remote brokers (only relevant to fetch and produce requests)
        val remoteTimeHist = newHistogram(name + "-RemoteTimeMs")

        With the separation specified above, any request can be defined as

        queueTime + localTime + remoteTime + responseSendTime = totalTime

        We can totally remove any kafkaapi dependency in the network layer with the proposed separation.

        Show
        Sriram Subramanian added a comment - Took a look at this. The main issue is that we seem to be tracking a bunch of metrics per request type and they all end up being tracked in the network layer. This has really made the network layer ugly. These metrics are really of two types Network level metrics and Request level metrics. 1. Metrics that can be tracked at the network level are those that do not have any large difference based on the request type. For example, queueTime is a metric that is supposed to be the amount of time a request spends in the request queue. Tracking this for each request type does not really make sense. This is a network level property and should be tracked at that level. 2. Metrics that can be tracked at the request level have a large difference based on the request type. For example, local time is the amount of time the KafkaApi handle method takes to complete. This largely depends on the request type and should be tracked at the request level. To summarize, with the decoupling specified above we have the following metrics at the two levels Network metrics ----------------------- // time a request spent in a request queue val queueTimeHist = newHistogram(name + "-QueueTimeMs") // time to send the response to the requester from the response queue val responseSendTimeHist = newHistogram(name + "-ResponseSendTimeMs") // total time taken for the request to be served val totalTimeHist = newHistogram(name + "-TotalTimeMs") Request metrics ------------------------ // request rate by type val requestRate = newMeter(name + "-RequestsPerSec", "requests", TimeUnit.SECONDS) // time a request takes to be processed at the local broker val localTimeHist = newHistogram(name + "-LocalTimeMs") // time a request takes to wait on remote brokers (only relevant to fetch and produce requests) val remoteTimeHist = newHistogram(name + "-RemoteTimeMs") With the separation specified above, any request can be defined as queueTime + localTime + remoteTime + responseSendTime = totalTime We can totally remove any kafkaapi dependency in the network layer with the proposed separation.
        Hide
        Sriram Subramanian added a comment -

        I would like to take this and complete it. Let me know if you want to work on it and I will reassign.

        Show
        Sriram Subramanian added a comment - I would like to take this and complete it. Let me know if you want to work on it and I will reassign.
        Hide
        Sriram Subramanian added a comment -

        This bug also tracks decoupling requestObj from RequestChannel if possible.

        Show
        Sriram Subramanian added a comment - This bug also tracks decoupling requestObj from RequestChannel if possible.

          People

          • Assignee:
            Sriram Subramanian
            Reporter:
            Neha Narkhede
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development