You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by selvaganesang <gi...@git.apache.org> on 2016/03/11 07:52:46 UTC

[GitHub] incubator-trafodion pull request: [TRAFODION-1886] Region Server f...

GitHub user selvaganesang opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/376

    [TRAFODION-1886] Region Server fails to come up at times with a NULL …

    …pointer exception in
    
    org.apache.hadoop.hbase.coprocessor.transactional.TrxRegionObserver class
    
    This issue is likely to happen when trafodion install fails for some reason

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/selvaganesang/incubator-trafodion trafodion-1886

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafodion/pull/376.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #376
    
----
commit af45e8cbd6610a51cefbdd036d245214dbdac3bb
Author: selvaganesang <se...@esgyn.com>
Date:   2016-03-11T06:47:52Z

    [TRAFODION-1886] Region Server fails to come up at times with a NULL pointer exception in
    org.apache.hadoop.hbase.coprocessor.transactional.TrxRegionObserver class
    
    This issue is likely to happen when trafodion install fails for some reason

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1886] Region Server f...

Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/376#discussion_r55860621
  
    --- Diff: core/sqf/src/seatrans/hbase-trx/src/main/java/org/apache/hadoop/hbase/coprocessor/transactional/SplitBalanceHelper.java ---
    @@ -304,11 +304,13 @@ protected void zkCleanup() {
               List<String> trafTables = ZKUtil.listChildrenNoWatch(zkw, zSplitBalPathNoSlash);
               List<String> hbaseTables = ZKUtil.listChildrenNoWatch(zkw, SplitBalanceHelper.zkTable);
               
    -          for(String tableName : trafTables) {
    +          if(trafTables != null && hbaseTables != null) {
    --- End diff --
    
    you are right Selva, thanks to help me understand this code.
    It now looks good for me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1886] Region Server f...

Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/376#discussion_r55844614
  
    --- Diff: core/sqf/src/seatrans/hbase-trx/src/main/java/org/apache/hadoop/hbase/coprocessor/transactional/SplitBalanceHelper.java ---
    @@ -304,11 +304,13 @@ protected void zkCleanup() {
               List<String> trafTables = ZKUtil.listChildrenNoWatch(zkw, zSplitBalPathNoSlash);
               List<String> hbaseTables = ZKUtil.listChildrenNoWatch(zkw, SplitBalanceHelper.zkTable);
               
    -          for(String tableName : trafTables) {
    +          if(trafTables != null && hbaseTables != null) {
    --- End diff --
    
    There is a possibility that trafTables is not null but hbaseTables is null, or vice versa. In this case, the cleanup will leave something.
    I am not quite clear, but I think this method is to clean up every leftover znodes. So the '&&' may not be proper here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1886] Region Server f...

Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/376#discussion_r55854436
  
    --- Diff: core/sqf/src/seatrans/hbase-trx/src/main/java/org/apache/hadoop/hbase/coprocessor/transactional/SplitBalanceHelper.java ---
    @@ -304,11 +304,13 @@ protected void zkCleanup() {
               List<String> trafTables = ZKUtil.listChildrenNoWatch(zkw, zSplitBalPathNoSlash);
               List<String> hbaseTables = ZKUtil.listChildrenNoWatch(zkw, SplitBalanceHelper.zkTable);
               
    -          for(String tableName : trafTables) {
    +          if(trafTables != null && hbaseTables != null) {
    --- End diff --
    
    trafTables should be subset of hbaseTables.  If it is just a plain hbase table, do you think it should be the responsibility of this class to cleanup the zk nodes. If so, we might need to re-factor this code. If not, this change should be fine though it is bit more defensive.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1886] Region Server f...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafodion/pull/376


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---