Uploaded image for project: 'Spark'
  1. Spark
  2. SPARK-26768

Remove useless code in BlockManager

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 2.4.0
    • 3.0.0
    • Spark Core
    • None

    Description

      Recently, when I was reading some code of `BlockManager.getBlockData`, I found that there are useless code that would never reach. The related codes is as below:

       

      override def getBlockData(blockId: BlockId): ManagedBuffer = {
        if (blockId.isShuffle) {
          shuffleManager.shuffleBlockResolver.getBlockData(blockId.asInstanceOf[ShuffleBlockId])
        } else {
          getLocalBytes(blockId) match {
            case Some(blockData) =>
              new BlockManagerManagedBuffer(blockInfoManager, blockId, blockData, true)
            case None =>
              // If this block manager receives a request for a block that it doesn't have then it's
              // likely that the master has outdated block statuses for this block. Therefore, we send
              // an RPC so that this block is marked as being unavailable from this block manager.
              reportBlockStatus(blockId, BlockStatus.empty)
              throw new BlockNotFoundException(blockId.toString)
          }
        }
      }
      
      def getLocalBytes(blockId: BlockId): Option[BlockData] = {
        logDebug(s"Getting local block $blockId as bytes")
        // As an optimization for map output fetches, if the block is for a shuffle, return it
        // without acquiring a lock; the disk store never deletes (recent) items so this should work
        if (blockId.isShuffle) {
          val shuffleBlockResolver = shuffleManager.shuffleBlockResolver
          // TODO: This should gracefully handle case where local block is not available. Currently
          // downstream code will throw an exception.
          val buf = new ChunkedByteBuffer(
            shuffleBlockResolver.getBlockData(blockId.asInstanceOf[ShuffleBlockId]).nioByteBuffer())
          Some(new ByteBufferBlockData(buf, true))
        } else {
          blockInfoManager.lockForReading(blockId).map { info => doGetLocalBytes(blockId, info) }
        }
      }
      

      the `blockId.isShuffle` is checked twice, but however it seems that in the method calling hierarchy of `BlockManager.getLocalBytes`, the another callsite of the `BlockManager.getLocalBytes` is at `TorrentBroadcast.readBlocks` where the blockId can never be a `ShuffleBlockId`.

       

      So I think we should remove these useless code for easy reading.

       

      Attachments

        1. Selection_037.jpg
          14 kB
          liupengcheng

        Issue Links

          Activity

            People

              liupengcheng liupengcheng
              liupengcheng liupengcheng
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: