Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 0.92.0
    • Fix Version/s: 0.92.3
    • Component/s: None
    • Labels:
      None

      Description

      Currently some 30 or so tests are failing on the "HBase-trunk-on-hadoop-23" build. It looks like most are reflection-based issues.

        Issue Links

          Activity

          Hide
          Todd Lipcon added a comment -

          TestLogRolling.testLogRollOnPipelineRestart is failing due to HDFS-2288

          Show
          Todd Lipcon added a comment - TestLogRolling.testLogRollOnPipelineRestart is failing due to HDFS-2288
          Hide
          Ted Yu added a comment -

          HBASE-4510 is marked for 0.94
          Moving this issue to 0.94 as well.

          Show
          Ted Yu added a comment - HBASE-4510 is marked for 0.94 Moving this issue to 0.94 as well.
          Hide
          Todd Lipcon added a comment -

          Why move to 0.94? This isn't marked a blocker, and we should be targeting the 92 branch, even if it doesn't make it for 0.92.0.

          Show
          Todd Lipcon added a comment - Why move to 0.94? This isn't marked a blocker, and we should be targeting the 92 branch, even if it doesn't make it for 0.92.0.
          Hide
          Ted Yu added a comment -

          Feel free to bring this and HBASE-4510 to 0.92, knowing they're not blockers.

          Show
          Ted Yu added a comment - Feel free to bring this and HBASE-4510 to 0.92, knowing they're not blockers.
          Hide
          stack added a comment -

          Knocking this down to major from critical.

          Show
          stack added a comment - Knocking this down to major from critical.
          Hide
          Gregory Chanan added a comment -

          I did some playing around with TestLogRolling.testLogRollOnPipelineRestart against 0.23. It looks like calling recoverLease on the HLog path and waiting awhile before trying to create the reader works. If you don't call recoverLease the reader will never succeed.

          Let's assume, for the sake of argument, that replicas waiting recovery will not return a full visible length.

          1) Are we supposed to have to call recoverLease? (I'm guessing the recovery design doc covers this, I'll check it out)
          2) Where should we call recoverLease? Just in the test? In HLog.createWriter? (it would block for awhile if we actually waited for the recovery)?

          Show
          Gregory Chanan added a comment - I did some playing around with TestLogRolling.testLogRollOnPipelineRestart against 0.23. It looks like calling recoverLease on the HLog path and waiting awhile before trying to create the reader works. If you don't call recoverLease the reader will never succeed. Let's assume, for the sake of argument, that replicas waiting recovery will not return a full visible length. 1) Are we supposed to have to call recoverLease? (I'm guessing the recovery design doc covers this, I'll check it out) 2) Where should we call recoverLease? Just in the test? In HLog.createWriter? (it would block for awhile if we actually waited for the recovery)?
          Hide
          Gregory Chanan added a comment -

          NOTE: I'm not sure if we should commit this, this is just a brainstorm.

          This is a patch for hbase-0.92 that fixes TestLogRolling.testLogRollOnPipelineRestart against hadoop-0.23 while not breaking it against hadoop-1.0.0. If we want to commit this, I can create patches for trunk/94.

          From my comment:
          1) It sounds like under the current design, we need to call recoverLease to read from the HLog:

          From the recovery design doc (see HDFS-2288):
          "• Any
 WaitingToBeRecovered
 replica 
does 
not 
serve 
any 
read and
 does 
not participate
 in 
a 
pipeline 
recovery.
          • A
 WaitingToBeRecovered 
replica 
will
 either 
become 
out dated
 and 
be deleted
 by 
NN 
if 
the 
client 
is 
still 
alive 
or 
be 
changed
 to
 be 
finalized 
as 
a result
 of 
lease
 recovery
 if
 the 
client
 dies"

          This is basically what I've done: if we cannot get a reader for the HLog b/c replicas-awaiting-recovery do not return a visible length, recover the lease, wait for it to finish, then retry getting the HLog reader. (Aside: It would be nice if the exception HDFS returned in this state was clearer than IOException).

          Is this is correct place for this code? Do we need it everywhere we call HLog.getReader? Any thoughts?

          Show
          Gregory Chanan added a comment - Attached HBASE-4254 -92.patch* NOTE: I'm not sure if we should commit this, this is just a brainstorm. This is a patch for hbase-0.92 that fixes TestLogRolling.testLogRollOnPipelineRestart against hadoop-0.23 while not breaking it against hadoop-1.0.0. If we want to commit this, I can create patches for trunk/94. From my comment: 1) It sounds like under the current design, we need to call recoverLease to read from the HLog: From the recovery design doc (see HDFS-2288 ): "• Any
 WaitingToBeRecovered
 replica 
does 
not 
serve 
any 
read and
 does 
not participate
 in 
a 
pipeline 
recovery. • A
 WaitingToBeRecovered 
replica 
will
 either 
become 
out dated
 and 
be deleted
 by 
NN 
if 
the 
client 
is 
still 
alive 
or 
be 
changed
 to
 be 
finalized 
as 
a result
 of 
lease
 recovery
 if
 the 
client
 dies" This is basically what I've done: if we cannot get a reader for the HLog b/c replicas-awaiting-recovery do not return a visible length, recover the lease, wait for it to finish, then retry getting the HLog reader. (Aside: It would be nice if the exception HDFS returned in this state was clearer than IOException). Is this is correct place for this code? Do we need it everywhere we call HLog.getReader? Any thoughts?
          Hide
          Ted Yu added a comment -

          Interesting idea.

          +        waitUntilNotUnderConstruction(dfsCluster,p.toUri().getPath());
          +        readRowsFromHLog(p,loggedRows);
          

          How do we know that the second call to readRowsFromHLog() wouldn't result in IOE ?

          +    do {
          +      Thread.sleep(100);
          +      blocks = (LocatedBlocks) getBlockLocations.invoke(null,cluster.getNameNode(),testPath,0,1);
          +    } while (blocks.isUnderConstruction());
          

          Do we need some timeout for the above wait ?

          Once the logic in the patch gets confirmation from various experts (including Todd), we can consider moving it to a better place.

          I would also like to refer to https://issues.apache.org/jira/browse/HDFS-2991 which we encountered in staging environment.
          For 0.23.2-SNAPSHOT, we should have the fix.

          Show
          Ted Yu added a comment - Interesting idea. + waitUntilNotUnderConstruction(dfsCluster,p.toUri().getPath()); + readRowsFromHLog(p,loggedRows); How do we know that the second call to readRowsFromHLog() wouldn't result in IOE ? + do { + Thread .sleep(100); + blocks = (LocatedBlocks) getBlockLocations.invoke( null ,cluster.getNameNode(),testPath,0,1); + } while (blocks.isUnderConstruction()); Do we need some timeout for the above wait ? Once the logic in the patch gets confirmation from various experts (including Todd), we can consider moving it to a better place. I would also like to refer to https://issues.apache.org/jira/browse/HDFS-2991 which we encountered in staging environment. For 0.23.2-SNAPSHOT, we should have the fix.
          Hide
          stack added a comment -

          @Gregory

          I see over in FSHDFSUTils that we call recoverLease via reflection currently. If this fix is for 0.96 where we depend on 1.0.0, we can do as you do and call w/o going via reflection.

          In current code too, it throws AlreadyBeingCreatedException if lease not available. I suppose new code just doesn't do that. In old code, we used just hang around:

          if (e instanceof AlreadyBeingCreatedException) {
                    // We expect that we'll get this message while the lease is still
                    // within its soft limit, but if we get it past that, it means
                    // that the RS is holding onto the file even though it lost its
                    // znode. We could potentially abort after some time here.
                    long waitedFor = System.currentTimeMillis() - startWaiting;
                    if (waitedFor > LEASE_SOFTLIMIT_PERIOD) {
                      LOG.warn("Waited " + waitedFor + "ms for lease recovery on " + p +
                        ":" + e.getMessage());
                    }
          
          ... we'd sleep a second and retry.
          

          The fancy-dancing asking the namenode if blocks are under construction will work for 0.23 only? If the method is not available, we just return immediately w/o pause. Maybe we should pause some time before going around the loop again.

          On 1., 'It sounds like under the current design, we need to call recoverLease to read from the HLog:' OK. Seems fair enough given we just restarted the datanodes.

          As to where the code belongs, looks like we have to add it everywhere a getReader on WAL is called (not too many places I'd say) but its weird that the fs semantic changes so much (with previous you could crash out the datanodes and it'd just 'work' when you got a new reader but now you have to recover outstanding leases... the old behavior was probably the 'wrong' one)

          Show
          stack added a comment - @Gregory I see over in FSHDFSUTils that we call recoverLease via reflection currently. If this fix is for 0.96 where we depend on 1.0.0, we can do as you do and call w/o going via reflection. In current code too, it throws AlreadyBeingCreatedException if lease not available. I suppose new code just doesn't do that. In old code, we used just hang around: if (e instanceof AlreadyBeingCreatedException) { // We expect that we'll get this message while the lease is still // within its soft limit, but if we get it past that, it means // that the RS is holding onto the file even though it lost its // znode. We could potentially abort after some time here. long waitedFor = System .currentTimeMillis() - startWaiting; if (waitedFor > LEASE_SOFTLIMIT_PERIOD) { LOG.warn( "Waited " + waitedFor + "ms for lease recovery on " + p + ":" + e.getMessage()); } ... we'd sleep a second and retry. The fancy-dancing asking the namenode if blocks are under construction will work for 0.23 only? If the method is not available, we just return immediately w/o pause. Maybe we should pause some time before going around the loop again. On 1., 'It sounds like under the current design, we need to call recoverLease to read from the HLog:' OK. Seems fair enough given we just restarted the datanodes. As to where the code belongs, looks like we have to add it everywhere a getReader on WAL is called (not too many places I'd say) but its weird that the fs semantic changes so much (with previous you could crash out the datanodes and it'd just 'work' when you got a new reader but now you have to recover outstanding leases... the old behavior was probably the 'wrong' one)
          Hide
          Todd Lipcon added a comment -

          I haven't had time to really dig into this yet – lots of details to sift through. But I think the idea of calling recoverLease anywhere we read from a WAL isn't quite right, since it will probably break replication. We read from open WALs in that circumstance, and calling recoverLease under an open file will kill the writer.

          Show
          Todd Lipcon added a comment - I haven't had time to really dig into this yet – lots of details to sift through. But I think the idea of calling recoverLease anywhere we read from a WAL isn't quite right, since it will probably break replication. We read from open WALs in that circumstance, and calling recoverLease under an open file will kill the writer.
          Hide
          stack added a comment -

          We read from open WALs in that circumstance, and calling recoverLease under an open file will kill the writer.

          Good point.

          Show
          stack added a comment - We read from open WALs in that circumstance, and calling recoverLease under an open file will kill the writer. Good point.
          Hide
          Todd Lipcon added a comment -

          This has been subsumed by other JIRAs, right?

          Show
          Todd Lipcon added a comment - This has been subsumed by other JIRAs, right?
          Hide
          stack added a comment -

          Resolving won't fix. We're more about hadoop2 these times. We don't explicitly support 0.23.

          Show
          stack added a comment - Resolving won't fix. We're more about hadoop2 these times. We don't explicitly support 0.23.

            People

            • Assignee:
              Todd Lipcon
              Reporter:
              Todd Lipcon
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development