[
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)