Review Board 1.7.22


OpenedRegionHandler is not enforcing atomicity of the operation it is performing

Review Request #2251 - Created Oct. 6, 2011 and updated

ramkrishna vasudevan
0.92, trunk
HBASE-4540
Reviewers
hbase
jgray, stack, tedyu
hbase
Fix for handling HBASE-4539 and HBASE-4540.
Ran all the testcases.  Added one new testcase to verify OpenedRegionHandler scenarios.
Also addresses Ted's comments.
Yes
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
Revision 1179945 New Change
[20] 423 lines
[+20] [+] public void setRegionsToReopen(List <HRegionInfo> regions) {
424
   */
424
   */
425
  boolean processRegionInTransition(final String encodedRegionName,
425
  boolean processRegionInTransition(final String encodedRegionName,
426
      final HRegionInfo regionInfo,
426
      final HRegionInfo regionInfo,
427
      final Map<ServerName,List<Pair<HRegionInfo,Result>>> deadServers)
427
      final Map<ServerName,List<Pair<HRegionInfo,Result>>> deadServers)
428
  throws KeeperException, IOException {
428
  throws KeeperException, IOException {
429
    RegionTransitionData data = ZKAssign.getData(watcher, encodedRegionName);
429
    Stat stat = new Stat();

    
   
430
    RegionTransitionData data = ZKAssign.getDataAndWatch(watcher,

    
   
431
        encodedRegionName, stat);
430
    if (data == null) return false;
432
    if (data == null) return false;
431
    HRegionInfo hri = regionInfo;
433
    HRegionInfo hri = regionInfo;
432
    if (hri == null) {
434
    if (hri == null) {
433
      Pair<HRegionInfo, ServerName> p =
435
      Pair<HRegionInfo, ServerName> p =
434
        MetaReader.getRegion(catalogTracker, data.getRegionName());
436
        MetaReader.getRegion(catalogTracker, data.getRegionName());
435
      if (p == null) return false;
437
      if (p == null) return false;
436
      hri = p.getFirst();
438
      hri = p.getFirst();
437
    }
439
    }
438
    processRegionsInTransition(data, hri, deadServers);
440
    processRegionsInTransition(data, hri, deadServers, stat.getVersion());
439
    return true;
441
    return true;
440
  }
442
  }
441

    
   
443

   
442
  void processRegionsInTransition(final RegionTransitionData data,
444
  void processRegionsInTransition(final RegionTransitionData data,
443
      final HRegionInfo regionInfo,
445
      final HRegionInfo regionInfo,
444
      final Map<ServerName,List<Pair<HRegionInfo,Result>>> deadServers)
446
      final Map<ServerName, List<Pair<HRegionInfo, Result>>> deadServers,

    
   
447
      int expectedVersion)
445
  throws KeeperException {
448
  throws KeeperException {
446
    String encodedRegionName = regionInfo.getEncodedName();
449
    String encodedRegionName = regionInfo.getEncodedName();
447
    LOG.info("Processing region " + regionInfo.getRegionNameAsString() +
450
    LOG.info("Processing region " + regionInfo.getRegionNameAsString() +
448
      " in state " + data.getEventType());
451
      " in state " + data.getEventType());
449
    synchronized (regionsInTransition) {
452
    synchronized (regionsInTransition) {
[+20] [20] 64 lines
[+20] public void setRegionsToReopen(List <HRegionInfo> regions) {
514
        } else if (!serverManager.isServerOnline(sn)
517
        } else if (!serverManager.isServerOnline(sn)
515
            && (isOnDeadServer(regionInfo, deadServers)
518
            && (isOnDeadServer(regionInfo, deadServers)
516
                || regionInfo.isMetaRegion() || regionInfo.isRootRegion())) {
519
                || regionInfo.isMetaRegion() || regionInfo.isRootRegion())) {
517
          forceOffline(regionInfo, data);
520
          forceOffline(regionInfo, data);
518
        } else {
521
        } else {
519
          new OpenedRegionHandler(master, this, regionInfo, sn).process();
522
          new OpenedRegionHandler(master, this, regionInfo, sn, expectedVersion)

    
   
523
              .process();
520
        }
524
        }
521
        break;
525
        break;
522
      }
526
      }
523
    }
527
    }
524
  }
528
  }
[+20] [20] 66 lines
[+20] [+] private boolean isOnDeadServer(final HRegionInfo regionInfo,
591
   * Method is called when a state change is suspected for an unassigned node.
595
   * Method is called when a state change is suspected for an unassigned node.
592
   * <p>
596
   * <p>
593
   * This deals with skipped transitions (we got a CLOSED but didn't see CLOSING
597
   * This deals with skipped transitions (we got a CLOSED but didn't see CLOSING
594
   * yet).
598
   * yet).
595
   * @param data
599
   * @param data

    
   
600
   * @param expectedVersion
596
   */
601
   */
597
  private void handleRegion(final RegionTransitionData data) {
602
  private void handleRegion(final RegionTransitionData data, int expectedVersion) {
598
    synchronized(regionsInTransition) {
603
    synchronized(regionsInTransition) {
599
      if (data == null || data.getOrigin() == null) {
604
      if (data == null || data.getOrigin() == null) {
600
        LOG.warn("Unexpected NULL input " + data);
605
        LOG.warn("Unexpected NULL input " + data);
601
        return;
606
        return;
602
      }
607
      }
[+20] [20] 150 lines
[+20] private void handleRegion(final RegionTransitionData data) { private void handleRegion(final RegionTransitionData data, int expectedVersion) {
753
          // Handle OPENED by removing from transition and deleted zk node
758
          // Handle OPENED by removing from transition and deleted zk node
754
          regionState.update(RegionState.State.OPEN,
759
          regionState.update(RegionState.State.OPEN,
755
              data.getStamp(), data.getOrigin());
760
              data.getStamp(), data.getOrigin());
756
          this.executorService.submit(
761
          this.executorService.submit(
757
            new OpenedRegionHandler(master, this, regionState.getRegion(),
762
            new OpenedRegionHandler(master, this, regionState.getRegion(),
758
              data.getOrigin()));
763
              data.getOrigin(), expectedVersion));
759
          break;
764
          break;
760
      }
765
      }
761
    }
766
    }
762
  }
767
  }
763

    
   
768

   
[+20] [20] 141 lines
[+20] [+] private void handleHBCK(RegionTransitionData data) {
905
   */
910
   */
906
  @Override
911
  @Override
907
  public void nodeCreated(String path) {
912
  public void nodeCreated(String path) {
908
    if(path.startsWith(watcher.assignmentZNode)) {
913
    if(path.startsWith(watcher.assignmentZNode)) {
909
      try {
914
      try {
910
        RegionTransitionData data = ZKAssign.getData(watcher, path);
915
        Stat stat = new Stat();

    
   
916
        RegionTransitionData data = ZKAssign.getDataAndWatch(watcher, path, stat);
911
        if (data == null) {
917
        if (data == null) {
912
          return;
918
          return;
913
        }
919
        }
914
        handleRegion(data);
920
        handleRegion(data, stat.getVersion());
915
      } catch (KeeperException e) {
921
      } catch (KeeperException e) {
916
        master.abort("Unexpected ZK exception reading unassigned node data", e);
922
        master.abort("Unexpected ZK exception reading unassigned node data", e);
917
      }
923
      }
918
    }
924
    }
919
  }
925
  }
[+20] [20] 12 lines
[+20] public void nodeCreated(String path) {
932
   */
938
   */
933
  @Override
939
  @Override
934
  public void nodeDataChanged(String path) {
940
  public void nodeDataChanged(String path) {
935
    if(path.startsWith(watcher.assignmentZNode)) {
941
    if(path.startsWith(watcher.assignmentZNode)) {
936
      try {
942
      try {
937
        RegionTransitionData data = ZKAssign.getData(watcher, path);
943
        Stat stat = new Stat();

    
   
944
        RegionTransitionData data = ZKAssign.getDataAndWatch(watcher, path, stat);
938
        if (data == null) {
945
        if (data == null) {
939
          return;
946
          return;
940
        }
947
        }
941
        handleRegion(data);
948
        handleRegion(data, stat.getVersion());
942
      } catch (KeeperException e) {
949
      } catch (KeeperException e) {
943
        master.abort("Unexpected ZK exception reading unassigned node data", e);
950
        master.abort("Unexpected ZK exception reading unassigned node data", e);
944
      }
951
      }
945
    }
952
    }
946
  }
953
  }
[+20] [20] 1935 lines
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java
Revision 1179945 New Change
 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
Revision 1179945 New Change
 
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
Revision 1179945 New Change
 
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
New File
 
  1. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java: Loading...
  2. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java: Loading...
  3. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java: Loading...
  4. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java: Loading...
  5. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java: Loading...