[jira] [Commented] (KAFKA-617) Refactor KafkaApis.handleOffsetRequest

["Jay Kreps (JIRA)" <ji...@apache.org>]

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

Jay Kreps commented on KAFKA-617:
---------------------------------

Evidence of code funk:

  def getOffsetsBefore(timestamp: Long, maxNumOffsets: Int): Seq[Long] = {
    val segsArray = segments.view
    var offsetTimeArray: Array[(Long, Long)] = null
    if(segsArray.last.size > 0)
      offsetTimeArray = new Array[(Long, Long)](segsArray.length + 1)
    else
      offsetTimeArray = new Array[(Long, Long)](segsArray.length)

    for(i <- 0 until segsArray.length)
      offsetTimeArray(i) = (segsArray(i).start, segsArray(i).messageSet.file.lastModified)
    if(segsArray.last.size > 0)
      offsetTimeArray(segsArray.length) = (logEndOffset, time.milliseconds)

    var startIndex = -1
    timestamp match {
      case OffsetRequest.LatestTime =>
        startIndex = offsetTimeArray.length - 1
      case OffsetRequest.EarliestTime =>
        startIndex = 0
      case _ =>
        var isFound = false
        debug("Offset time array = " + offsetTimeArray.foreach(o => "%d, %d".format(o._1, o._2)))
        startIndex = offsetTimeArray.length - 1
        while (startIndex >= 0 && !isFound) {
          if (offsetTimeArray(startIndex)._2 <= timestamp)
            isFound = true
          else
            startIndex -=1
        }
    }

    val retSize = maxNumOffsets.min(startIndex + 1)
    val ret = new Array[Long](retSize)
    for(j <- 0 until retSize) {
      ret(j) = offsetTimeArray(startIndex)._1
      startIndex -= 1
    }
    // ensure that the returned seq is in descending order of offsets
    ret.toSeq.sortBy(- _)
  }

I suspect this predates our knowledge of using scala collections fully and can maybe be reduced to a few maps and filters.
                
> Refactor KafkaApis.handleOffsetRequest
> --------------------------------------
>
>                 Key: KAFKA-617
>                 URL: https://issues.apache.org/jira/browse/KAFKA-617
>             Project: Kafka
>          Issue Type: Improvement
>            Reporter: Jay Kreps
>
> This code path is just really bad.
> All the business logic is in LogManager and Log.
> Also the implementation is really funky.
> LogManager already supports getLog and log allows you to get the segment list so there shouldn't be any logic except in KafkaApis.

--
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