[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15898321#comment-15898321
]
Enis Soztutar commented on HBASE-17680:
---
Remove this:
{code}
- hbase::Configuration conf;
+ //hbase::Configuration conf;
{code}
Please commit to branch afterwards. Thanks.
> 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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.txt, 17680.v23.txt, 17680.v26.txt,
> 17680.v27.txt, 17680.v28.txt, 17680.v29.txt, 17680.v30.txt, 17680.v31.txt,
> 17680.v33.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15898233#comment-15898233
]
Enis Soztutar commented on HBASE-17680:
---
Thanks Ted for the changes.
- Please remove ALL commented out code. This is the third time for the same
comment.
{code}
+ //hbase::Configuration conf;
+ // hbase::TestUtil *test_util = new hbase::TestUtil(2,
ClientTest::kDefHBaseConfPath.c_str());
{code}
- Refactor {{addColMid_}} -> {{add_col_mid_}}. and {{setConfMid_}}.
- These methods should take {{const std::string&}} instead of {{char *}}.
{code}
{{void WriteConf(jobject conf, const char *filepath);}}
+jbyteArray MiniCluster::StrToByteChar(const char *str) {
+TestUtil::TestUtil(int servers, const char *confPath)
{code}
- Remove:
{code}
+ LOG(INFO) << "retrieving " << key;
+ LOG(INFO) << "Got string " << val;
{code}
and maybe:
{code}
+ LOG(INFO) << "retrieved port " << port;
{code}
- Change these to check for NULL instead:
{code}
if (put == 0) {
+ if (mid == 0) {
+ if (n == 0) return NULL;
{code}
- I'll do the changes for client-test for not depending on the WriteConf() as a
follow up.
> 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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.txt, 17680.v23.txt, 17680.v26.txt,
> 17680.v27.txt, 17680.v28.txt, 17680.v29.txt, 17680.v30.txt, 17680.v31.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15898154#comment-15898154 ] Ted Yu commented on HBASE-17680: "buck test --all" passed for patch v31. > 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.v18.txt, > 17680.v1.txt, 17680.v20.txt, 17680.v22.txt, 17680.v23.txt, 17680.v26.txt, > 17680.v27.txt, 17680.v28.txt, 17680.v29.txt, 17680.v30.txt, 17680.v31.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15895347#comment-15895347
]
Enis Soztutar commented on HBASE-17680:
---
Can you please remove all commented out code. There are still references.
Also, in multiple places, you should remove these comments as well:
{code}
// The connection stays open and we don't want that.
// So we are stopping the connection.
// We can remove this once we have fixed the folly part
- delete test_util;
{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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.txt, 17680.v23.txt, 17680.v26.txt,
> 17680.v27.txt, 17680.v28.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15893716#comment-15893716
]
Ted Yu commented on HBASE-17680:
Ran valgrind again.
libjvm.so still showed up but there was no occurrence of MiniCluster.
This was from last run:
{code}
==21686==by 0x426260: hbase::MiniCluster::CreateVM(JavaVM_**)
(mini-cluster.cc:73)
==21686==by 0x426B5A: hbase::MiniCluster::Setup() (mini-cluster.cc:115)
{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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.txt, 17680.v23.txt, 17680.v26.txt,
> 17680.v27.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15893389#comment-15893389 ] Ted Yu commented on HBASE-17680: bq. This behavior coming from the existing code How about addressing the last comment in another issue (since it is pre-existing behavior) ? > 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.v18.txt, > 17680.v1.txt, 17680.v20.txt, 17680.v22.txt, 17680.v23.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15893355#comment-15893355
]
Ted Yu commented on HBASE-17680:
w.r.t. lint warnings, there are a lot of existing ones.
e.g.
{code}
serde/region-info.h:24: Found C system header after other header. Should be:
region-info.h, c system, c++ system, other. [build/include_order] [4]
{code}
I am trying to get rid of ones in new files.
> 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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.txt, 17680.v23.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15893228#comment-15893228
]
Enis Soztutar commented on HBASE-17680:
---
I've checked the v23 patch. You did not address these previous comments:
- There is still a very large number of these kind of warnings:
{code}
core/filter-test.cc:65:39: warning: ISO C++11 does not allow conversion from
string literal to 'char *' [-Wwritable-strings]
FilterTest::test_util_->CreateTable("t", "d");
{code}
You should use std::string everywhere, except when you are sending it to the
JVM object. So, the method signatures like:
{code}
jobject MiniCluster::CreateTable(char *tblNam, char *familyName) {
{code}
should be always taking std::string, or better {{const std::string&}}. We want
to minimize the {{char *}} usage to be only happening in the JVM layer. You can
use string::c_str().
- Also this warning:
{code}
test-util/mini-cluster.cc:260:1: warning: control reaches end of non-void
function [-Wreturn-type]
{code}
- Some of the fields in MiniCluster does not end with {{_}}. Examples are
{{cluster}}, {{jvm}}, etc.
- As per above, you should not need WriteConf() method at all. Please construct
a Configuration object and populate it via calling {{Configuration::Set()}}
from the java-level Configuration object. You seem to be doing that in
TestUtil, so please remove the WriteConf and setting the path, etc. Also return
std::string in GetConf(), etc.
- We use {{make lint}} to run the cpplint script. Can you please address these
new warnings as well:
{code}
test-util/test-util.cc:65: Add #include for make_unique<>
[build/include_what_you_use] [4]
test-util/test-util.cc:77: Could not find a newline character at the end of
the file. [whitespace/ending_newline] [5]
Done processing test-util/test-util.cc
test-util/test-util.h:84: Add #include for shared_ptr<>
[build/include_what_you_use] [4]
Done processing test-util/test-util.h
test-util/mini-cluster.cc:22: "glog/logging.h" already included at
test-util/mini-cluster.cc:21 [build/include] [4]
test-util/mini-cluster.cc:25: Found C system header after C++ system header.
Should be: mini-cluster.h, c system, c++ system, other. [build/include_order]
[4]
test-util/mini-cluster.cc:26: Found C system header after C++ system header.
Should be: mini-cluster.h, c system, c++ system, other. [build/include_order]
[4]
test-util/mini-cluster.cc:70: Missing space before { [whitespace/braces] [5]
test-util/mini-cluster.cc:75: Using C-style cast. Use reinterpret_cast(...) instead [readability/casting] [4]
test-util/mini-cluster.cc:227: Using C-style cast. Use reinterpret_cast(...) instead [readability/casting] [4]
test-util/mini-cluster.cc:313: Add #include for string
[build/include_what_you_use] [4]
Done processing test-util/mini-cluster.cc
test-util/mini-cluster.h:28: Include the directory when naming .h files
[build/include] [4]
test-util/mini-cluster.h:35: Add #include for string
[build/include_what_you_use] [4]
{code}
- GetConf() is overloaded. For the one returning a configuration value, you
should name it GetConfValue().
- Should the jobject cluster be a field in MiniCluster. No need to pass it to
methods like GetConf() I think. In StartCluster(), you can save it in the
field.
- This behavior coming from the existing code:
{code}
Creating a TestUtil will spin up a cluster with numRegionServers region servers.
{code}
But I think it is wrong. Can we change it so that TestUtil ctor does not start
the cluster, but you have to manually call StartCluster. I think it is better
this way, because in theory you should be able use the test util without the
cluster.
Other then these, the patch looks good.
> 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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.txt, 17680.v23.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15889492#comment-15889492
]
Ted Yu commented on HBASE-17680:
Noticed the following from valgrind output:
{code}
==21686== Invalid write of size 4
==21686==at 0x91BBBCC: ???
==21686==by 0x91AFE53: ???
==21686==by 0x91AFFFC: ???
==21686==by 0x91AFFFC: ???
==21686==by 0x91A84E6: ???
==21686==by 0x5497D7A: ??? (in
/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so)
==21686==by 0x5496643: ??? (in
/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so)
==21686==by 0x5496C07: ??? (in
/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so)
==21686==by 0x584007D: ??? (in
/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so)
==21686==by 0x54BEE41: JNI_CreateJavaVM (in
/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/libjvm.so)
==21686==by 0x426260: hbase::MiniCluster::CreateVM(JavaVM_**)
(mini-cluster.cc:73)
==21686==by 0x426B5A: hbase::MiniCluster::Setup() (mini-cluster.cc:115)
==21686== Address 0xffefeb710 is on thread 1's stack
==21686== 81920 bytes below stack pointer
{code}
Line 73 is this line:
{code}
rv = JNI_CreateJavaVM(jvm, (void **)_, );
{code}
Where jvm is JavaVM **jvm
Without symbols from libjvm.so, it is not easy to know what the actual error is.
> 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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888972#comment-15888972
]
Ted Yu commented on HBASE-17680:
After installing valgrind, I got (using above valgrind command):
{code}
valgrind: the 'impossible' happened:
LibVEX called failure_exit().
host stacktrace:
==65==at 0x38083F48: ??? (in /usr/lib/valgrind/memcheck-amd64-linux)
==65==by 0x38084064: ??? (in /usr/lib/valgrind/memcheck-amd64-linux)
{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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888961#comment-15888961
]
Enis Soztutar commented on HBASE-17680:
---
I was able to run the tests successfully.
I checked the valgrind output with client-test like this:
{code}
valgrind buck-out/gen/core/client-test 2>&1 | tee val.out
{code}
It is showing thousands of illegal memory accesses for the JVM now. I am not
sure whether these are related to how we pass the arguments.
> 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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1515#comment-1515
]
Enis Soztutar commented on HBASE-17680:
---
Nevermind, I think it is due to the {{mvn install}} command being run from
outside the docker (because .m2 paths are not docker paths). Let me build
inside the docker.
> 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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1514#comment-1514
]
Enis Soztutar commented on HBASE-17680:
---
It is failing like this:
{code}
STANDARD ERR
PASS<100ms 3 Passed 0 Skipped 0 Failed //core:get-test
PASS<100ms 17 Passed 0 Skipped 0 Failed
//core:hbase_configuration-test
FAIL<100ms 0 Passed 0 Skipped 1 Failed //core:location-cache-test
FAILURE //core:location-cache-test main: test program aborted before finishing
STANDARD OUT
Running main() from gmock_main.cc
[==] Running 3 tests from 1 test case.
[--] Global test environment set-up.
[--] 3 tests from LocationCacheTest
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0228 20:25:37.822675 30820 mini-cluster.cc:57] set clspath 0
I0228 20:25:37.890406 30820 mini-cluster.cc:77] Launched JVM!
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888783#comment-15888783
]
Ted Yu commented on HBASE-17680:
{code}
PASS<100ms 2 Passed 0 Skipped 0 Failed
//connection:connection-pool-test
PASS<100ms 3 Passed 0 Skipped 0 Failed
//serde:client-deserializer-test
PASS<100ms 4 Passed 0 Skipped 0 Failed //serde:client-serializer-test
PASS<100ms 1 Passed 0 Skipped 0 Failed
//serde:region-info-deserializer-test
PASS<100ms 4 Passed 0 Skipped 0 Failed //serde:server-name-test
PASS<100ms 3 Passed 0 Skipped 0 Failed //serde:table-name-test
PASS<100ms 3 Passed 0 Skipped 0 Failed //serde:zk-deserializer-test
PASS<100ms 1 Passed 0 Skipped 0 Failed //utils:user-util-test
PASS<100ms 7 Passed 0 Skipped 0 Failed //core:cell-test
PASS 3.8s 6 Passed 0 Skipped 0 Failed //core:client-test
PASS<100ms 5 Passed 0 Skipped 0 Failed //core:configuration-test
PASS<100ms 3 Passed 0 Skipped 0 Failed //core:get-test
PASS 247ms 17 Passed 0 Skipped 0 Failed
//core:hbase_configuration-test
PASS<100ms 2 Passed 0 Skipped 0 Failed //core:request_converter-test
PASS<100ms 2 Passed 0 Skipped 0 Failed //core:result-test
PASS<100ms 5 Passed 0 Skipped 0 Failed //core:scan-test
PASS<100ms 2 Passed 0 Skipped 0 Failed //core:time_range-test
RESULTS FOR //core:location-cache-test
PASS 5.1s 3 Passed 0 Skipped 0 Failed //core:location-cache-test
TESTS PASSED
PASS 2.7s 2 Passed 0 Skipped 0 Failed //core:filter-test
{code}
Which test(s) failed for you ?
> 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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15888736#comment-15888736
]
Enis Soztutar commented on HBASE-17680:
---
Are you gonna address the remaining review comments?
The command {{buck test --all}} still fails, BTW.
> 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.v18.txt,
> 17680.v1.txt, 17680.v20.txt, 17680.v22.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15886687#comment-15886687
]
Enis Soztutar commented on HBASE-17680:
---
I am also getting a lot of these warnings:
{code}
core/filter-test.cc:65:39: warning: ISO C++11 does not allow conversion from
string literal to 'char *' [-Wwritable-strings]
FilterTest::test_util_->CreateTable("t", "d");
{code}
Also this fails:
{code}
buck test --all
{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.v18.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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}} and constructed via
{{std::make_unique()}}; {{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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15886091#comment-15886091 ] Ted Yu commented on HBASE-17680: Performed "make" under hbase-native-client directory. The build passed. > 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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15883100#comment-15883100 ] Ted Yu commented on HBASE-17680: w.r.t. Makefile, on docker the library we link with is under /usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server where /usr/lib/jvm/java-8-openjdk-amd64/ is JAVA_HOME On Mac, JAVA_HOME is /Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home But there is no /Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home/jre/lib/amd64/server directory. > 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.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15883050#comment-15883050
]
Ted Yu commented on HBASE-17680:
Performed "mvn install -DskipTests" on docker (running on Mac):
{code}
[INFO] Apache HBase - Shaded - Server . SUCCESS [ 14.793 s]
[INFO] Apache HBase - Archetypes .. SUCCESS [ 0.220 s]
[INFO] Apache HBase - Exemplar for hbase-client archetype . SUCCESS [ 20.397 s]
[INFO] Apache HBase - Exemplar for hbase-shaded-client archetype SUCCESS [
23.015 s]
[INFO] Apache HBase - Archetype builder ... SUCCESS [ 42.005 s]
[INFO]
[INFO] BUILD SUCCESS
[INFO]
[INFO] Total time: 16:33 min
[INFO] Finished at: 2017-02-24T16:33:22+00:00
{code}
On Mac, same command took:
{code}
[INFO]
[INFO] BUILD SUCCESS
[INFO]
[INFO] Total time: 01:42 min
[INFO] Finished at: 2017-02-24T08:38:48-08:00
{code}
Building C++ code using buck is very fast.
We should leverage the agility of C++ in docker environment.
> 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.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15882164#comment-15882164 ] Devaraj Das commented on HBASE-17680: - I think for now it's fine to make the whole thing work via Buck (that assumes docker). For the Makefile based builds we can read JAVA_HOME, etc. It makes sense to write the Java wrapper for the HTU that combines various operations, and write a thinner layer for the JNI. > 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.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15881865#comment-15881865 ] Ted Yu commented on HBASE-17680: w.r.t. compiler_flags, on Mac, there is no $JAVA_HOME/include/linux directory. While on docker VM, $JAVA_HOME/include/linux is needed for including jni.h > 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.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15881683#comment-15881683 ] Enis Soztutar commented on HBASE-17680: --- bq. Not sure of why we should create NativeTestingUtility - it would be wrapper around HTU and is written in Java, not C++. The difference is that you do not need to have to code in the JNI layer and the code will be significantly less, as well as more maintainable. The only thing you need to pass back is a return results. For example, in the tests, usually we create a table with a given family, and put some data. You just need two methods which directly forward the request to the java class in this case, no need to import Put object, Table object, Admin, etc. > 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.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15881670#comment-15881670
]
Enis Soztutar commented on HBASE-17680:
---
Couple of more comments:
- At least some of the methods are C-style, and rest are C++ style. We can
just stick with C++ and encapsulate everything inside the class.
- Also, you can use much nicer ifstream instead of fopen / flose, and use
std::string instead of mallocing c-strings. You can use {{string::c_str()}} to
pass back to the JVM args.
- Instead of {{(*env).Foo()}}, you should use {{env->Foo()}}.
> 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.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15881652#comment-15881652 ] Ted Yu commented on HBASE-17680: Not sure of why we should create NativeTestingUtility - it would be wrapper around HTU and is written in Java, not C++. When native client calls NativeTestingUtility, same translation needs to be carried out. bq. maybe merge mini-cluster to test-util.cc I can do the above. > 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.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15881646#comment-15881646
]
Enis Soztutar commented on HBASE-17680:
---
Thanks for working on this [~devaraj] and [~ted_yu].
This helps a lot in reducing the test execution time as well, since we do not
have to wait for java and jruby instantiation multiple times. If I interpret
the above, the run times goes from ~60 secs to <10 secs. Plus, we need the
multi-regionserver capabilities as well as killing servers, etc for the native
client tests which are already available in the mini hbase cluster.
For the patch:
- Can we move the mini-cluster to be under {{test-util}} module? It does not
belong in core.
- I really like the way that we can call any method from HTU, Connection,
Admin, Table etc, but most of the code in mini-cluster.cc is unnecessarily in
the cpp side. Can we do this instead. Create a NativeTestingUtility.java in
hbase-server, and also maybe merge mini-cluster to test-util.cc. The java
counter-part will contain almost all of the code that we need to invoke from
native (like writeConf(), create_table(), etc in the mini-cluster. This will be
better, because almost all of the code will be in the Java side which is way
more maintainable. mini-cluster.cc will just call the corresponding java method
in NativeTestingUtility class.
- Notice that methods like tablePut, create_table, etc will be dramatically
simpler.
- Method names in cpp usually use camel case with initial upper case, so
methods like {{start_cluster()}} should be named {{StartCluster()}}.
- You also need to call {{DestroyJavaVM()}} once the testing is done. Maybe
add it to Shutdown() or something.
- I think this {{+compiler_flags = ['-I',
'/usr/lib/jvm/java-8-openjdk-amd64/include/', '-I',
'/usr/lib/jvm/java-8-openjdk-amd64/include/linux'],
}} assumes that we are running in the docker container. Our builds are weird,
where there is the buck based build and the make based build. However, both are
supposed to work without the docker env. So maybe we need to source the
JAVA_HOME.
> 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.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[
https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15879524#comment-15879524
]
Ted Yu commented on HBASE-17680:
The two tests where shell command is replaced:
{code}
RESULTS FOR //core:location-cache-test
PASS 5.2s 3 Passed 0 Skipped 0 Failed //core:location-cache-test
TESTS PASSED
RESULTS FOR //core:client-test
PASS 2.8s 6 Passed 0 Skipped 0 Failed //core:client-test
TESTS PASSED
{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.v1.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15879481#comment-15879481 ] stack commented on HBASE-17680: --- We already write a cache of classpath at ./target/cached_classpath.txt > 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.v1.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)
[jira] [Commented] (HBASE-17680) Run mini cluster through JNI in tests
[ https://issues.apache.org/jira/browse/HBASE-17680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15879430#comment-15879430 ] Ted Yu commented on HBASE-17680: docker hardcodes CLASSPATH as "/usr/src/buck/build/bootstrapper/bootstrapper.jar" Workaround is to embed the output of "bin/hbase classpath" command in a file (/tmp/clspath) which would be read by MiniCluster#create_vm() > 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.v1.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)
