Re: Review Request 65647: ATLAS-2444: HDFS namenode federation support

2018-02-14 Thread Madhan Neethiraj 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65647/#review197586
---


Ship it!




Ship It!

- Madhan Neethiraj


On Feb. 14, 2018, 9:34 p.m., Apoorv Naik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65647/
> ---
> 
> (Updated Feb. 14, 2018, 9:34 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, and Sarath 
> Subramanian.
> 
> 
> Bugs: ATLAS-2444
> https://issues.apache.org/jira/browse/ATLAS-2444
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> See ATLAS-2444
> 
> 
> Diffs
> -
> 
>   
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java
>  7cca48f11 
>   addons/models/1000-Hadoop/1020-fs_model.json ff17261bc 
>   
> addons/storm-bridge/src/main/java/org/apache/atlas/storm/hook/StormAtlasHook.java
>  57fc7a1fd 
>   common/pom.xml a7ab6a312 
>   common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java 
> PRE-CREATION 
>   
> common/src/test/java/org/apache/atlas/utils/HdfsNameServiceResolverTest.java 
> PRE-CREATION 
>   common/src/test/resources/atlas-log4j.xml PRE-CREATION 
>   common/src/test/resources/hdfs-site.xml PRE-CREATION 
>   pom.xml c15e0dad7 
> 
> 
> Diff: https://reviews.apache.org/r/65647/diff/3/
> 
> 
> Testing
> ---
> 
> mvn clean package executes successfully.
> 
> The modified war was tested on a multi-node cluster with one nameServiceId 
> and 2 NameNodes, the required information was captured successfully in the 
> entity when an external table was created from beeline (hive context)
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 65647: ATLAS-2444: HDFS namenode federation support

2018-02-14 Thread Apoorv Naik 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65647/
---

(Updated Feb. 14, 2018, 9:34 p.m.)


Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, and Sarath 
Subramanian.


Changes
---

Refactoring around caching, moved all Constants into Resolver class and updated 
the path.replace to StringUtils.replaceOnce(...) to avoid replacing other values


Bugs: ATLAS-2444
https://issues.apache.org/jira/browse/ATLAS-2444


Repository: atlas


Description
---

See ATLAS-2444


Diffs (updated)
-

  
addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java
 7cca48f11 
  addons/models/1000-Hadoop/1020-fs_model.json ff17261bc 
  
addons/storm-bridge/src/main/java/org/apache/atlas/storm/hook/StormAtlasHook.java
 57fc7a1fd 
  common/pom.xml a7ab6a312 
  common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java 
PRE-CREATION 
  common/src/test/java/org/apache/atlas/utils/HdfsNameServiceResolverTest.java 
PRE-CREATION 
  common/src/test/resources/atlas-log4j.xml PRE-CREATION 
  common/src/test/resources/hdfs-site.xml PRE-CREATION 
  pom.xml c15e0dad7 


Diff: https://reviews.apache.org/r/65647/diff/3/

Changes: https://reviews.apache.org/r/65647/diff/2-3/


Testing
---

mvn clean package executes successfully.

The modified war was tested on a multi-node cluster with one nameServiceId and 
2 NameNodes, the required information was captured successfully in the entity 
when an external table was created from beeline (hive context)


Thanks,

Apoorv Naik

Re: Review Request 65647: ATLAS-2444: HDFS namenode federation support

2018-02-14 Thread Madhan Neethiraj 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65647/#review197575
---




addons/models/1000-Hadoop/patches/005-hdfs_path_add_nameServiceId_attribute.json
Lines 1 (patched)


Given migration from earlier version of Atlas to 1.0 will be via 
export/import, I think it will be better to add 'nameServiceId' attribute 
directly in hdfs_path entity-def - instead of adding via a patch.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 43 (patched)


Caching all the URLs handled by HdfsNameServiceResolver may result in the 
map to grow very large and take up a lot of memory. Consider avoding this cache.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 56 (patched)


Consider using int for paramter 'port'.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 98 (patched)


This would replace all occurances of "uri.getAuthority()" in the URL with 
nsId - which may not be desired. Only the first instance should be replaced:

hdfs://myhost/d1/d2/myhost.txt ==> hdfs://ns1/d1/d2/ns1.txt.

Please review and update.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 230 (patched)


Why not directly add these constants in outer class? Just as 
'NS_ID_FOR_PATH', 'PATH_WITH_NSID' were added above.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 231 (patched)


I think the default port can be configured in HDFS configuration. Instead 
of hardcoding "8020" here, read default port from HDFS configuration.


- Madhan Neethiraj


On Feb. 14, 2018, 6:36 p.m., Apoorv Naik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65647/
> ---
> 
> (Updated Feb. 14, 2018, 6:36 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, and Sarath 
> Subramanian.
> 
> 
> Bugs: ATLAS-2444
> https://issues.apache.org/jira/browse/ATLAS-2444
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> See ATLAS-2444
> 
> 
> Diffs
> -
> 
>   
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java
>  7cca48f11 
>   
> addons/models/1000-Hadoop/patches/005-hdfs_path_add_nameServiceId_attribute.json
>  PRE-CREATION 
>   
> addons/storm-bridge/src/main/java/org/apache/atlas/storm/hook/StormAtlasHook.java
>  57fc7a1fd 
>   common/pom.xml a7ab6a312 
>   common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java 
> PRE-CREATION 
>   
> common/src/test/java/org/apache/atlas/utils/HdfsNameServiceResolverTest.java 
> PRE-CREATION 
>   common/src/test/resources/atlas-log4j.xml PRE-CREATION 
>   common/src/test/resources/hdfs-site.xml PRE-CREATION 
>   pom.xml c15e0dad7 
> 
> 
> Diff: https://reviews.apache.org/r/65647/diff/2/
> 
> 
> Testing
> ---
> 
> mvn clean package executes successfully.
> 
> The modified war was tested on a multi-node cluster with one nameServiceId 
> and 2 NameNodes, the required information was captured successfully in the 
> entity when an external table was created from beeline (hive context)
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 65647: ATLAS-2444: HDFS namenode federation support

2018-02-14 Thread Apoorv Naik 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65647/
---

(Updated Feb. 14, 2018, 6:36 p.m.)


Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, and Sarath 
Subramanian.


Changes
---

Addressed review comments.


Bugs: ATLAS-2444
https://issues.apache.org/jira/browse/ATLAS-2444


Repository: atlas


Description
---

See ATLAS-2444


Diffs (updated)
-

  
addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java
 7cca48f11 
  
addons/models/1000-Hadoop/patches/005-hdfs_path_add_nameServiceId_attribute.json
 PRE-CREATION 
  
addons/storm-bridge/src/main/java/org/apache/atlas/storm/hook/StormAtlasHook.java
 57fc7a1fd 
  common/pom.xml a7ab6a312 
  common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java 
PRE-CREATION 
  common/src/test/java/org/apache/atlas/utils/HdfsNameServiceResolverTest.java 
PRE-CREATION 
  common/src/test/resources/atlas-log4j.xml PRE-CREATION 
  common/src/test/resources/hdfs-site.xml PRE-CREATION 
  pom.xml c15e0dad7 


Diff: https://reviews.apache.org/r/65647/diff/2/

Changes: https://reviews.apache.org/r/65647/diff/1-2/


Testing
---

mvn clean package executes successfully.

The modified war was tested on a multi-node cluster with one nameServiceId and 
2 NameNodes, the required information was captured successfully in the entity 
when an external table was created from beeline (hive context)


Thanks,

Apoorv Naik

Re: Review Request 65647: ATLAS-2444: HDFS namenode federation support

2018-02-13 Thread Madhan Neethiraj 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65647/#review197481
---




addons/storm-bridge/src/main/java/org/apache/atlas/storm/hook/StormAtlasHook.java
Lines 222 (patched)


Consider replacing 'config.get("HdfsBolt.fsUrl") + hdfsUri' with 
'hdfsPathStr' - which was assigned in previous line.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 127 (patched)


shouldn't "ret = uri.getHost()" be "ret = 
reverseNameServiceMapping.get(uri.getHost())"?


- Madhan Neethiraj


On Feb. 14, 2018, 4:49 a.m., Apoorv Naik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65647/
> ---
> 
> (Updated Feb. 14, 2018, 4:49 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, and Sarath 
> Subramanian.
> 
> 
> Bugs: ATLAS-2444
> https://issues.apache.org/jira/browse/ATLAS-2444
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> See ATLAS-2444
> 
> 
> Diffs
> -
> 
>   
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java
>  7cca48f11 
>   
> addons/models/1000-Hadoop/patches/005-hdfs_path_add_nameServiceId_attribute.json
>  PRE-CREATION 
>   
> addons/storm-bridge/src/main/java/org/apache/atlas/storm/hook/StormAtlasHook.java
>  57fc7a1fd 
>   common/pom.xml a7ab6a312 
>   common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java 
> PRE-CREATION 
>   
> common/src/test/java/org/apache/atlas/utils/HdfsNameServiceResolverTest.java 
> PRE-CREATION 
>   common/src/test/resources/atlas-log4j.xml PRE-CREATION 
>   common/src/test/resources/hdfs-site.xml PRE-CREATION 
>   pom.xml c15e0dad7 
> 
> 
> Diff: https://reviews.apache.org/r/65647/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean package executes successfully.
> 
> The modified war was tested on a multi-node cluster with one nameServiceId 
> and 2 NameNodes, the required information was captured successfully in the 
> entity when an external table was created from beeline (hive context)
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Review Request 65647: ATLAS-2444: HDFS namenode federation support

2018-02-13 Thread Apoorv Naik 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65647/
---

Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, and Sarath 
Subramanian.


Bugs: ATLAS-2444
https://issues.apache.org/jira/browse/ATLAS-2444


Repository: atlas


Description
---

See ATLAS-2444


Diffs
-

  
addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java
 7cca48f11 
  
addons/models/1000-Hadoop/patches/005-hdfs_path_add_nameServiceId_attribute.json
 PRE-CREATION 
  
addons/storm-bridge/src/main/java/org/apache/atlas/storm/hook/StormAtlasHook.java
 57fc7a1fd 
  common/pom.xml a7ab6a312 
  common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java 
PRE-CREATION 
  common/src/test/java/org/apache/atlas/utils/HdfsNameServiceResolverTest.java 
PRE-CREATION 
  common/src/test/resources/atlas-log4j.xml PRE-CREATION 
  common/src/test/resources/hdfs-site.xml PRE-CREATION 
  pom.xml c15e0dad7 


Diff: https://reviews.apache.org/r/65647/diff/1/


Testing
---

mvn clean package executes successfully.

The modified war was tested on a multi-node cluster with one nameServiceId and 
2 NameNodes, the required information was captured successfully in the entity 
when an external table was created from beeline (hive context)


Thanks,

Apoorv Naik