[ 
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886491#comment-15886491
 ] 

Enis Soztutar commented on HBASE-17680:
---------------------------------------

Thanks Ted for the updated patch. 
- No commented out code please.  
- Even if the include directories are different, we cannot hard code the java 
location like this in the buck and make builds. 
- We are using c++14 and smart pointers exclusively unless there is a reason 
not to. So below: 
{code} 
+  MiniCluster* mini;
{code} 
should be a {{std::unique_ptr<MiniCluster>}} and constructed via 
{{std::make_unique<MiniCluster>()}}; {{JNIEnv* env}} are exceptions since it is 
interacting with the C code. But we can also encapsulate those with 
unique_ptr's after construction. 
- Indentation is off for some of the methods in mini-cluster.cc. You can run 
bin/format-code.sh to auto-correct the format. 
- {{WriteConf}} I think writing conf to a file, and reading it back should not 
be necessary at all. Unlike the java counterpart, Configuration is much simpler 
in the cpp side. You can either load it via HBaseConfigurationLoader which 
reads from the XML files, or you can manually create a Configuration object 
which does not know anything about XML. You can then just populate relevant 
properties from the Java configuration. So you should be able to iterate via 
the Java Conf object, and set values in the cpp Conf object without having to 
go through the file serialization. 
- We still need to destroy the JVM after we are done. Were you able to find out 
the segfault issues? 
- Rename some of these according to the conventions: 
{code}
+    void setup();   -> Setup() 
+    jobject getHTU();   -> htu()  // field accessor methods can be named with 
lower case. 
+    JNIEnv* getEnv();  -> env()  
+    jbyteArray str_to_byte_array(char *str);  -> StrToByteChar() 
+    jobject getAdmin();   -> admin
{code}
All fields of class should have names ending with {{_}}. Like {{putCtor_}}, 
{{env_}}, etc. 
- Remove these debug statements? 
{code}
 LOG(INFO)
{code}

> Run mini cluster through JNI in tests
> -------------------------------------
>
>                 Key: HBASE-17680
>                 URL: https://issues.apache.org/jira/browse/HBASE-17680
>             Project: HBase
>          Issue Type: Sub-task
>            Reporter: Ted Yu
>            Assignee: Ted Yu
>         Attachments: 17680.v14.txt, 17680.v17.txt, 17680.v1.txt, 
> 17680.v3.txt, 17680.v8.txt
>
>
> Currently tests start local hbase cluster through hbase shell.
> There is less control over the configuration of the local cluster this way.
> This issue would replace hbase shell with JNI interface to mini cluster.
> We would have full control over the cluster behavior.
> Thanks to [~devaraj] who started this initiative.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to