Hadoop Common
  1. Hadoop Common
  2. HADOOP-4499

DFSClient should invoke checksumOk only once.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.18.0
    • Fix Version/s: 0.18.3
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      This is a follow up for HADOOP-3914. HADOOP-3914 added a log when DFSClient tries to invoke checksumOk more than once. This jira is mainly for removing the debug log since we know why it happens.

      1. HADOOP-4499.patch
        1 kB
        Raghu Angadi
      2. HADOOP-4499-branch-18.patch
        1 kB
        Raghu Angadi
      3. HADOOP-4499-branch-19.patch
        1 kB
        Raghu Angadi

        Issue Links

          Activity

          Hide
          Raghu Angadi added a comment -

          The problem was that read() return data even if gotEOS for datanode connection is already set because there could be data left in FSInputChecker buffer (upto 512 bytes). The attached patch sends checksum only when 'gotEOS' changes from false to true.

          I will attach a trunk patch that removes the debug log.

          Show
          Raghu Angadi added a comment - The problem was that read() return data even if gotEOS for datanode connection is already set because there could be data left in FSInputChecker buffer (upto 512 bytes). The attached patch sends checksum only when 'gotEOS' changes from false to true. I will attach a trunk patch that removes the debug log.
          Hide
          Raghu Angadi added a comment -

          patch for trunk is attached.

          Show
          Raghu Angadi added a comment - patch for trunk is attached.
          Hide
          Hairong Kuang added a comment -

          Hi Raghu, is it possible to set gotEOS to be true only once?

          Show
          Hairong Kuang added a comment - Hi Raghu, is it possible to set gotEOS to be true only once?
          Hide
          Raghu Angadi added a comment -

          > Hi Raghu, is it possible to set gotEOS to be true only once?
          yes. it is set to true only once in readChunk(), and readChunk() return -1 if it is called again.

          Show
          Raghu Angadi added a comment - > Hi Raghu, is it possible to set gotEOS to be true only once? yes. it is set to true only once in readChunk() , and readChunk() return -1 if it is called again.
          Hide
          Hairong Kuang added a comment -

          OK I see. Now I understand the code better. The problem was caused by the data buffering. The confusing part is that gotEOS represents the end of "raw" stream not the block stream.

          +1.

          Show
          Hairong Kuang added a comment - OK I see. Now I understand the code better. The problem was caused by the data buffering. The confusing part is that gotEOS represents the end of "raw" stream not the block stream. +1.
          Hide
          Raghu Angadi added a comment -

          Thanks Hairong.
          Here is test-patch output :

               [exec] -1 overall.  
          
               [exec]     +1 @author.  The patch does not contain any @author tags.
          
               [exec]     -1 tests included.  The patch doesn't appear to include any new or modified tests.
               [exec]                         Please justify why no tests are needed for this patch.
          
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
          
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
          
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
          
               [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          
          

          This functionality is already covered TestDataBlockScanner.

          Show
          Raghu Angadi added a comment - Thanks Hairong. Here is test-patch output : [exec] -1 overall. [exec] +1 @author. The patch does not contain any @author tags. [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no tests are needed for this patch. [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. This functionality is already covered TestDataBlockScanner.
          Hide
          Raghu Angadi added a comment -

          I just committed this.

          Show
          Raghu Angadi added a comment - I just committed this.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk #646 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/646/)
          . DFSClient should invoke checksumOk only once. (Raghu Angadi)

          Show
          Hudson added a comment - Integrated in Hadoop-trunk #646 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/646/ ) . DFSClient should invoke checksumOk only once. (Raghu Angadi)
          Hide
          stack added a comment -

          Is this supposed to have fixed HADOOP-4562?

          Show
          stack added a comment - Is this supposed to have fixed HADOOP-4562 ?
          Hide
          Raghu Angadi added a comment -

          Yes. I will close HADOOP-4562.

          Show
          Raghu Angadi added a comment - Yes. I will close HADOOP-4562 .
          Hide
          Raghu Angadi added a comment -

          Since HADOOP-3914 is committed to 0.18, I think this should go into 18 as well.

          Show
          Raghu Angadi added a comment - Since HADOOP-3914 is committed to 0.18, I think this should go into 18 as well.
          Hide
          Raghu Angadi added a comment -

          patch for 0.18.

          Show
          Raghu Angadi added a comment - patch for 0.18.
          Hide
          Hairong Kuang added a comment -

          +1 on the 0.18 patch.

          Show
          Hairong Kuang added a comment - +1 on the 0.18 patch.
          Hide
          Raghu Angadi added a comment -

          Just committed to 0.18.3.

          Show
          Raghu Angadi added a comment - Just committed to 0.18.3.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk #653 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/653/)
          new section for 0.18.3. and is moved to 0.18.3

          Show
          Hudson added a comment - Integrated in Hadoop-trunk #653 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/653/ ) new section for 0.18.3. and is moved to 0.18.3

            People

            • Assignee:
              Raghu Angadi
              Reporter:
              Raghu Angadi
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development