[jira] [Comment Edited] (BIGTOP-1493) Add smoke test for tachyon

["Konstantin Boudnik (JIRA)" <ji...@apache.org>]

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

Konstantin Boudnik edited comment on BIGTOP-1493 at 11/14/14 2:04 AM:
----------------------------------------------------------------------

bq. this look awkward: hadoop is normally expected to be found under $HADOOP_HOME/bin rather than HADOOP_MAPREDUCE_HOME

I agree that its awkward.  I need the examples jar and I need the hadoop command. I know that the other smoke tests expect hadoop to be in the path, so I can do that. I could also require HADOOP_HOME (or should it be prefix...). That shouldn't be too bad to require since you can just source /etc/default/hadoop for these.  Kinda tempted to go with HADOOP_HOME but since the other tests assume hadoop is in the path, think that that would be the best way to go.  Thoughts?

bq. I'd prefer an assert here (and in doFirst method) instead of using an exception for control flow. Besides throwing and not catching an exception is a bad idea

The only throw that is explicit in the path is the NPE. Is the statement for the NPE? If so I can make it a assert.  The reason that I felt that it was ok to throw and not have any handling in this case is that gradle tries to enforce that these values are provided. So since gradle is verifying that they are never null, assert does make sense here.

bq. if itest depends on asm (which I don't recall) shall we fix it in iTest instead?
+  // looks like itest depends on this but doesn't get picked up properly
+  // assuming it gets this from hadoop which uses 3.2
+  testCompile 'asm:asm:3.2'

Agree, its way better to fix this in itest if that is the root cause. I rather that be a different JIRA, since its unknown what the effort to fix it would be.

bq. Also, we are using enforcer plugin in Maven, so I am wondering if it's possible to do something similar at the top level of Gradle build for smoke-tests? It seems to be a feature that will be required all over the tests. Thoughts?

Looking at the enforcer for `bigtop-tests/test-execution/common/pom.xml`, this just verifies that the hadoop env properties are set. Currently the gradle tests do the same with the below code in the build file.

{code}
+test.doFirst {
+  checkEnv([
+    "TACHYON_HOME",
+    "TACHYON_MASTER_ADDRESS",
+    "HADOOP_MAPRED_HOME"
+    ])
+}
{code}


bq. I know that it is done everywhere in the smokes
+log4j.rootLogger=TRACE, A1
but shall we approach it similarly to how we've done it before in maven executors poms? E.g. setting the log level to INFO by default via a property available for all the tests and changeable in the runtime? See bigtop-tests/test-execution/common/pom.xml for a reference. If you think it is worthy - could open a JIRA so we can improve this later? In the meanwhile, it isn't a blocker for this patch"

Its simple to do something like the below in log4j. Let me know if this is ok.  Can set -Dbigtop.default.logger=INFO,A1 to have it changed.

{code}
bigtop.default.logger=TRACE,A1
log4j.rootLogger=${bigtop.default.logger}
{code}


was (Author: dcapwell):
"this look awkward: hadoop is normally expected to be found under $HADOOP_HOME/bin rather than HADOOP_MAPREDUCE_HOME
"${hadoopMapredHome}/bin/hadoop"

I agree that its awkward.  I need the examples jar and I need the hadoop command. I know that the other smoke tests expect hadoop to be in the path, so I can do that. I could also require HADOOP_HOME (or should it be prefix...). That shouldn't be too bad to require since you can just source /etc/default/hadoop for these.  Kinda tempted to go with HADOOP_HOME but since the other tests assume hadoop is in the path, think that that would be the best way to go.  Thoughts?


"I'd prefer an assert here (and in doFirst method) instead of using an exception for control flow. Besides throwing and not catching an exception is a bad idea"

The only throw that is explicit in the path is the NPE. Is the statement for the NPE? If so I can make it a assert.  The reason that I felt that it was ok to throw and not have any handling in this case is that gradle tries to enforce that these values are provided. So since gradle is verifying that they are never null, assert does make sense here.


"if itest depends on asm (which I don't recall) shall we fix it in iTest instead?
+  // looks like itest depends on this but doesn't get picked up properly
+  // assuming it gets this from hadoop which uses 3.2
+  testCompile 'asm:asm:3.2'"

Agree, its way better to fix this in itest if that is the root cause. I rather that be a different JIRA, since its unknown what the effort to fix it would be.


"Also, we are using enforcer plugin in Maven, so I am wondering if it's possible to do something similar at the top level of Gradle build for smoke-tests? It seems to be a feature that will be required all over the tests. Thoughts?"

Looking at the enforcer for `bigtop-tests/test-execution/common/pom.xml`, this just verifies that the hadoop env properties are set. Currently the gradle tests do the same with the below code in the build file.

{code}
+test.doFirst {
+  checkEnv([
+    "TACHYON_HOME",
+    "TACHYON_MASTER_ADDRESS",
+    "HADOOP_MAPRED_HOME"
+    ])
+}
{code}


"I know that it is done everywhere in the smokes
+log4j.rootLogger=TRACE, A1
but shall we approach it similarly to how we've done it before in maven executors poms? E.g. setting the log level to INFO by default via a property available for all the tests and changeable in the runtime? See bigtop-tests/test-execution/common/pom.xml for a reference. If you think it is worthy - could open a JIRA so we can improve this later? In the meanwhile, it isn't a blocker for this patch"

Its simple to do something like the below in log4j. Let me know if this is ok.  Can set -Dbigtop.default.logger=INFO,A1 to have it changed.

{code}
bigtop.default.logger=TRACE,A1
log4j.rootLogger=${bigtop.default.logger}
{code}

> Add smoke test for tachyon
> --------------------------
>
>                 Key: BIGTOP-1493
>                 URL: https://issues.apache.org/jira/browse/BIGTOP-1493
>             Project: Bigtop
>          Issue Type: Test
>          Components: tests
>    Affects Versions: 0.8.0
>            Reporter: jay vyas
>            Assignee: David Capwell
>         Attachments: BIGTOP-1493.1.patch
>
>
> Tachyon as an in memory cache will be emerging as a common feature in hadoop and spark based clusters.
> Lets create a smoke test under the {{smoke-tests}} which uses the {{tachyon:///}} url.  
> To get started with the smoke tests, you can 
> 1) Run the vagrant recipe under bigtop-deploy/vm/vagrant-puppet .  This will demonstrate the way you spin up a bigtop based hadoop cluster. 
> 2) Now, you can do {{vagrant ssh}} into your cluster, and simply edit the smoke tests or install stuff.  Re running the smoke-tests.sh file should test that your new tests work .  At that point, just exit your VM, and commit your changes and submit the new patch. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)