Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha1
    • Fix Version/s: 2.7.0
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Implementation of truncate in HDFS-3107 does not allow truncating files which are in a snapshot. It is desirable to be able to truncate and still keep the old file state of the file in the snapshot.

      1. HDFSSnapshotWithTruncateDesign.docx
        0.1 kB
        Tsz Wo Nicholas Sze
      2. HDFS-7056.15_branch2.patch
        123 kB
        Plamen Jeliazkov
      3. editsStored.xml
        30 kB
        Konstantin Shvachko
      4. editsStored
        5 kB
        Konstantin Shvachko
      5. HDFS-3107-HDFS-7056-combined-15.patch
        145 kB
        Konstantin Shvachko
      6. HDFS-7056-15.patch
        124 kB
        Konstantin Shvachko
      7. HDFS-3107-HDFS-7056-combined-13.patch
        145 kB
        Konstantin Shvachko
      8. HDFS-7056-13.patch
        123 kB
        Konstantin Shvachko
      9. HDFS-3107-HDFS-7056-combined.patch
        140 kB
        Konstantin Shvachko
      10. HDFS-3107-HDFS-7056-combined.patch
        139 kB
        Plamen Jeliazkov
      11. HDFS-3107-HDFS-7056-combined.patch
        139 kB
        Plamen Jeliazkov
      12. HDFS-7056.patch
        117 kB
        Konstantin Shvachko
      13. HDFS-3107-HDFS-7056-combined.patch
        140 kB
        Plamen Jeliazkov
      14. HDFS-7056.patch
        117 kB
        Konstantin Shvachko
      15. HDFS-3107-HDFS-7056-combined.patch
        133 kB
        Plamen Jeliazkov
      16. HDFS-7056.patch
        111 kB
        Plamen Jeliazkov
      17. HDFS-3107-HDFS-7056-combined.patch
        134 kB
        Plamen Jeliazkov
      18. HDFS-7056.patch
        111 kB
        Plamen Jeliazkov
      19. HDFS-3107-HDFS-7056-combined.patch
        131 kB
        Plamen Jeliazkov
      20. HDFS-7056.patch
        105 kB
        Plamen Jeliazkov
      21. HDFS-3107-HDFS-7056-combined.patch
        130 kB
        Plamen Jeliazkov
      22. HDFS-7056.patch
        107 kB
        Konstantin Shvachko
      23. HDFS-7056.patch
        100 kB
        Konstantin Shvachko
      24. HDFS-3107-HDFS-7056-combined.patch
        124 kB
        Plamen Jeliazkov
      25. HDFS-7056.patch
        100 kB
        Konstantin Shvachko
      26. HDFSSnapshotWithTruncateDesign.docx
        93 kB
        Guo Ruijing

        Issue Links

          Activity

          Hide
          shv Konstantin Shvachko added a comment -

          Here are some details of the implementation. LMK if it sounds reasonable. I'll update the design doc accordingly.

          When file is in a snapshot and not on a block boundary then a new last block is created for the truncated file, which will hold the truncated data of the original file. The original block will remain in the snapshot copy of the file unchanged.
          There are two main parts in implementing this.

          1. The truncate recovery on a DN will copy a part of the last block replica into a new block file, instead of just truncating the replica as HDFS-3107 does now. The truncating logic can be kept as an optimization for the case when file is not in a snapshot.
          2. SnapshotCopy of INodeFile should be extended with a list of blocks, referencing blocks that composed the file when the snapshot was taken. When there are multiple snapshots of the same file each snapshot copy may have different lists of blocks, if file have been truncated and appended between the snapshots. As a prt of this change we will need to adjust logic when deleting a file and deleting a snapshot because some blocks may or some may not need to be invalidated. Here is an overview of operations related to the change.
            • There is no change in createSnapshot operation. The snapshot diffs will be introduced when the files are actually modified.
            • Append is not changing, because the file has the list of blocks, which is separate from the lists of snapshot copies.
            • File delete should check if a block belongs to a snapshot before sending it to the invalidates queue. Only the initial prefix of blocks common with the latest snapshot should be retained in BlocksMap. Therefore removeFile should find the common prefix of blocks with the latest snapshot and invalidate the rest of them.
            • Deleting a snapshot copy of a file should check if a block belongs to another snapshot before sending it to the invalidates queue. It is not necessary to check all snapshots, only the previous and the next snapshots should be verified for blocks common with snapshot being deleted. Therefore removing a snapshot one should find a prefix of blocks of the current snapshot which are in common with either the previous or the next snapshot. The rest of the blocks can be invalidated.
            • I would propose to copy the entire list of blocks to the snapshot copy. This will simplify the implementation. We can optimize this later by storing references only to the blocks that are different between the current state of the file and the snapshot.
          Show
          shv Konstantin Shvachko added a comment - Here are some details of the implementation. LMK if it sounds reasonable. I'll update the design doc accordingly. When file is in a snapshot and not on a block boundary then a new last block is created for the truncated file, which will hold the truncated data of the original file. The original block will remain in the snapshot copy of the file unchanged. There are two main parts in implementing this. The truncate recovery on a DN will copy a part of the last block replica into a new block file, instead of just truncating the replica as HDFS-3107 does now. The truncating logic can be kept as an optimization for the case when file is not in a snapshot. SnapshotCopy of INodeFile should be extended with a list of blocks, referencing blocks that composed the file when the snapshot was taken. When there are multiple snapshots of the same file each snapshot copy may have different lists of blocks, if file have been truncated and appended between the snapshots. As a prt of this change we will need to adjust logic when deleting a file and deleting a snapshot because some blocks may or some may not need to be invalidated. Here is an overview of operations related to the change. There is no change in createSnapshot operation. The snapshot diffs will be introduced when the files are actually modified. Append is not changing, because the file has the list of blocks, which is separate from the lists of snapshot copies. File delete should check if a block belongs to a snapshot before sending it to the invalidates queue. Only the initial prefix of blocks common with the latest snapshot should be retained in BlocksMap. Therefore removeFile should find the common prefix of blocks with the latest snapshot and invalidate the rest of them. Deleting a snapshot copy of a file should check if a block belongs to another snapshot before sending it to the invalidates queue. It is not necessary to check all snapshots, only the previous and the next snapshots should be verified for blocks common with snapshot being deleted. Therefore removing a snapshot one should find a prefix of blocks of the current snapshot which are in common with either the previous or the next snapshot. The rest of the blocks can be invalidated. I would propose to copy the entire list of blocks to the snapshot copy. This will simplify the implementation. We can optimize this later by storing references only to the blocks that are different between the current state of the file and the snapshot.
          Hide
          jingzhao Jing Zhao added a comment -

          The proposed design looks pretty good to me. I agree we can copy the entire block list to file snapshot copy right now.

          Show
          jingzhao Jing Zhao added a comment - The proposed design looks pretty good to me. I agree we can copy the entire block list to file snapshot copy right now.
          Hide
          harip Hari Mankude added a comment -

          Is the proposal to copy the list of blocks to snapshot copy only when the file is truncated or is it when snapshot is taken irrespective of whether file is truncated or not?

          After the file is truncated and then appended again, will all subsequent snapshots of the file get a copy of block list?

          Show
          harip Hari Mankude added a comment - Is the proposal to copy the list of blocks to snapshot copy only when the file is truncated or is it when snapshot is taken irrespective of whether file is truncated or not? After the file is truncated and then appended again, will all subsequent snapshots of the file get a copy of block list?
          Hide
          shv Konstantin Shvachko added a comment -

          The proposal is to only copy block list when file is truncated. This allowes to avoid changes for append.
          If the file is truncated and then appended the subsequent snapshots will not need to have a copy of the block list. They will reference the original list of the file, until the file is truncated again.

          Show
          shv Konstantin Shvachko added a comment - The proposal is to only copy block list when file is truncated. This allowes to avoid changes for append. If the file is truncated and then appended the subsequent snapshots will not need to have a copy of the block list. They will reference the original list of the file, until the file is truncated again.
          Hide
          rguo Guo Ruijing added a comment -

          Attached HDFS Snapshot With Truncate Design for reference/review.

          Show
          rguo Guo Ruijing added a comment - Attached HDFS Snapshot With Truncate Design for reference/review.
          Hide
          shv Konstantin Shvachko added a comment -

          Hey Guo. Thanks for uploading the design doc. I have not had a chance to read it entirely. On a brief review it seems to be inline with the ideas laid out above.
          Let me give an update on the work we did with Plamen so far.

          • Replica copying logic is implemented
          • Block list is added to the diff (on truncatre only)
          • Snapshot reads are working
          • File delete logic is in place.

          The remaining items are adjusting snapshot deletes and writing lots of test cases.

          Guys, if there is other parallel effort going on could you please update the jira, so that we could coordinate / combine forces. If needed we can post a preliminary patch.

          Show
          shv Konstantin Shvachko added a comment - Hey Guo. Thanks for uploading the design doc. I have not had a chance to read it entirely. On a brief review it seems to be inline with the ideas laid out above. Let me give an update on the work we did with Plamen so far. Replica copying logic is implemented Block list is added to the diff (on truncatre only) Snapshot reads are working File delete logic is in place. The remaining items are adjusting snapshot deletes and writing lots of test cases. Guys, if there is other parallel effort going on could you please update the jira, so that we could coordinate / combine forces. If needed we can post a preliminary patch.
          Hide
          shv Konstantin Shvachko added a comment -

          Here is the patch for supporting snapshots and rollbacks with truncate. It goes on top of the latest patch in HDFS-3107.
          This is our joint work with Plamen. Would be good if somebody could review it.

          Show
          shv Konstantin Shvachko added a comment - Here is the patch for supporting snapshots and rollbacks with truncate. It goes on top of the latest patch in HDFS-3107 . This is our joint work with Plamen. Would be good if somebody could review it.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          The editLogs in HDFS-3107 can be used for this patch as well. They don't need to be regenerated for this because those editLogs performed a truncate on the block boundary.

          Show
          zero45 Plamen Jeliazkov added a comment - The editLogs in HDFS-3107 can be used for this patch as well. They don't need to be regenerated for this because those editLogs performed a truncate on the block boundary.
          Hide
          shv Konstantin Shvachko added a comment -

          Guo, wanted to comment on your design doc. Very comprehensive design, thanks. Feels like you have been working on the implementation.
          Our implementation is mostly in line with your design, except section 5.2.9. We make a copy of the origninal replica in tmpDir, then move it into finalized dir. That way incomplete replica in case of DN failure will be cleaned up automatically. We also implemented the optimized case, when the replica can be truncated in-place if there are no snapshots or upgrades.
          The deletion of a file and a snapshot are quite accurate.

          Show
          shv Konstantin Shvachko added a comment - Guo, wanted to comment on your design doc. Very comprehensive design, thanks. Feels like you have been working on the implementation. Our implementation is mostly in line with your design, except section 5.2.9. We make a copy of the origninal replica in tmpDir, then move it into finalized dir. That way incomplete replica in case of DN failure will be cleaned up automatically. We also implemented the optimized case, when the replica can be truncated in-place if there are no snapshots or upgrades. The deletion of a file and a snapshot are quite accurate.
          Hide
          huLiu Hu Liu, added a comment -

          Hi Konstantin Shvachko,

          Thanks for your comments. It's glad to see your patch.
          Actually Guo and I have finished the POC for this several months ago. But we couldn't open source it due to legal issue of company.

          Show
          huLiu Hu Liu, added a comment - Hi Konstantin Shvachko , Thanks for your comments. It's glad to see your patch. Actually Guo and I have finished the POC for this several months ago. But we couldn't open source it due to legal issue of company.
          Hide
          jingzhao Jing Zhao added a comment -

          Thanks for working on this, Konstantin Shvachko and Plamen Jeliazkov. So far I just went through the namenode snapshot part (INode, INodeFile, FileDiff, and FileDiffList) and I will continue reviewing the remaining.

          1. Looks like findLaterSnapshotWithBlocks and findEarlierSnapshotWithBlocks are always coupled with FileDiff#getBlocks. Maybe we can combine them so that we can wrap the logic like the following code into two methods like findBlocksAfter and findBlocksBefore?
            +    FileDiff diff = getDiffs().getDiffById(snapshot);
            +    BlockInfo[] snapshotBlocks = diff == null ? getBlocks() : diff.getBlocks();
            +    if(snapshotBlocks != null)
            +      return snapshotBlocks;
            +    // Blocks are not in the current snapshot
            +    // Find next snapshot with blocks present or return current file blocks
            +    diff = getDiffs().findLaterSnapshotWithBlocks(diff.getSnapshotId());
            +    snapshotBlocks = (diff == null) ? getBlocks() : diff.getBlocks();
            
          2. Since the same block can be included in different file diffs, we may have duplicated blocks in the collectedBlocks. Will this lead to duplicated records in invalid block list?
              public void destroyAndCollectSnapshotBlocks(
                  BlocksMapUpdateInfo collectedBlocks) {
                for(FileDiff d : asList())
                  d.destroyAndCollectSnapshotBlocks(collectedBlocks);
              }
            
          3. INodeFile#destroyAndCollectBlocks destroys the whole file, including the file diffs for snapshots. Thus we do not need to call collectBlocksAndClear and define a new destroyAndCollectAllBlocks method. Instead, we can simply first destroy all the blocks belonging to the current file, then check if calling sf.getDiffs().destroyAndCollectSnapshotBlocks is necessary.
            +    FileWithSnapshotFeature sf = getFileWithSnapshotFeature();
            +    if(sf == null || getDiffs().asList().isEmpty()) {
            +      destroyAndCollectAllBlocks(collectedBlocks, removedINodes);
            +      return;
            +    }
            +    sf.getDiffs().destroyAndCollectSnapshotBlocks(collectedBlocks);
            
          4. How do we currently calculate/update quota for a file? I guess we need to update the quota calculation algorithm for an INodeFile here.
          5. I guess the semantic of findEarlierSnapshotWithBlocks is to find the FileDiff that satisfies: 1) its block list is not null, and 2) its snapshot id is less than the given snapshotId. However, if the given snapshotId is not CURRENT_STATE_ID, the current implementation may return a FileDiff whose snapshot id is >= the given snapshotId (since getDiffById may return a diff with snapshot id greater than the given id).
              public FileDiff findEarlierSnapshotWithBlocks(int snapshotId) {
                FileDiff diff = (snapshotId == Snapshot.CURRENT_STATE_ID) ?
                    getLast() : getDiffById(snapshotId);
                BlockInfo[] snapshotBlocks = null;
                while(diff != null) {
                  snapshotBlocks = diff.getBlocks();
                  if(snapshotBlocks != null)
                    break;
                  int p = getPrior(diff.getSnapshotId(), true);
                  diff = (p == Snapshot.NO_SNAPSHOT_ID) ? null : getDiffById(p);
                }
                return diff;
              }
            
          6. Still for findEarlierSnapshotWithBlocks, because getPrior currently is a log(n) operation, the worst time complexity thus can be nlog(n). Considering the list of the snapshot diff list is usually not big (we have an upper limit for the total number of snapshots), we may consider directly doing a linear scan for the file diff list.
          7. In INode.java, why do we need the following change?
               public final boolean isInLatestSnapshot(final int latestSnapshotId) {
            -    if (latestSnapshotId == Snapshot.CURRENT_STATE_ID) {
            +    if (latestSnapshotId == Snapshot.CURRENT_STATE_ID ||
            +        latestSnapshotId == Snapshot.NO_SNAPSHOT_ID) {
            
          8. Nit: need to add "{" and "}" for the while loop according to our current coding style. Similar for several other places (e.g., FileDiffList#destroyAndCollectSnapshotBlocks).
            +    while(i < currentBlocks.length && i < snapshotBlocks.length &&
            +          currentBlocks[i] == snapshotBlocks[i])
            +      i++;
            +    // Collect the remaining blocks of the file
            +    while(i < currentBlocks.length)
            +      collectedBlocks.addDeleteBlock(currentBlocks[i++]);
            
          9. Minor: In the following code, instead of calling getDiffById to search for the file diff, we can let AbstractINodeDiffList#saveSelf2Snapshot return the diff it just finds/creates.
              public void saveSelf2Snapshot(int latestSnapshotId, INodeFile iNodeFile,
                  INodeFileAttributes snapshotCopy, boolean withBlocks)
                      throws QuotaExceededException {
                super.saveSelf2Snapshot(latestSnapshotId, iNodeFile, snapshotCopy);
                if(! withBlocks) return;
                final FileDiff diff = getDiffById(latestSnapshotId);
                // Store blocks if this is the first update
                diff.setBlocks(iNodeFile.getBlocks());
              }
            
          10. Nit: also, for the above code, maybe it's more direct to have
            if (withBlocks) {
              final FileDiff diff = getDiffById(latestSnapshotId);
              // Store blocks if this is the first update
              diff.setBlocks(iNodeFile.getBlocks());
            }
            
          Show
          jingzhao Jing Zhao added a comment - Thanks for working on this, Konstantin Shvachko and Plamen Jeliazkov . So far I just went through the namenode snapshot part (INode, INodeFile, FileDiff, and FileDiffList) and I will continue reviewing the remaining. Looks like findLaterSnapshotWithBlocks and findEarlierSnapshotWithBlocks are always coupled with FileDiff#getBlocks . Maybe we can combine them so that we can wrap the logic like the following code into two methods like findBlocksAfter and findBlocksBefore? + FileDiff diff = getDiffs().getDiffById(snapshot); + BlockInfo[] snapshotBlocks = diff == null ? getBlocks() : diff.getBlocks(); + if (snapshotBlocks != null ) + return snapshotBlocks; + // Blocks are not in the current snapshot + // Find next snapshot with blocks present or return current file blocks + diff = getDiffs().findLaterSnapshotWithBlocks(diff.getSnapshotId()); + snapshotBlocks = (diff == null ) ? getBlocks() : diff.getBlocks(); Since the same block can be included in different file diffs, we may have duplicated blocks in the collectedBlocks . Will this lead to duplicated records in invalid block list? public void destroyAndCollectSnapshotBlocks( BlocksMapUpdateInfo collectedBlocks) { for (FileDiff d : asList()) d.destroyAndCollectSnapshotBlocks(collectedBlocks); } INodeFile#destroyAndCollectBlocks destroys the whole file, including the file diffs for snapshots. Thus we do not need to call collectBlocksAndClear and define a new destroyAndCollectAllBlocks method. Instead, we can simply first destroy all the blocks belonging to the current file, then check if calling sf.getDiffs().destroyAndCollectSnapshotBlocks is necessary. + FileWithSnapshotFeature sf = getFileWithSnapshotFeature(); + if (sf == null || getDiffs().asList().isEmpty()) { + destroyAndCollectAllBlocks(collectedBlocks, removedINodes); + return ; + } + sf.getDiffs().destroyAndCollectSnapshotBlocks(collectedBlocks); How do we currently calculate/update quota for a file? I guess we need to update the quota calculation algorithm for an INodeFile here. I guess the semantic of findEarlierSnapshotWithBlocks is to find the FileDiff that satisfies: 1) its block list is not null, and 2) its snapshot id is less than the given snapshotId . However, if the given snapshotId is not CURRENT_STATE_ID , the current implementation may return a FileDiff whose snapshot id is >= the given snapshotId (since getDiffById may return a diff with snapshot id greater than the given id). public FileDiff findEarlierSnapshotWithBlocks( int snapshotId) { FileDiff diff = (snapshotId == Snapshot.CURRENT_STATE_ID) ? getLast() : getDiffById(snapshotId); BlockInfo[] snapshotBlocks = null ; while (diff != null ) { snapshotBlocks = diff.getBlocks(); if (snapshotBlocks != null ) break ; int p = getPrior(diff.getSnapshotId(), true ); diff = (p == Snapshot.NO_SNAPSHOT_ID) ? null : getDiffById(p); } return diff; } Still for findEarlierSnapshotWithBlocks, because getPrior currently is a log(n) operation, the worst time complexity thus can be nlog(n) . Considering the list of the snapshot diff list is usually not big (we have an upper limit for the total number of snapshots), we may consider directly doing a linear scan for the file diff list. In INode.java, why do we need the following change? public final boolean isInLatestSnapshot( final int latestSnapshotId) { - if (latestSnapshotId == Snapshot.CURRENT_STATE_ID) { + if (latestSnapshotId == Snapshot.CURRENT_STATE_ID || + latestSnapshotId == Snapshot.NO_SNAPSHOT_ID) { Nit: need to add "{" and "}" for the while loop according to our current coding style. Similar for several other places (e.g., FileDiffList#destroyAndCollectSnapshotBlocks ). + while (i < currentBlocks.length && i < snapshotBlocks.length && + currentBlocks[i] == snapshotBlocks[i]) + i++; + // Collect the remaining blocks of the file + while (i < currentBlocks.length) + collectedBlocks.addDeleteBlock(currentBlocks[i++]); Minor: In the following code, instead of calling getDiffById to search for the file diff, we can let AbstractINodeDiffList#saveSelf2Snapshot return the diff it just finds/creates. public void saveSelf2Snapshot( int latestSnapshotId, INodeFile iNodeFile, INodeFileAttributes snapshotCopy, boolean withBlocks) throws QuotaExceededException { super .saveSelf2Snapshot(latestSnapshotId, iNodeFile, snapshotCopy); if (! withBlocks) return ; final FileDiff diff = getDiffById(latestSnapshotId); // Store blocks if this is the first update diff.setBlocks(iNodeFile.getBlocks()); } Nit: also, for the above code, maybe it's more direct to have if (withBlocks) { final FileDiff diff = getDiffById(latestSnapshotId); // Store blocks if this is the first update diff.setBlocks(iNodeFile.getBlocks()); }
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching combined patch here as well.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching combined patch here as well.
          Hide
          shv Konstantin Shvachko added a comment -

          > Actually Guo and I have finished the POC for this several months ago. But we couldn't open source it

          Hi Hu.It shows indeed in the design. Too bad you couldn't open source yours.
          Hope ours is similar. I know at least getBlocks(snapshotId) method is in common
          Looking at Jing's comments.

          Show
          shv Konstantin Shvachko added a comment - > Actually Guo and I have finished the POC for this several months ago. But we couldn't open source it Hi Hu.It shows indeed in the design. Too bad you couldn't open source yours. Hope ours is similar. I know at least getBlocks(snapshotId) method is in common Looking at Jing's comments.
          Hide
          shv Konstantin Shvachko added a comment -

          (1) Done.
          (2) No duplicate blocks in InvalidateBlocks, because the map replaces old values with the new ones when the key is the same.
          We can optimize here by making BlocksMapUpdateInfo.toDeleteList a map rather than list. I didn't see it critical, but we can open an issue for that.
          (3) Good catch. Like code reduction. This came from an attempt to make destroyAndCollectBlocks() "smart" about deleting current file blocks or a snapshot. Not any more.
          (4) We want to count only bytes that actually take space on DataNodes.
          Without truncate we can count current blocks only and ignore all snapshot blocks.
          With truncate we should

          • subtract removed bytes in case of in-place truncate, which we do.
          • add bytes of the newly created block when we copy on truncate. I think we missed this case. Plamen says he is checking.

          (5) We call findEarlierSnapshotWithBlocks() for existing snapshots. So getDiffById() should find the exact match. It can return larger id only if the given snapshotId does not exist.
          I added an assert and never hit it. Another way is to generalize findEarlierSnapshotWithBlocks() so that it always dealt with snapshots earlier than snapshotId. LMK if it worth generalizing.
          (6) getPrior currently is a log( n ). Indeed, where n is the number of snapshots. And it's all in memory, and it is only when you truncate. I am saying I can live with that. But if you have a way to iterate snapshots in their historical order it would be even better.
          (7) Plamen says this is because Snapshot.findLatestSnapshot() may return NO_SNAPSHOT_ID, which breaks recordModification() if you don't have that additional check. We see it when commitBlockSynchronization() is called for truncated block.
          (8, 9, 10) Sure.

          Attaching patch with everything fixed as commented, but the quotas (4).

          Show
          shv Konstantin Shvachko added a comment - (1) Done. (2) No duplicate blocks in InvalidateBlocks, because the map replaces old values with the new ones when the key is the same. We can optimize here by making BlocksMapUpdateInfo.toDeleteList a map rather than list. I didn't see it critical, but we can open an issue for that. (3) Good catch. Like code reduction. This came from an attempt to make destroyAndCollectBlocks() "smart" about deleting current file blocks or a snapshot. Not any more. (4) We want to count only bytes that actually take space on DataNodes. Without truncate we can count current blocks only and ignore all snapshot blocks. With truncate we should subtract removed bytes in case of in-place truncate, which we do. add bytes of the newly created block when we copy on truncate. I think we missed this case. Plamen says he is checking. (5) We call findEarlierSnapshotWithBlocks() for existing snapshots. So getDiffById() should find the exact match. It can return larger id only if the given snapshotId does not exist. I added an assert and never hit it. Another way is to generalize findEarlierSnapshotWithBlocks() so that it always dealt with snapshots earlier than snapshotId. LMK if it worth generalizing. (6) getPrior currently is a log( n ). Indeed, where n is the number of snapshots. And it's all in memory, and it is only when you truncate. I am saying I can live with that. But if you have a way to iterate snapshots in their historical order it would be even better. (7) Plamen says this is because Snapshot.findLatestSnapshot() may return NO_SNAPSHOT_ID , which breaks recordModification() if you don't have that additional check. We see it when commitBlockSynchronization() is called for truncated block. (8, 9, 10) Sure. Attaching patch with everything fixed as commented, but the quotas (4).
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Regarding (4), I see two issues right now.

          INodeFile.diskspaceConsumed() has always done its calculation based on the current INodeFile blocks list. Prior to truncate, it was assumed the file only grew, so diskspaceConsumed was always correct based on the current INodeFile blocks list. With truncate now, especially with copy-on-truncate and certain blocks living only in Snapshot diffs, diskspaceConsumed becomes a question.

          Is INodeFile.diskspaceConsumed() meant to only return the disk space consumed of just the current/latest INodeFile (as it currently does)? Or should it go through its diffs and return the disk space consumed of all blocks that belong to this BlockCollection?

          I could not find any documentation for diskspaceConsumed() method.

          Based on the code I see in INodeFile.computeContentSummary4Snapshot(), it seems Snapshot diffs are only calculated towards disk space when the latest INodeFile has been deleted. The disk space consumed calculation falls back to the last diff that was known about.

          Show
          zero45 Plamen Jeliazkov added a comment - Regarding (4), I see two issues right now. INodeFile.diskspaceConsumed() has always done its calculation based on the current INodeFile blocks list. Prior to truncate, it was assumed the file only grew, so diskspaceConsumed was always correct based on the current INodeFile blocks list. With truncate now, especially with copy-on-truncate and certain blocks living only in Snapshot diffs, diskspaceConsumed becomes a question. Is INodeFile.diskspaceConsumed() meant to only return the disk space consumed of just the current/latest INodeFile (as it currently does)? Or should it go through its diffs and return the disk space consumed of all blocks that belong to this BlockCollection? I could not find any documentation for diskspaceConsumed() method. Based on the code I see in INodeFile.computeContentSummary4Snapshot(), it seems Snapshot diffs are only calculated towards disk space when the latest INodeFile has been deleted. The disk space consumed calculation falls back to the last diff that was known about.
          Hide
          shv Konstantin Shvachko added a comment -

          This patch implements diskspaceConsumed() and computeContentSummary() so that it takes into account blocks in the file and in the snapshot.
          As Plamen stated with truncate diskspaceConsumed() cannot just compute bytes of the current file. The new implementation actually counts all bytes that are stored on DataNodes as blocks of a file or of its snapshot. The number of bytes is multiplied by the replication factor as in current code.
          Note that if truncates are not used the space consumed will be the same as today.

          Show
          shv Konstantin Shvachko added a comment - This patch implements diskspaceConsumed() and computeContentSummary() so that it takes into account blocks in the file and in the snapshot. As Plamen stated with truncate diskspaceConsumed() cannot just compute bytes of the current file. The new implementation actually counts all bytes that are stored on DataNodes as blocks of a file or of its snapshot. The number of bytes is multiplied by the replication factor as in current code. Note that if truncates are not used the space consumed will be the same as today.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching refreshed combined patch of the latest HDFS-3107 and latest HDFS-7056 work.

          The notable changes between the 2 versions of the combined patches is the Quota / diskSpaceConsumed() improvements and the fixes to comments raised in HDFS-7056.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching refreshed combined patch of the latest HDFS-3107 and latest HDFS-7056 work. The notable changes between the 2 versions of the combined patches is the Quota / diskSpaceConsumed() improvements and the fixes to comments raised in HDFS-7056 .
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12680652/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision eace218.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.namenode.TestCommitBlockSynchronization
          org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
          org.apache.hadoop.hdfs.TestFileCreation

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/8706//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8706//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12680652/HDFS-3107-HDFS-7056-combined.patch against trunk revision eace218. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 9 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.TestCommitBlockSynchronization org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer org.apache.hadoop.hdfs.TestFileCreation +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/8706//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8706//console This message is automatically generated.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          I just looked into this. I'll have a new 7056 patch shortly.

          1. The TestOfflineEditsViewer.testStored failure was known already from HDFS-3107.
          2. The TestFileCreation.testDeleteOnExit was un-reproducible for me on my local machine; the cause of this failure (NoClassDefFoundError) is unclear.
          3. The two TestCommitBlockSynchronization failures were due to a slight change in the retry detection logic of commitBlockSynchronization – it revealed a flaw in the test as the mocked INodeFile (in both cases) had null for it's last block even though the test assumed the INodeFile had a completed last block.
          Show
          zero45 Plamen Jeliazkov added a comment - I just looked into this. I'll have a new 7056 patch shortly. The TestOfflineEditsViewer.testStored failure was known already from HDFS-3107 . The TestFileCreation.testDeleteOnExit was un-reproducible for me on my local machine; the cause of this failure (NoClassDefFoundError) is unclear. The two TestCommitBlockSynchronization failures were due to a slight change in the retry detection logic of commitBlockSynchronization – it revealed a flaw in the test as the mocked INodeFile (in both cases) had null for it's last block even though the test assumed the INodeFile had a completed last block.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching new HDFS-7056 patch with TestCommitBlockSynchronization tests fixed.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching new HDFS-7056 patch with TestCommitBlockSynchronization tests fixed.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching refreshed combined patch of latest HDFS-3107 and HDFS-7056.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching refreshed combined patch of latest HDFS-3107 and HDFS-7056 .
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12680984/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision 46f6f9d.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8721//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12680984/HDFS-3107-HDFS-7056-combined.patch against trunk revision 46f6f9d. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8721//console This message is automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12680984/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision 46f6f9d.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8722//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12680984/HDFS-3107-HDFS-7056-combined.patch against trunk revision 46f6f9d. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8722//console This message is automatically generated.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Trunk had moved on since I generated my patch. Refreshed both combined and regular patch.

          Made some additional changes with help from Konstantin:

          1. Patch failed with compilation error due to HDFS-7381. Updated patch to account for new BlockIdManager in trunk.
          2. Renamed parameter of commitBlockSynchronization(), from "lastblock" to "oldBlock". We did this change because commitBlockSynchronization() handles both regular recovery and copy-on-write truncate now.
          3. Removed a call in commitBlockSynchronization() to getStoredBlock() because we could get it from iFile.getLastBlock().
          4. Fixed TestCommitBlockSynchronization again by making mocked INodeFile return BlockInfoUC when calling getLastBlock() on it.
          Show
          zero45 Plamen Jeliazkov added a comment - Trunk had moved on since I generated my patch. Refreshed both combined and regular patch. Made some additional changes with help from Konstantin: Patch failed with compilation error due to HDFS-7381 . Updated patch to account for new BlockIdManager in trunk. Renamed parameter of commitBlockSynchronization(), from "lastblock" to "oldBlock". We did this change because commitBlockSynchronization() handles both regular recovery and copy-on-write truncate now. Removed a call in commitBlockSynchronization() to getStoredBlock() because we could get it from iFile.getLastBlock(). Fixed TestCommitBlockSynchronization again by making mocked INodeFile return BlockInfoUC when calling getLastBlock() on it.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12681191/HDFS-7056.patch
          against trunk revision b0a9cd3.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8726//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12681191/HDFS-7056.patch against trunk revision b0a9cd3. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8726//console This message is automatically generated.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Seems Jenkins grabbed the HDFS-7056 patch. I will re-attach the combined patch so bot can run.

          Show
          zero45 Plamen Jeliazkov added a comment - Seems Jenkins grabbed the HDFS-7056 patch. I will re-attach the combined patch so bot can run.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12681253/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision e073b61.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.TestDFSUpgradeFromImage
          org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/8728//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8728//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12681253/HDFS-3107-HDFS-7056-combined.patch against trunk revision e073b61. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.TestDFSUpgradeFromImage org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/8728//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8728//console This message is automatically generated.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          TestDFSUpgradeFromImage.testUpgradeFromCorruptRel22Image failure seems unrelated to this work.
          I was able to reproduce the test failure locally but the test never performs any truncate calls.
          It fails a Preconditions check when BlockIdManager (via FSNamesystem) performs clear() – again, seems like something from HDFS-7381.

          Show
          zero45 Plamen Jeliazkov added a comment - TestDFSUpgradeFromImage.testUpgradeFromCorruptRel22Image failure seems unrelated to this work. I was able to reproduce the test failure locally but the test never performs any truncate calls. It fails a Preconditions check when BlockIdManager (via FSNamesystem) performs clear() – again, seems like something from HDFS-7381 .
          Hide
          cmccabe Colin P. McCabe added a comment -

          The design doc makes sense to me-- thanks for spelling it all out. I'm afraid I can't see some of the pictures because I don't have MS Windows or MS Office. Maybe you can attach a PDF at some point?

          Did you consider a design where the inode ID of the truncated file changes? That might simplify some things... In that case, we would be treating truncated files a lot like files that been deleted and then re-created with the same name. On the other hand, perhaps the current design can achieve higher memory efficiency?

          635   * @return true if and client does not need to wait for block recovery,
          636	   * false if client needs to wait for block recovery.
          

          There is an extra "if and" in this comment.

          +  /**
          +   *  Check if this block has the same id, genStamp, and numBytes as the other.
          +   */
          +  public boolean sameAs(Block other) {
          +    return matchingIdAndGenStamp(this, other) &&
          +        getNumBytes() == other.getNumBytes();
          +  }
          

          I'd prefer not to add this method, since it's only used once in a unit test. And we already have Block#equals and Block#compareTo.

          I can see we're creating a lease while the new last block of the file is created (or the current last block is truncated.) How does this interact with recoverLease? If the client calls recoverLease on this file that is being truncated, will the system still be in a reasonable state?

          Show
          cmccabe Colin P. McCabe added a comment - The design doc makes sense to me-- thanks for spelling it all out. I'm afraid I can't see some of the pictures because I don't have MS Windows or MS Office. Maybe you can attach a PDF at some point? Did you consider a design where the inode ID of the truncated file changes? That might simplify some things... In that case, we would be treating truncated files a lot like files that been deleted and then re-created with the same name. On the other hand, perhaps the current design can achieve higher memory efficiency? 635 * @ return true if and client does not need to wait for block recovery, 636 * false if client needs to wait for block recovery. There is an extra "if and" in this comment. + /** + * Check if this block has the same id, genStamp, and numBytes as the other. + */ + public boolean sameAs(Block other) { + return matchingIdAndGenStamp( this , other) && + getNumBytes() == other.getNumBytes(); + } I'd prefer not to add this method, since it's only used once in a unit test. And we already have Block#equals and Block#compareTo . I can see we're creating a lease while the new last block of the file is created (or the current last block is truncated.) How does this interact with recoverLease ? If the client calls recoverLease on this file that is being truncated, will the system still be in a reasonable state?
          Hide
          shv Konstantin Shvachko added a comment -
          • TestDFSUpgradeFromImage.testUpgradeFromCorruptRel22Image() failure is reported in HDFS-7393.
          • TestOfflineEditsViewer.testStored() is expected to fail without binary and xml editsStored files, which are provided separately from the patch.
          Show
          shv Konstantin Shvachko added a comment - TestDFSUpgradeFromImage.testUpgradeFromCorruptRel22Image() failure is reported in HDFS-7393 . TestOfflineEditsViewer.testStored() is expected to fail without binary and xml editsStored files, which are provided separately from the patch.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching refreshed HDFS-7056 and combined HDFS-3107 & HDFS-7056 patch to address Colin P. McCabe's comments.

          As for interaction with recoverLease the basic answer is it will behave the same as create and append (since the file will no longer be under construction after truncate completes).

          Otherwise, since the last block of the file is under recovery and the file is under construction, the recoverLease operation will renew the lease and trigger block recovery again, just like it does with a file that has not completed regular block recovery.

          I believe the unit tests even show this – I encourage you to look through them.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching refreshed HDFS-7056 and combined HDFS-3107 & HDFS-7056 patch to address Colin P. McCabe 's comments. As for interaction with recoverLease the basic answer is it will behave the same as create and append (since the file will no longer be under construction after truncate completes). Otherwise, since the last block of the file is under recovery and the file is under construction, the recoverLease operation will renew the lease and trigger block recovery again, just like it does with a file that has not completed regular block recovery. I believe the unit tests even show this – I encourage you to look through them.
          Hide
          shv Konstantin Shvachko added a comment -

          Colin thank you for the review.

          I don't have MS Windows or MS Office. Maybe you can attach a PDF at some point?

          Guo is the owner of the document.
          I use LibreOffice, which shows me the .docx file including all pictures. No Windows.
          On my side, I updated the design doc for truncate to reflect snapshots and attached the pdf file under HDFS-3107.

          Did you consider a design where the inode ID of the truncated file changes?

          I see what you mean. But that would result in creating a new inode for each truncate op under a snapshot. You are right it will use more RAM.

          I'd prefer not to add this method, since it's only used once in a unit test. And we already have Block#equals and Block#compareTo.

          sameAs() is indeed used only once, but in FSDirectory, not a unit test. It could have been used in a few more places, where we check that two blocks have the same ids, genStamps and lengths (that is are the same). We do have equals(), compareTo(), and matchingIdAndGenStamp(). But they are different from sameAs() as the first two compare only blockIds, and the third adds genStamps.
          Not that it is imperative for the truncate feature, just explaining how it could have been useful.

          If the client calls recoverLease on this file that is being truncated

          Agree with Plamen. This is no different from create or append. That is, if the lease has not expired yet, but recoverLease() is called

          • by a different client, then the operation will fail (with a rather awkward exception).
          • by the same client, then the truncate recovery will be re-triggered.

          If the lease expired by the time recoverLease() is called, then

          • it is a no op, when file is already closed,
          • and it will re-trigger the recovery when the file is still under construction.
          Show
          shv Konstantin Shvachko added a comment - Colin thank you for the review. I don't have MS Windows or MS Office. Maybe you can attach a PDF at some point ? Guo is the owner of the document. I use LibreOffice, which shows me the .docx file including all pictures. No Windows. On my side, I updated the design doc for truncate to reflect snapshots and attached the pdf file under HDFS-3107 . Did you consider a design where the inode ID of the truncated file changes ? I see what you mean. But that would result in creating a new inode for each truncate op under a snapshot. You are right it will use more RAM. I'd prefer not to add this method, since it's only used once in a unit test. And we already have Block#equals and Block#compareTo. sameAs() is indeed used only once, but in FSDirectory, not a unit test. It could have been used in a few more places, where we check that two blocks have the same ids, genStamps and lengths (that is are the same). We do have equals(), compareTo(), and matchingIdAndGenStamp(). But they are different from sameAs() as the first two compare only blockIds, and the third adds genStamps. Not that it is imperative for the truncate feature, just explaining how it could have been useful. If the client calls recoverLease on this file that is being truncated Agree with Plamen. This is no different from create or append. That is, if the lease has not expired yet, but recoverLease() is called by a different client, then the operation will fail (with a rather awkward exception). by the same client, then the truncate recovery will be re-triggered. If the lease expired by the time recoverLease() is called, then it is a no op, when file is already closed, and it will re-trigger the recovery when the file is still under construction.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12681500/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision d005404.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/8739//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/8739//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8739//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12681500/HDFS-3107-HDFS-7056-combined.patch against trunk revision d005404. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/8739//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/8739//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/8739//console This message is automatically generated.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          FindBugs appears to be unrelated.

          New FindBugs points to inconsistent synchronization in org.apache.hadoop.hdfs.DFSOutputStream.
          A class we don't touch in this work.

          Show
          zero45 Plamen Jeliazkov added a comment - FindBugs appears to be unrelated. New FindBugs points to inconsistent synchronization in org.apache.hadoop.hdfs.DFSOutputStream. A class we don't touch in this work.
          Hide
          Byron Wong Byron Wong added a comment -

          There's a bug in INodeFile.computeContentSummary.
          Given a file that has a snapshot, the computeContentSummary will compute file length as 0 if either
          a) the file has no diffs (state of file was unchanged since creation of the snapshot)
          b) the file is not currently deleted

          Show
          Byron Wong Byron Wong added a comment - There's a bug in INodeFile.computeContentSummary . Given a file that has a snapshot, the computeContentSummary will compute file length as 0 if either a) the file has no diffs (state of file was unchanged since creation of the snapshot) b) the file is not currently deleted
          Hide
          shv Konstantin Shvachko added a comment -

          Updating patch to current trunk.
          Byron Wong was testing extensively snapshots with truncate during last few week. He found too corner cases:

          1. file size was not computed in ContentSummary for a file that has snapshots and is not deleted
          2. Disk space computation was incorrect when snapshots are deleted in a certain sequence after mutliple truncates.

          Both are fixed in the latest patch, and we added a series of test cases to capture this.

          Its been a while. Jing and Colin had good comments, which have been addressed by now.
          Could you guys please review so that we could finally commit this. It really takes cycles to keep up with evolving trunk.

          Show
          shv Konstantin Shvachko added a comment - Updating patch to current trunk. Byron Wong was testing extensively snapshots with truncate during last few week. He found too corner cases: file size was not computed in ContentSummary for a file that has snapshots and is not deleted Disk space computation was incorrect when snapshots are deleted in a certain sequence after mutliple truncates. Both are fixed in the latest patch, and we added a series of test cases to capture this. Its been a while. Jing and Colin had good comments, which have been addressed by now. Could you guys please review so that we could finally commit this. It really takes cycles to keep up with evolving trunk.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching new combined patch based on Konstantin's latest HDFS-3107 and HDFS-7056 patch.

          The editsStored files from HDFS-3107 JIRA continue to work.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching new combined patch based on Konstantin's latest HDFS-3107 and HDFS-7056 patch. The editsStored files from HDFS-3107 JIRA continue to work.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12686918/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision 3681de2.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          -1 javac. The applied patch generated 1221 javac compiler warnings (more than the trunk's current 0 warnings).

          -1 javadoc. The javadoc tool appears to have generated 49 warning messages.
          See https://builds.apache.org/job/PreCommit-HDFS-Build/9026//artifact/patchprocess/diffJavadocWarnings.txt for details.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.blockmanagement.TestDatanodeManager
          org.apache.hadoop.hdfs.server.namenode.TestCacheDirectives
          org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
          org.apache.hadoop.hdfs.TestDistributedFileSystem

          The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9026//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9026//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9026//artifact/patchprocess/diffJavacWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9026//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12686918/HDFS-3107-HDFS-7056-combined.patch against trunk revision 3681de2. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. -1 javac . The applied patch generated 1221 javac compiler warnings (more than the trunk's current 0 warnings). -1 javadoc . The javadoc tool appears to have generated 49 warning messages. See https://builds.apache.org/job/PreCommit-HDFS-Build/9026//artifact/patchprocess/diffJavadocWarnings.txt for details. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.blockmanagement.TestDatanodeManager org.apache.hadoop.hdfs.server.namenode.TestCacheDirectives org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer org.apache.hadoop.hdfs.TestDistributedFileSystem The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9026//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9026//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9026//artifact/patchprocess/diffJavacWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9026//console This message is automatically generated.
          Hide
          Byron Wong Byron Wong added a comment -

          I've been doing some extensive testing on the patches, many of which have already been mentioned by Konstantin Shvachko.
          Here are a couple of test cases I've tried:
          1) Deleting snapshots directly after multiple truncates.
          2) Deleting snapshots directly after an append that happened after multiple truncates.
          3) Verifying that the result of computeContentSummary is as expected after truncating and renaming (i.e. create snapshot, rename, truncate, computeContentSummary).
          4) Verify that the correct blocks are being preserved and the correct ones are being deleted after deleting a file that has been truncated, appended, and snapshotted.

          Everything works as expected as of the latest patches, so +1. Good work, guys.

          Show
          Byron Wong Byron Wong added a comment - I've been doing some extensive testing on the patches, many of which have already been mentioned by Konstantin Shvachko . Here are a couple of test cases I've tried: 1) Deleting snapshots directly after multiple truncates. 2) Deleting snapshots directly after an append that happened after multiple truncates. 3) Verifying that the result of computeContentSummary is as expected after truncating and renaming (i.e. create snapshot, rename, truncate, computeContentSummary). 4) Verify that the correct blocks are being preserved and the correct ones are being deleted after deleting a file that has been truncated, appended, and snapshotted. Everything works as expected as of the latest patches, so +1. Good work, guys.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          I just checked all the failures reported by Jenkins BOT.

          1. FindBugs is unrelated – we don't modify getBlockLocations. Issue comes from HDFS-7463.
          2. JavaDoc warnings are from stuff we've never touched.
          3. Java warnings are from files we've never touched.
          4. All the "failed / timed out" tests passed on my local machine.
            (Except TestOfflineEditsViewer, which passes once you have the correct edits files loaded)
          Show
          zero45 Plamen Jeliazkov added a comment - I just checked all the failures reported by Jenkins BOT. FindBugs is unrelated – we don't modify getBlockLocations. Issue comes from HDFS-7463 . JavaDoc warnings are from stuff we've never touched. Java warnings are from files we've never touched. All the "failed / timed out" tests passed on my local machine. (Except TestOfflineEditsViewer, which passes once you have the correct edits files loaded)
          Hide
          shv Konstantin Shvachko added a comment -

          Updating the patches once again. This is the snapshot part of truncate.
          The update is mostly related to Jing Zhao's refactoring of HDFS-7509.
          There are no changes to the truncate logic itself.

          Show
          shv Konstantin Shvachko added a comment - Updating the patches once again. This is the snapshot part of truncate. The update is mostly related to Jing Zhao 's refactoring of HDFS-7509 . There are no changes to the truncate logic itself.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching combined patch.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching combined patch.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12688105/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision ef1fc51.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9077//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12688105/HDFS-3107-HDFS-7056-combined.patch against trunk revision ef1fc51. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9077//console This message is automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12688105/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision ef1fc51.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9078//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12688105/HDFS-3107-HDFS-7056-combined.patch against trunk revision ef1fc51. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9078//console This message is automatically generated.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching newly combined patch.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching newly combined patch.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12688173/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision 5df7ecb.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9082//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12688173/HDFS-3107-HDFS-7056-combined.patch against trunk revision 5df7ecb. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9082//console This message is automatically generated.
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching "git diff" version of patch instead of IntelliJ created patch.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching "git diff" version of patch instead of IntelliJ created patch.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12688200/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision efe6357.

          -1 patch. Trunk compilation may be broken.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9086//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12688200/HDFS-3107-HDFS-7056-combined.patch against trunk revision efe6357. -1 patch . Trunk compilation may be broken. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9086//console This message is automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12688184/HDFS-3107-HDFS-7056-combined.patch
          against trunk revision 0402bad.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureToleration
          org.apache.hadoop.hdfs.server.blockmanagement.TestDatanodeManager
          org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
          org.apache.hadoop.fs.TestSymlinkHdfsFileContext

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9084//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9084//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9084//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12688184/HDFS-3107-HDFS-7056-combined.patch against trunk revision 0402bad. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureToleration org.apache.hadoop.hdfs.server.blockmanagement.TestDatanodeManager org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer org.apache.hadoop.fs.TestSymlinkHdfsFileContext Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9084//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9084//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9084//console This message is automatically generated.
          Hide
          cmccabe Colin P. McCabe added a comment -
            public boolean truncate(String src, long newLength, String clientName)
          

          How about calling the first argument "target" rather than "src"? "src" and "dst" make sense for rename, not so much for truncate since there is no "destination"

          I would still like to see these functions return an enum like:

          enum TruncateResult { FINISHED, IN_PROGRESS }
          

          the problem with returning a boolean is that people will assume that if it returns false, the operation failed. This is how File#delete works, how File#mkdir works, how pretty much everything in the Java standard library works. We have some stuff like this in Hadoop's FileSystem.java as well. So it will be confusing to users if our API returns a boolean that means something different. But by returning an enum, we make it clear what is going on. Of course, we can still use booleans in the protobuf if that's more convenient. This is just a user API thing.

              * @param bc file
          +   * @param lastBlockDelta num of bytes to remove from block
              * @return the last block locations if the block is partial or null otherwise
              */
             public LocatedBlock convertLastBlockToUnderConstruction(
          -      BlockCollection bc) throws IOException {
          +      BlockCollection bc, long lastBlockDelta) throws IOException {
               BlockInfo oldBlock = bc.getLastBlock();
               if(oldBlock == null ||
          -        bc.getPreferredBlockSize() == oldBlock.getNumBytes())
          +       bc.getPreferredBlockSize() == oldBlock.getNumBytes() - lastBlockDelta)
          

          How about calling this bytesToRemove rather than lastBlockDelta? When I think of "delta," I think of something that could be either positive or negative.

                       }
          +            // If we are performing a truncate recovery than set recovery fields
          +            // to old block.
          +            boolean truncateRecovery = b.getRecoveryBlock() != null;
          +            boolean copyOnTruncateRecovery = truncateRecovery &&
          +                b.getRecoveryBlock().getBlockId() != b.getBlockId();
          +            ExtendedBlock primaryBlock = (copyOnTruncateRecovery) ?
          +                new ExtendedBlock(blockPoolId, b.getRecoveryBlock()) :
          +                new ExtendedBlock(blockPoolId, b);
                       // If we only get 1 replica after eliminating stale nodes, then choose all
          

          recoveryBlock seems like a very generic name for this. It seems that BlockInfoUnderConstruction#recoveryBlock is only used for truncate, never for anything else. So let's call it BlockInfoUnderConstruction#truncateDst or something.

          message RecoveringBlockProto {
            required uint64 newGenStamp = 1;      // New genstamp post recovery
            required LocatedBlockProto block = 2; // Block to be recovered
          +  optional BlockProto newBlock = 3;     // New block for recovery (truncate)
          }
          

          Same comment here. Since this is only for truncate, then put truncate in the name somewhere.

          --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto
          +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto
          @@ -59,6 +59,7 @@ message UpdateReplicaUnderRecoveryRequestProto {
             required ExtendedBlockProto block = 1; // Block identifier
             required uint64 recoveryId = 2;        // New genstamp of the replica
             required uint64 newLength = 3;         // New length of the replica
          +  optional uint64 newBlockId = 4;        // New blockId for copy (truncate)
           }
          

          There isn't any default given here for newBlockId, so what's going to happen in a mixed-version scenario? Where you have older datanodes communicating with newer ones. Like in this code:

                storageID = impl.updateReplicaUnderRecovery(
                    PBHelper.convert(request.getBlock()), request.getRecoveryId(),
                    request.getNewBlockId(), request.getNewLength());
          

          It will either be a null pointer exception, or returning 0 unexpectedly, right? I would say just pass in a Long rather than a long, and check for null. If it's null, then don't do anything to the id.
          This would also mean that you could just pass null (i.e. don't pass anything for the optional value) rather than re-setting the block's ID to itself, when doing a non-truncate recovery.

              boolean copyOnTruncate = rur.getBlockId() != newBlockId;
          

          Can we have an accessor for this in ReplicaUnderRecovery? I see this pattern in a lot of places.

             * 2.) INode length is truncated now – clients can only read up to new length.
          

          Should be "new clients can only read up to the truncated length."
          Old clients may be able to read above the truncated length for a while.

              @Override
              public void writeFields(DataOutputStream out) throws IOException {
                FSImageSerialization.writeString(src, out);
                FSImageSerialization.writeString(clientName, out);
                FSImageSerialization.writeString(clientMachine, out);
                FSImageSerialization.writeLong(newLength, out);
                FSImageSerialization.writeLong(timestamp, out);
                int size = truncateBlock != null ? 1 : 0;
                Block[] blocks = new Block[size];
                if (truncateBlock != null) {
                  blocks[0] = truncateBlock;
                }
                FSImageSerialization.writeCompactBlockArray(blocks, out);
              }
          

          This code writes out all of the fields in truncateBlock: blockId, numBytes, generationStamp. But we only care about blockId and generationStamp, right? It might be better just to manually write out blockId and generationStamp, rather than using FSImageSerialization.writeCompactBlockArray.

              if(oldLength == newLength)
                return true;
              if(oldLength < newLength)
                throw new HadoopIllegalArgumentException(
                    "Cannot truncate to a larger file size. Current size: " + oldLength +
                        ", truncate size: " + newLength + ".");
          

          Missing braces around "if". Same in a few other places

          INodeFile: can these changes be split out into a separate patch? They seem like they could be done as a standalone patch since they just add utility methods and so on. Same with the changes in AbstractINodeDiffList and some of the other snapshot stuff.

          Show
          cmccabe Colin P. McCabe added a comment - public boolean truncate( String src, long newLength, String clientName) How about calling the first argument "target" rather than "src"? "src" and "dst" make sense for rename , not so much for truncate since there is no "destination" I would still like to see these functions return an enum like: enum TruncateResult { FINISHED, IN_PROGRESS } the problem with returning a boolean is that people will assume that if it returns false, the operation failed. This is how File#delete works, how File#mkdir works, how pretty much everything in the Java standard library works. We have some stuff like this in Hadoop's FileSystem.java as well. So it will be confusing to users if our API returns a boolean that means something different. But by returning an enum, we make it clear what is going on. Of course, we can still use booleans in the protobuf if that's more convenient. This is just a user API thing. * @param bc file + * @param lastBlockDelta num of bytes to remove from block * @ return the last block locations if the block is partial or null otherwise */ public LocatedBlock convertLastBlockToUnderConstruction( - BlockCollection bc) throws IOException { + BlockCollection bc, long lastBlockDelta) throws IOException { BlockInfo oldBlock = bc.getLastBlock(); if (oldBlock == null || - bc.getPreferredBlockSize() == oldBlock.getNumBytes()) + bc.getPreferredBlockSize() == oldBlock.getNumBytes() - lastBlockDelta) How about calling this bytesToRemove rather than lastBlockDelta ? When I think of "delta," I think of something that could be either positive or negative. } + // If we are performing a truncate recovery than set recovery fields + // to old block. + boolean truncateRecovery = b.getRecoveryBlock() != null ; + boolean copyOnTruncateRecovery = truncateRecovery && + b.getRecoveryBlock().getBlockId() != b.getBlockId(); + ExtendedBlock primaryBlock = (copyOnTruncateRecovery) ? + new ExtendedBlock(blockPoolId, b.getRecoveryBlock()) : + new ExtendedBlock(blockPoolId, b); // If we only get 1 replica after eliminating stale nodes, then choose all recoveryBlock seems like a very generic name for this. It seems that BlockInfoUnderConstruction#recoveryBlock is only used for truncate, never for anything else. So let's call it BlockInfoUnderConstruction#truncateDst or something. message RecoveringBlockProto { required uint64 newGenStamp = 1; // New genstamp post recovery required LocatedBlockProto block = 2; // Block to be recovered + optional BlockProto newBlock = 3; // New block for recovery (truncate) } Same comment here. Since this is only for truncate, then put truncate in the name somewhere. --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto @@ -59,6 +59,7 @@ message UpdateReplicaUnderRecoveryRequestProto { required ExtendedBlockProto block = 1; // Block identifier required uint64 recoveryId = 2; // New genstamp of the replica required uint64 newLength = 3; // New length of the replica + optional uint64 newBlockId = 4; // New blockId for copy (truncate) } There isn't any default given here for newBlockId , so what's going to happen in a mixed-version scenario? Where you have older datanodes communicating with newer ones. Like in this code: storageID = impl.updateReplicaUnderRecovery( PBHelper.convert(request.getBlock()), request.getRecoveryId(), request.getNewBlockId(), request.getNewLength()); It will either be a null pointer exception, or returning 0 unexpectedly, right? I would say just pass in a Long rather than a long , and check for null. If it's null , then don't do anything to the id. This would also mean that you could just pass null (i.e. don't pass anything for the optional value) rather than re-setting the block's ID to itself, when doing a non-truncate recovery. boolean copyOnTruncate = rur.getBlockId() != newBlockId; Can we have an accessor for this in ReplicaUnderRecovery ? I see this pattern in a lot of places. * 2.) INode length is truncated now – clients can only read up to new length. Should be " new clients can only read up to the truncated length." Old clients may be able to read above the truncated length for a while. @Override public void writeFields(DataOutputStream out) throws IOException { FSImageSerialization.writeString(src, out); FSImageSerialization.writeString(clientName, out); FSImageSerialization.writeString(clientMachine, out); FSImageSerialization.writeLong(newLength, out); FSImageSerialization.writeLong(timestamp, out); int size = truncateBlock != null ? 1 : 0; Block[] blocks = new Block[size]; if (truncateBlock != null ) { blocks[0] = truncateBlock; } FSImageSerialization.writeCompactBlockArray(blocks, out); } This code writes out all of the fields in truncateBlock: blockId , numBytes , generationStamp . But we only care about blockId and generationStamp , right? It might be better just to manually write out blockId and generationStamp , rather than using FSImageSerialization.writeCompactBlockArray . if (oldLength == newLength) return true ; if (oldLength < newLength) throw new HadoopIllegalArgumentException( "Cannot truncate to a larger file size. Current size: " + oldLength + ", truncate size: " + newLength + "." ); Missing braces around "if". Same in a few other places INodeFile : can these changes be split out into a separate patch? They seem like they could be done as a standalone patch since they just add utility methods and so on. Same with the changes in AbstractINodeDiffList and some of the other snapshot stuff.
          Hide
          shv Konstantin Shvachko added a comment -

          calling the first argument "target" rather than "src"? "src" and "dst" make sense for rename
          This is traditional. If you check ClientProtocol you should see all methods there use src to identify the file name parameter. Including create, append, complete, setReplication, etc. One can interpret it as a source path for the operation, rather than as an opposite to destination.
          Do you really want to make an exclusion for truncate and call it target?

          I would still like to see these functions return an enum ... people will assume that if it returns false, the operation failed.
          I see some issues with that:

          • In java failure of an operation is signalled by throwing an exception. This is how most of our APIs do and should work.
          • delete() according to the documentation "returns true only if the existing file or directory was actually removed".
          • mkdirs() in fact always returns true as per NameNode implementation. Could have been of void type.
          • boolean truncate() is modelled after boolean recoverLease(). Both return true if the file is known to be closed upon return from the method.

          If you feel strong about returning enum, which I am not, we can create a new jira to change both return values. It will break wire compatibility for recoverLease() though.

          calling this bytesToRemove rather than lastBlockDelta?
          Sure.

          recoveryBlock seems like a very generic name for this.
          Makes sense. How about truncateBlock?

          There isn't any default given here for newBlockId, so what's going to happen in a mixed-version scenario?
          Very good point! Thanks. We should add an explicit default value and treat it under the assumption that old DNs cannot do truncate recovery.

          Can we have an accessor for this in ReplicaUnderRecovery? I see this pattern in a lot of places.
          I found only one occurence of this - the one you identified. There are other places that compare block Ids, but they are applied to Blocks rather than ReplicaUnderRecovery.

          This code writes out all of the fields in truncateBlock: blockId, numBytes, generationStamp. But we only care about blockId and generationStamp, right?
          In fact, all three fields are needed. We need to know newLength when we restart truncate recovery after reading edits.
          Basically, with copy-on-truncate we need two blocks the old one with the old length and the new with the new length. Before snapshots we needed only the new length and the gen stamp, because there was no replica copy. We had only the newLength as a field back then. truncateBlock was added to support snapshots.

          Missing braces around "if".
          Will fix.

          INodeFile: can these changes be split out into a separate patch?
          What do you mean?
          INodeFile, AbstractINodeDiffList, and the other snapshot stuff are an intrinsic part of this implementation of the snapshot support for truncate. This is the separate jira. And that is why we keep it in two separate patches: one in HDFS-3071 and another one here.

          We will incorporate your suggestions shortly.

          Show
          shv Konstantin Shvachko added a comment - calling the first argument "target" rather than "src"? "src" and "dst" make sense for rename This is traditional. If you check ClientProtocol you should see all methods there use src to identify the file name parameter. Including create , append , complete , setReplication , etc. One can interpret it as a source path for the operation , rather than as an opposite to destination . Do you really want to make an exclusion for truncate and call it target ? I would still like to see these functions return an enum ... people will assume that if it returns false, the operation failed. I see some issues with that: In java failure of an operation is signalled by throwing an exception. This is how most of our APIs do and should work. delete() according to the documentation "returns true only if the existing file or directory was actually removed". mkdirs() in fact always returns true as per NameNode implementation. Could have been of void type. boolean truncate() is modelled after boolean recoverLease() . Both return true if the file is known to be closed upon return from the method. If you feel strong about returning enum, which I am not, we can create a new jira to change both return values. It will break wire compatibility for recoverLease() though. calling this bytesToRemove rather than lastBlockDelta ? Sure. recoveryBlock seems like a very generic name for this. Makes sense. How about truncateBlock ? There isn't any default given here for newBlockId, so what's going to happen in a mixed-version scenario ? Very good point! Thanks. We should add an explicit default value and treat it under the assumption that old DNs cannot do truncate recovery. Can we have an accessor for this in ReplicaUnderRecovery? I see this pattern in a lot of places. I found only one occurence of this - the one you identified. There are other places that compare block Ids, but they are applied to Blocks rather than ReplicaUnderRecovery. This code writes out all of the fields in truncateBlock: blockId, numBytes, generationStamp. But we only care about blockId and generationStamp, right ? In fact, all three fields are needed. We need to know newLength when we restart truncate recovery after reading edits. Basically, with copy-on-truncate we need two blocks the old one with the old length and the new with the new length. Before snapshots we needed only the new length and the gen stamp, because there was no replica copy. We had only the newLength as a field back then. truncateBlock was added to support snapshots. Missing braces around "if". Will fix. INodeFile: can these changes be split out into a separate patch ? What do you mean? INodeFile, AbstractINodeDiffList, and the other snapshot stuff are an intrinsic part of this implementation of the snapshot support for truncate. This is the separate jira. And that is why we keep it in two separate patches: one in HDFS-3071 and another one here. We will incorporate your suggestions shortly.
          Hide
          jingzhao Jing Zhao added a comment - - edited

          Some comments so far. I will continue reviewing this weekend.

          1. I may have missed something here, but does the current patch persist FileDiff#blocks into FSImage? Not correctly persisting this information can cause data loss.
          2. We need to have tests for the above case. I suggest we include the FileDiff#blocks information into the output of
            INodeFile#dumpTreeRecursively, and add tests similar to TestSnapshot#checkFSImage for every operation in TestFileTruncate.
          3. I'm still a little confused about the current searching process in findLaterSnapshotBlocks. The following code looks like an nlog(n)
            process since getPrior has log(n) complexity. Why not just doing a linear scan in the diff list after the given snapshot id?
              public BlockInfo[] findLaterSnapshotBlocks(int snapshotId) {
                assert snapshotId != Snapshot.NO_SNAPSHOT_ID : "Wrong snapshot id";
                if(snapshotId == Snapshot.CURRENT_STATE_ID)
                  return null;
                int p = Snapshot.CURRENT_STATE_ID;
                FileDiff diff = null;
                while(snapshotId < p) {
                  p = getPrior(p, true);
                  if(p < snapshotId) break;
                  FileDiff cur = getDiffById(p);
                  if(cur.getBlocks() != null)
                    diff = cur;
                }
                return (diff == null) ? null : diff.getBlocks();
              }
            
          4. Nit: I still see a lot of places missing " {" and "}

            " for "if"/"while"/"for" statements.

          Show
          jingzhao Jing Zhao added a comment - - edited Some comments so far. I will continue reviewing this weekend. I may have missed something here, but does the current patch persist FileDiff#blocks into FSImage? Not correctly persisting this information can cause data loss. We need to have tests for the above case. I suggest we include the FileDiff#blocks information into the output of INodeFile#dumpTreeRecursively , and add tests similar to TestSnapshot#checkFSImage for every operation in TestFileTruncate. I'm still a little confused about the current searching process in findLaterSnapshotBlocks . The following code looks like an nlog(n) process since getPrior has log(n) complexity. Why not just doing a linear scan in the diff list after the given snapshot id? public BlockInfo[] findLaterSnapshotBlocks( int snapshotId) { assert snapshotId != Snapshot.NO_SNAPSHOT_ID : "Wrong snapshot id" ; if (snapshotId == Snapshot.CURRENT_STATE_ID) return null ; int p = Snapshot.CURRENT_STATE_ID; FileDiff diff = null ; while (snapshotId < p) { p = getPrior(p, true ); if (p < snapshotId) break ; FileDiff cur = getDiffById(p); if (cur.getBlocks() != null ) diff = cur; } return (diff == null ) ? null : diff.getBlocks(); } Nit: I still see a lot of places missing " {" and "} " for "if"/"while"/"for" statements.
          Hide
          shv Konstantin Shvachko added a comment -

          > Why not just doing a linear scan in the diff list after the given snapshot id?

          Jing, can you give an example of linear scan in existing code to clarify your point.
          I'll address the rest in a bit.

          Show
          shv Konstantin Shvachko added a comment - > Why not just doing a linear scan in the diff list after the given snapshot id? Jing, can you give an example of linear scan in existing code to clarify your point. I'll address the rest in a bit.
          Hide
          jingzhao Jing Zhao added a comment -

          maybe something like the following? I wrote it in a hurry, so the code may not be in a good shape. but the general idea is to directly process on the diff list instead of keeping calling getPrior and getDiffById.

            public BlockInfo[] findBlocksAfter(INodeFile currentINode, int snapshot) {
              if (snapshot == Snapshot.CURRENT_STATE_ID) {
                return currentINode.getBlocks();
              } else {
                List<FileDiff> diffs = this.asList();
                int i = Collections.binarySearch(diffs, snapshot);
                i = i >= 0 ? i : -i - 1;
                for (; i < diffs.size(); i++) {
                  final FileDiff diff = diffs.get(i);
                  if (diff.getBlocks() != null) {
                    return diff.getBlocks();
                  }
                }
                return currentINode.getBlocks();
              }
            }
          
          Show
          jingzhao Jing Zhao added a comment - maybe something like the following? I wrote it in a hurry, so the code may not be in a good shape. but the general idea is to directly process on the diff list instead of keeping calling getPrior and getDiffById . public BlockInfo[] findBlocksAfter(INodeFile currentINode, int snapshot) { if (snapshot == Snapshot.CURRENT_STATE_ID) { return currentINode.getBlocks(); } else { List<FileDiff> diffs = this .asList(); int i = Collections.binarySearch(diffs, snapshot); i = i >= 0 ? i : -i - 1; for (; i < diffs.size(); i++) { final FileDiff diff = diffs.get(i); if (diff.getBlocks() != null ) { return diff.getBlocks(); } } return currentINode.getBlocks(); } }
          Hide
          cmccabe Colin P. McCabe added a comment -

          Do you really want to make an exclusion for truncate and call it target?

          Hmm. There are other cases where we use something other than "src" to indicate the target path in ClientProtocol.java. For example, createSymlink uses "target". setQuota uses "path" not "src". I would argue that we should not use "src" unless there is a "dst" or destination. However, I don't feel that strongly about this... we can always change it later.

          [return value discussion]

          I agree that FileSystem#mkdir should never have returned a boolean, but only thrown an exception. But, we can't change it now The same thing applies to Java's File methods, by the way... the fact that they don't throw an IOException is very frustrating, and motivated Sun to create the new java.nio.File APIs which do throw IOException rather than just returning false on any kind of error.

          Similarly, I don't think we can change recoverLease at this point. We can't break wire compatibility or API compatibility, as you know. However, I think that its return convention is very confusing. It was a mistake. I have never met someone who wasn't an HDFS developer who understood when this method returns false and when it returns true, and I've often been asked about it. I will let other people comment on this, but I would strongly encourage you not to copy this convention.

          Makes sense. How about truncateBlock?

          Sure.

          Show
          cmccabe Colin P. McCabe added a comment - Do you really want to make an exclusion for truncate and call it target? Hmm. There are other cases where we use something other than "src" to indicate the target path in ClientProtocol.java . For example, createSymlink uses "target". setQuota uses "path" not "src". I would argue that we should not use "src" unless there is a "dst" or destination. However, I don't feel that strongly about this... we can always change it later. [return value discussion] I agree that FileSystem#mkdir should never have returned a boolean, but only thrown an exception. But, we can't change it now The same thing applies to Java's File methods, by the way... the fact that they don't throw an IOException is very frustrating, and motivated Sun to create the new java.nio.File APIs which do throw IOException rather than just returning false on any kind of error. Similarly, I don't think we can change recoverLease at this point. We can't break wire compatibility or API compatibility, as you know. However, I think that its return convention is very confusing. It was a mistake. I have never met someone who wasn't an HDFS developer who understood when this method returns false and when it returns true, and I've often been asked about it. I will let other people comment on this, but I would strongly encourage you not to copy this convention. Makes sense. How about truncateBlock? Sure.
          Hide
          shv Konstantin Shvachko added a comment -

          Here is what we did in the new patch.

          • Jing it was very good catch with persisting snapshot blocks into fsimage. My fault. I thought it was covered by our tests, because we have a bunch of restarts, but it turned out restart does not save the image as it used before.
            New patch persists snapshot blocks and adds a test case, which explicitly saves namespace before restarting.
          • Also bumped up the NameNode layout version.
          • Added optimized findLaterSnapshotBlocks()
          • Did the renames requested by Colin.
          • Added default value for the newBlockId field.
            • This actually raised a question for me how it will work with rolling upgrades. Thinking about it.
          • Colin, I did not introduce enum as the return value. I would have if there were three or more values or a potential to have more. But its exactly two. You are right we cannot change recoverLease() now. So it is better to have the same pattern in truncate() to avoid even more confusion why this is done differently in two cases.
            Anyways we clearly disagree, so it would be good if anybody esle could weigh on the subject.
          Show
          shv Konstantin Shvachko added a comment - Here is what we did in the new patch. Jing it was very good catch with persisting snapshot blocks into fsimage. My fault. I thought it was covered by our tests, because we have a bunch of restarts, but it turned out restart does not save the image as it used before. New patch persists snapshot blocks and adds a test case, which explicitly saves namespace before restarting. Also bumped up the NameNode layout version. Added optimized findLaterSnapshotBlocks() Did the renames requested by Colin. Added default value for the newBlockId field. This actually raised a question for me how it will work with rolling upgrades. Thinking about it. Colin, I did not introduce enum as the return value. I would have if there were three or more values or a potential to have more. But its exactly two. You are right we cannot change recoverLease() now. So it is better to have the same pattern in truncate() to avoid even more confusion why this is done differently in two cases. Anyways we clearly disagree, so it would be good if anybody esle could weigh on the subject.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12688597/HDFS-3107-HDFS-7056-combined-13.patch
          against trunk revision 7bc0a6d.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
          org.apache.hadoop.fs.TestSymlinkHdfsFileContext
          org.apache.hadoop.hdfs.server.namenode.TestFileTruncate

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9105//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9105//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9105//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12688597/HDFS-3107-HDFS-7056-combined-13.patch against trunk revision 7bc0a6d. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer org.apache.hadoop.fs.TestSymlinkHdfsFileContext org.apache.hadoop.hdfs.server.namenode.TestFileTruncate Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9105//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9105//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9105//console This message is automatically generated.
          Hide
          cos Konstantin Boudnik added a comment -

          I would have if there were three or more values or a potential to have more. But its exactly two. You are right we cannot change recoverLease() now. So it is better to have the same pattern in truncate() to avoid even more confusion why this is done differently in two cases.

          I tend to agree with Konstantin Shvachko here: a sudden introduction a new contract's fashion will be more confusing. Besides, enforcing a enum for just two possible return values sounds excessive and unnecessary. It'd be totally acceptable if the method could return say seven different states.

          This actually raised a question for me how it will work with rolling upgrades. Thinking about it.

          Shall we address the rolling upgrade issue in a separate ticket? It seems that dragging this much longer will have a significant impact on the patch maintenance: we already see multiple iterations of the same patch just because of some minor changes in the trunk.

          Show
          cos Konstantin Boudnik added a comment - I would have if there were three or more values or a potential to have more. But its exactly two. You are right we cannot change recoverLease() now. So it is better to have the same pattern in truncate() to avoid even more confusion why this is done differently in two cases. I tend to agree with Konstantin Shvachko here: a sudden introduction a new contract's fashion will be more confusing. Besides, enforcing a enum for just two possible return values sounds excessive and unnecessary. It'd be totally acceptable if the method could return say seven different states. This actually raised a question for me how it will work with rolling upgrades. Thinking about it. Shall we address the rolling upgrade issue in a separate ticket? It seems that dragging this much longer will have a significant impact on the patch maintenance: we already see multiple iterations of the same patch just because of some minor changes in the trunk.
          Hide
          jingzhao Jing Zhao added a comment -

          Some further comments:

          1. In FileDiffList#combineAndCollectSnapshotBlocks, why do we pass null for collectBlocksAndClear? In case that the file has been deleted and we're deleting the last snapshot, we need to delete the whole INode thus should pass a non-null INode List to collectBlocksAndClear. We should also have unit tests for this if we can confirm this is an issue.
            +    BlockInfo[] removedBlocks = removed.getBlocks();
            +    if(removedBlocks == null) {
            +      FileWithSnapshotFeature sf = file.getFileWithSnapshotFeature();
            +      assert sf != null : "FileWithSnapshotFeature is null";
            +      if(sf.isCurrentFileDeleted())
            +        sf.collectBlocksAndClear(file, collectedBlocks, null);
            +      return;
            +    }
            
          2. The semantic of findEarlierSnapshotBlocks is not easy to follow. Looks like when snapshotId is Snapshot.CURRENT_STATE_ID the function selects an exclusive semantic, while for others the function chooses an inclusive semantic. We need to make it more consistent here. Also it can be optimized similarly as findLaterSnapshotBlocks.
          3. Minor: in the current patch findLaterSnapshotBlocks is always coupled with an extra null check and if it returns null getBlocks is called. This extra check/call can actually be combined into findLaterSnapshotBlocks.
            +    snapshotBlocks = getDiffs().findLaterSnapshotBlocks(diff.getSnapshotId());
            +    return (snapshotBlocks == null) ? getBlocks() : snapshotBlocks;
            
          4. Also bumped up the NameNode layout version.

            Could you generally explain why we need to bump up the NN layout version here? The protobuf-based fsimage should be able to handle its compatibility. One benefit for bumping up layoutversion can be preventing rolling downgrade so that an fsimage produced by "snapshot+truncate" cannot be directly loaded by the old version jar.

          Show
          jingzhao Jing Zhao added a comment - Some further comments: In FileDiffList#combineAndCollectSnapshotBlocks , why do we pass null for collectBlocksAndClear ? In case that the file has been deleted and we're deleting the last snapshot, we need to delete the whole INode thus should pass a non-null INode List to collectBlocksAndClear . We should also have unit tests for this if we can confirm this is an issue. + BlockInfo[] removedBlocks = removed.getBlocks(); + if (removedBlocks == null ) { + FileWithSnapshotFeature sf = file.getFileWithSnapshotFeature(); + assert sf != null : "FileWithSnapshotFeature is null " ; + if (sf.isCurrentFileDeleted()) + sf.collectBlocksAndClear(file, collectedBlocks, null ); + return ; + } The semantic of findEarlierSnapshotBlocks is not easy to follow. Looks like when snapshotId is Snapshot.CURRENT_STATE_ID the function selects an exclusive semantic, while for others the function chooses an inclusive semantic. We need to make it more consistent here. Also it can be optimized similarly as findLaterSnapshotBlocks . Minor: in the current patch findLaterSnapshotBlocks is always coupled with an extra null check and if it returns null getBlocks is called. This extra check/call can actually be combined into findLaterSnapshotBlocks . + snapshotBlocks = getDiffs().findLaterSnapshotBlocks(diff.getSnapshotId()); + return (snapshotBlocks == null ) ? getBlocks() : snapshotBlocks; Also bumped up the NameNode layout version. Could you generally explain why we need to bump up the NN layout version here? The protobuf-based fsimage should be able to handle its compatibility. One benefit for bumping up layoutversion can be preventing rolling downgrade so that an fsimage produced by "snapshot+truncate" cannot be directly loaded by the old version jar.
          Hide
          shv Konstantin Shvachko added a comment -
          1. When the last snapshot is deleted it doesn't even go into combineAndCollectSnapshotBlocks(). It cleans everything via directory's deleted list. See destroyDeletedList(), which is called from destroyDiffAndCollectBlocks(). The patch did not change anything here.
            So passing null into collectBlocksAndClear() is not a problem. But I agree it is more logical, so I added the parameter. I also added a test case to make sure that the number of inodes is decreasing in this case.
          2. findEarlierSnapshotBlocks is not easy to follow.
            You seem to be doing alright. And snapshots are not easy to follow in the first place.
            I think I simplified and optimized it.
          3. findLaterSnapshotBlocks is always coupled with an extra null check
            I considered it. It will require passing INodeFile as a parameter into{{findLaterSnapshotBlocks()}}, which to me unnecessarily breaks the hierarchy of classes. We would be calling a method on the INodeFile member, which in turn will call a method of INodeFile. I would rather avoid such cyclicals if possible, which I know we use everywhere.
          4. Could you generally explain why we need to bump up the NN layout version here?
            1. preventing rolling downgrade is one thing
            2. If you do regular startup without rolling upgrade, then you should be prompted to use upgrade option. Otherwise, you loose opportunity to rollback if needed.
            3. I thought that knowing which version supports truncate will be useful for rolling upgrades from pre-truncate versions. Say if you upgrade active NN before SBN and call truncate, SBN will crash. So it may be better not to use new functionality until the upgrade is done.

          Was looking at the Jenkins warning from the last build.

          • Turned out one findbugs warning is related to truncate. Fixed it in HDFS-3107.
          • The other warning is covered in HDFS-7551.
          • TestSymlinkHdfsFileContext failure is related to HADOOP-11432
          • TestOEV is expected.
          • Could not reproduce TestFileTruncate wait timeout. Will keep an eye if it happens again.
          Show
          shv Konstantin Shvachko added a comment - When the last snapshot is deleted it doesn't even go into combineAndCollectSnapshotBlocks() . It cleans everything via directory's deleted list. See destroyDeletedList() , which is called from destroyDiffAndCollectBlocks() . The patch did not change anything here. So passing null into collectBlocksAndClear() is not a problem. But I agree it is more logical, so I added the parameter. I also added a test case to make sure that the number of inodes is decreasing in this case. findEarlierSnapshotBlocks is not easy to follow. You seem to be doing alright. And snapshots are not easy to follow in the first place. I think I simplified and optimized it. findLaterSnapshotBlocks is always coupled with an extra null check I considered it. It will require passing INodeFile as a parameter into{{findLaterSnapshotBlocks()}}, which to me unnecessarily breaks the hierarchy of classes. We would be calling a method on the INodeFile member, which in turn will call a method of INodeFile. I would rather avoid such cyclicals if possible, which I know we use everywhere. Could you generally explain why we need to bump up the NN layout version here ? preventing rolling downgrade is one thing If you do regular startup without rolling upgrade, then you should be prompted to use upgrade option. Otherwise, you loose opportunity to rollback if needed. I thought that knowing which version supports truncate will be useful for rolling upgrades from pre-truncate versions. Say if you upgrade active NN before SBN and call truncate, SBN will crash. So it may be better not to use new functionality until the upgrade is done. Was looking at the Jenkins warning from the last build. Turned out one findbugs warning is related to truncate. Fixed it in HDFS-3107 . The other warning is covered in HDFS-7551 . TestSymlinkHdfsFileContext failure is related to HADOOP-11432 TestOEV is expected. Could not reproduce TestFileTruncate wait timeout. Will keep an eye if it happens again.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12688840/HDFS-3107-HDFS-7056-combined-15.patch
          against trunk revision 5caebba.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 10 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.TestDecommission
          org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9118//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9118//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9118//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12688840/HDFS-3107-HDFS-7056-combined-15.patch against trunk revision 5caebba. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 10 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.TestDecommission org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9118//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9118//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9118//console This message is automatically generated.
          Hide
          shv Konstantin Shvachko added a comment -

          The latest build was actually successful.

          • Findbugs warning is fixed by HDFS-7583
          • TestDecommission.testIncludeByRegistrationName() failure is reported in HDFS-7083

          Are there any unresolved issues remaining? Jing Zhao, Colin P. McCabe, everybody. If not could you please cast your votes.
          The patches are still relevant for trunk.

          Show
          shv Konstantin Shvachko added a comment - The latest build was actually successful. Findbugs warning is fixed by HDFS-7583 TestDecommission.testIncludeByRegistrationName() failure is reported in HDFS-7083 Are there any unresolved issues remaining? Jing Zhao , Colin P. McCabe , everybody. If not could you please cast your votes. The patches are still relevant for trunk.
          Hide
          cos Konstantin Boudnik added a comment -

          I've looked again into the latest patch. The code looks clean and I don't see naming issues to be a blocker for the patch. I went through the testing part once more and the coverage looks pretty comprehensive as well. There was a pretty decent effort to test this internally and no issues were found. Hence +1 [binding]

          Show
          cos Konstantin Boudnik added a comment - I've looked again into the latest patch. The code looks clean and I don't see naming issues to be a blocker for the patch. I went through the testing part once more and the coverage looks pretty comprehensive as well. There was a pretty decent effort to test this internally and no issues were found. Hence +1 [binding]
          Hide
          shv Konstantin Shvachko added a comment -

          This needs editsStored updated as well for TestOEV to pass, since the layout version changed.

          Show
          shv Konstantin Shvachko added a comment - This needs editsStored updated as well for TestOEV to pass, since the layout version changed.
          Hide
          shv Konstantin Shvachko added a comment -

          I just committed this to trunk. Thank you Plamen.
          Thanks everybody who contributed with reviews, comments, design, testing.

          Show
          shv Konstantin Shvachko added a comment - I just committed this to trunk. Thank you Plamen. Thanks everybody who contributed with reviews, comments, design, testing.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #6853 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6853/)
          HDFS-7056. Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #6853 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6853/ ) HDFS-7056 . Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #72 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/72/)
          HDFS-7056. Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #72 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/72/ ) HDFS-7056 . Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f) hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #806 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/806/)
          HDFS-7056. Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #806 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/806/ ) HDFS-7056 . Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #69 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/69/)
          HDFS-7056. Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #69 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/69/ ) HDFS-7056 . Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk #2004 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2004/)
          HDFS-7056. Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #2004 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2004/ ) HDFS-7056 . Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #73 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/73/)
          HDFS-7056. Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #73 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/73/ ) HDFS-7056 . Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2023 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2023/)
          HDFS-7056. Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2023 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2023/ ) HDFS-7056 . Snapshot support for truncate. Contributed by Konstantin Shvachko and Plamen Jeliazkov. (shv: rev 08ac06283a3e9bf0d49d873823aabd419b08e41f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/InterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/FsDatasetSpi.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/fsimage.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FSImageFormatPBSnapshot.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoUnderConstruction.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored.xml hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/SimulatedFSDataset.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileDiff.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeLayoutVersion.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockRecoveryCommand.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/FileWithSnapshotFeature.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java
          Hide
          zero45 Plamen Jeliazkov added a comment -

          Attaching patch for branch-2 port. Tests passed locally using the editsStored file attached to this JIRA.

          Show
          zero45 Plamen Jeliazkov added a comment - Attaching patch for branch-2 port. Tests passed locally using the editsStored file attached to this JIRA.
          Hide
          shv Konstantin Shvachko added a comment -

          Merged to branch-2.

          Show
          shv Konstantin Shvachko added a comment - Merged to branch-2.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          HDFSSnapshotWithTruncateDesign.docx: obsoletes the previous doc.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - HDFSSnapshotWithTruncateDesign.docx: obsoletes the previous doc.
          Hide
          jingzhao Jing Zhao added a comment -

          Hi Konstantin Shvachko and Plamen Jeliazkov, I just noticed that the current FileDiffList#findEarlierSnapshotBlocks is like this:

              ......
              int i = Collections.binarySearch(diffs, snapshotId);
              BlockInfoContiguous[] blocks = null;
              for(i = i >= 0 ? i : -i; i < diffs.size(); i--) {
                blocks = diffs.get(i).getBlocks();
                if(blocks != null) {
                  break;
                }
              }
              return blocks;
            }
          

          Since the semantic of this function is to find blocks recorded in a diff with snapshot id <= the given id, I guess the for loop should looks like:

            for(i = i >= 0 ? i : -i-2; i < diffs.size() && i >= 0; i--) {
              ......
            }
          
          Show
          jingzhao Jing Zhao added a comment - Hi Konstantin Shvachko and Plamen Jeliazkov , I just noticed that the current FileDiffList#findEarlierSnapshotBlocks is like this: ...... int i = Collections.binarySearch(diffs, snapshotId); BlockInfoContiguous[] blocks = null ; for (i = i >= 0 ? i : -i; i < diffs.size(); i--) { blocks = diffs.get(i).getBlocks(); if (blocks != null ) { break ; } } return blocks; } Since the semantic of this function is to find blocks recorded in a diff with snapshot id <= the given id, I guess the for loop should looks like: for (i = i >= 0 ? i : -i-2; i < diffs.size() && i >= 0; i--) { ...... }
          Hide
          shv Konstantin Shvachko added a comment -

          Sounds right, Jing. Let me write it down to make sure my math is right this time.
          binarySearch(diffs, snapshotId) returns i == -insertPoint-1, where insertPoint is the index of the first element greater than the key. So if the snapshotId is not found then we can start searching backwards from insertPoint-1 == -i-2.
          So we've been checking two extra elements. Good catch, thanks. Will create a jira to fix this.

          Show
          shv Konstantin Shvachko added a comment - Sounds right, Jing. Let me write it down to make sure my math is right this time. binarySearch(diffs, snapshotId) returns i == -insertPoint-1 , where insertPoint is the index of the first element greater than the key. So if the snapshotId is not found then we can start searching backwards from insertPoint-1 == -i-2 . So we've been checking two extra elements. Good catch, thanks. Will create a jira to fix this.
          Hide
          jingzhao Jing Zhao added a comment -

          That's right, Konstantin Shvachko. And more importantly, we need to have "i >= 0" in the loop condition.

          Show
          jingzhao Jing Zhao added a comment - That's right, Konstantin Shvachko . And more importantly, we need to have "i >= 0" in the loop condition.
          Hide
          shv Konstantin Shvachko added a comment -

          The patch is up for review in HDFS-7831.

          Show
          shv Konstantin Shvachko added a comment - The patch is up for review in HDFS-7831 .

            People

            • Assignee:
              zero45 Plamen Jeliazkov
              Reporter:
              shv Konstantin Shvachko
            • Votes:
              0 Vote for this issue
              Watchers:
              19 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development