Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.7.0
    • Fix Version/s: 0.8.0
    • Component/s: tests
    • Labels:
      None

      Description

      The BigTop TestFuseDFS need

      • tests for consistency after individual writes : consistency in FUSE mounts is important since there are often associated multitenant workloads. Writes should result in immediate recognition of a file.
      • Should use HCFS semantics (That way we can confidently use and maintain this for alternative file system implementations).
      • should use logging instead of System.out. That way all the logs can be picked up from the same place in case we change the logging framework. Logging in trace mode, in any case, will allow us to see everything that the ITest shell does, and so might as well use it for other logs.
      1. BIGTOP-1221.6.patch
        18 kB
        jay vyas
      2. BIGTOP-1221.5.patch
        18 kB
        jay vyas
      3. BIGTOP-1221.4.intellij.patch
        16 kB
        jay vyas
      4. BIGTOP-1221.patch
        11 kB
        Konstantin Boudnik
      5. BIGTOP-1221-tested-refined-3.patch
        17 kB
        jay vyas

        Issue Links

          Activity

          jay vyas created issue -
          Hide
          jay vyas added a comment -

          I have some of this already locally, just need to test it in my bigtop VMs. Unless someone else wants to do a more comprehensive upgrade of the TestHDFSFuse.groovy, I will put in my little patch which includes 2 or 3 new unit tests, and some renames. Also will rename it to TestHCFSFuse.groovy.

          Show
          jay vyas added a comment - I have some of this already locally, just need to test it in my bigtop VMs. Unless someone else wants to do a more comprehensive upgrade of the TestHDFSFuse.groovy, I will put in my little patch which includes 2 or 3 new unit tests, and some renames. Also will rename it to TestHCFSFuse.groovy.
          Hide
          jay vyas added a comment -

          Okay, i found a bug while working this: These tests are order dependant it seems, and they have expectaions not gauranteed. Im fixing all of that in this patch and adding some real assertions that normal file commands work. Expect it to be somewhat of a beefy patch. But its a great initial start, thanks to whoever started this FUSE test , its really important to HCFS /.

          Show
          jay vyas added a comment - Okay, i found a bug while working this: These tests are order dependant it seems, and they have expectaions not gauranteed. Im fixing all of that in this patch and adding some real assertions that normal file commands work. Expect it to be somewhat of a beefy patch. But its a great initial start, thanks to whoever started this FUSE test , its really important to HCFS /.
          Hide
          jay vyas added a comment - - edited

          OKay here it is ! Some hardcore Fuse tests for HDFS (oops did i say that ... i meant HCFS ). Also the framework is easy to expand. To add a new test:

          1) Add a shell setup command (i.e. "touch a")
          2) Add a test command (i.e. "ls *")
          3) Add a labmda to evalueate the test (i.e. "sh.getOut().toString().contains("a"))"

          This will pave the way for a much more comprehensive smoke test, and will work for any HCFS as long as they create a fuse mount. For HDFS, ive confirmed it works as-is :

           
          
          [root@vagrant bigtop]# groovy -classpath /usr/lib/hadoop/hadoop-common-2.0.6-alpha.jar:/root/.m2/repository/org/apache/bigtop/itest/itest-common/0.8.0-SNAPSHOT/itest-common-0.8.0-SNAPSHOT.jar:/usr/lib/hadoop/lib/guava-11.0.2.jar:/etc/hadoop/conf/ /vagrant/TestFuseDFS2.groovy 
          mounting dfs://vagrant.bigtop1:17020 on /tmp/hcfs-test
          [/tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1]
          [file1]
          /bin/cp -rf /tmp/FUSETEST_bigtop /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cf2 out : []
          NOW IN THE VALIDATOR FUNCTION .....................
          cat output = [hi_bigtop, hi_bigtop] [] 0
          After cd, pwd=/tmp/hcfs-test/user/root/TestFuseDFS-testDir
          mkdir /tmp/hcfs-test/user/root/TestFuseDFS-testDir/targetdir && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp2 out : []
          touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/a out : []
          [total 8.0K, -rw-r--r--  1 root hadoop    0 Feb 27 01:10 a, drwxrwxrwx 10 root hadoop 4.0K Feb 27 01:10 .., drwxr-xr-x  3 root hadoop 4.0K Feb 27 01:10 .]
          mkdir -p /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1/innerfile out : []
          touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file-removed out : []
          JUnit 4 Runner, Tests: 8, Failures: 0, Time: 5533
          
          Show
          jay vyas added a comment - - edited OKay here it is ! Some hardcore Fuse tests for HDFS (oops did i say that ... i meant HCFS ). Also the framework is easy to expand. To add a new test: 1) Add a shell setup command (i.e. "touch a") 2) Add a test command (i.e. "ls *") 3) Add a labmda to evalueate the test (i.e. "sh.getOut().toString().contains("a"))" This will pave the way for a much more comprehensive smoke test, and will work for any HCFS as long as they create a fuse mount. For HDFS, ive confirmed it works as-is : [root@vagrant bigtop]# groovy -classpath /usr/lib/hadoop/hadoop-common-2.0.6-alpha.jar:/root/.m2/repository/org/apache/bigtop/itest/itest-common/0.8.0-SNAPSHOT/itest-common-0.8.0-SNAPSHOT.jar:/usr/lib/hadoop/lib/guava-11.0.2.jar:/etc/hadoop/conf/ /vagrant/TestFuseDFS2.groovy mounting dfs://vagrant.bigtop1:17020 on /tmp/hcfs-test [/tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1] [file1] /bin/cp -rf /tmp/FUSETEST_bigtop /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cf2 out : [] NOW IN THE VALIDATOR FUNCTION ..................... cat output = [hi_bigtop, hi_bigtop] [] 0 After cd, pwd=/tmp/hcfs-test/user/root/TestFuseDFS-testDir mkdir /tmp/hcfs-test/user/root/TestFuseDFS-testDir/targetdir && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp2 out : [] touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/a out : [] [total 8.0K, -rw-r--r-- 1 root hadoop 0 Feb 27 01:10 a, drwxrwxrwx 10 root hadoop 4.0K Feb 27 01:10 .., drwxr-xr-x 3 root hadoop 4.0K Feb 27 01:10 .] mkdir -p /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1/innerfile out : [] touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file-removed out : [] JUnit 4 Runner, Tests: 8, Failures: 0, Time: 5533
          jay vyas made changes -
          Field Original Value New Value
          Attachment BIGTOP-1221.patch [ 12631413 ]
          Hide
          jay vyas added a comment - - edited

          https://gist.github.com/jayunit100/9242438 <-- these tests (if you remove the "sleep 2" statement) can be used to highlight that the "copy" action on HDFS Fuse mount is only eventually consistent. I'll create a JIRA for that. Hope we can commit this patch soon, its a huge improvement i think to bigtop testing of FUSE mounts, and is also useable to the broader HCFS community where multi-tenant workloads are of increasing importance.

          For a more distilled reproducer of this, you just run a simple bash script like this:

           
            #!/bin/bash
            for i in {1..50}; 
            do    
              cp a.txt /tmp/hdfs-test/tmp/root/$i
              ls -altrh /tmp/hdfs-test/tmp/root/$i
            done
          
          Show
          jay vyas added a comment - - edited https://gist.github.com/jayunit100/9242438 <-- these tests (if you remove the "sleep 2" statement) can be used to highlight that the "copy" action on HDFS Fuse mount is only eventually consistent. I'll create a JIRA for that. Hope we can commit this patch soon, its a huge improvement i think to bigtop testing of FUSE mounts, and is also useable to the broader HCFS community where multi-tenant workloads are of increasing importance. For a more distilled reproducer of this, you just run a simple bash script like this: #!/bin/bash for i in {1..50}; do cp a.txt /tmp/hdfs-test/tmp/root/$i ls -altrh /tmp/hdfs-test/tmp/root/$i done
          Hide
          jay vyas added a comment -
          • Tl;DR ... this patch is ready to go.
          Show
          jay vyas added a comment - Tl;DR ... this patch is ready to go. I've created an official JIRA for the exposed bug shown by testCP here, https://issues.apache.org/jira/browse/HDFS-6027 ping the usual suspects who review my patches Roman Shaposhnik Konstantin Boudnik Bruno Mahé Mark Grover
          jay vyas made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Assignee jay vyas [ jayunit100 ]
          Hide
          Mark Grover added a comment -

          I do have a few nitpicks/comments. Mind uploading on reviewboard?

          Show
          Mark Grover added a comment - I do have a few nitpicks/comments. Mind uploading on reviewboard?
          Hide
          jay vyas added a comment -
          Show
          jay vyas added a comment - okay here you go ! https://reviews.apache.org/r/18593/
          Hide
          Mark Grover added a comment -

          Before I get into the nitty gritty, I don't know how I feel about updating the HDFS tests for HCFS. I'd think that it may make sense to still have HDFS tests separate from HCFS since we may be interested in testing certain things in HDFS which may not apply to HCFS. Am I making sense here? They may share code (and we can re-factor it to be that way) instead of plain update to HDFS tests.

          Show
          Mark Grover added a comment - Before I get into the nitty gritty, I don't know how I feel about updating the HDFS tests for HCFS. I'd think that it may make sense to still have HDFS tests separate from HCFS since we may be interested in testing certain things in HDFS which may not apply to HCFS. Am I making sense here? They may share code (and we can re-factor it to be that way) instead of plain update to HDFS tests.
          Hide
          jay vyas added a comment -

          I cant really think of any FUSE operatoin which would be tested differently for HDFS versus other HCFS implementations. After all, FUSE is an interface which should have no knowledge of the underlying file system implementation.

          Show
          jay vyas added a comment - I cant really think of any FUSE operatoin which would be tested differently for HDFS versus other HCFS implementations. After all, FUSE is an interface which should have no knowledge of the underlying file system implementation.
          Hide
          Raymie Stata added a comment -

          FYI, in the past, our users were able to get FUSE to fail fairly regularly with "chgrp" and "chown" operations. To avoid this, we switched FUSE to read-only at some pre-2.0.5 Hadoop release and haven't switched it back since (so I don't know if this is still a problem, but I suspect it is).

          Show
          Raymie Stata added a comment - FYI, in the past, our users were able to get FUSE to fail fairly regularly with "chgrp" and "chown" operations. To avoid this, we switched FUSE to read-only at some pre-2.0.5 Hadoop release and haven't switched it back since (so I don't know if this is still a problem, but I suspect it is).
          Hide
          jay vyas added a comment -

          Yes, thats why this patch is important

          FUSE is an interface, and we should test it as such.

          If we dont want to run certain FUSE tests in HDFS, then we can specify that as a configuration parameter. Creating a special unit test for every FileSystem implementation (HDFS,Kosmos,GlusterFileSystem,S3FileSystem,....) is a really massive undertaking.

          Mark Grover ... is this convincing you at all ? If not I'm okay changing the architecture. But the key goal here is to have a solid HCFS FUSE test that confirms that distributed file system implementations of the FUSE interface work correctly.

          Show
          jay vyas added a comment - Yes, thats why this patch is important FUSE is an interface, and we should test it as such. If we dont want to run certain FUSE tests in HDFS, then we can specify that as a configuration parameter. Creating a special unit test for every FileSystem implementation (HDFS,Kosmos,GlusterFileSystem,S3FileSystem,....) is a really massive undertaking. Mark Grover ... is this convincing you at all ? If not I'm okay changing the architecture. But the key goal here is to have a solid HCFS FUSE test that confirms that distributed file system implementations of the FUSE interface work correctly.
          Hide
          jay vyas added a comment -

          tl;dr: Mark Grover , i can refactor the patch easily to just be a separate HCFS test, in the hadoop/hcfs package. just let me know what are the nits and i will fix them

          Show
          jay vyas added a comment - tl;dr: Mark Grover , i can refactor the patch easily to just be a separate HCFS test, in the hadoop/hcfs package. just let me know what are the nits and i will fix them
          Hide
          jay vyas added a comment -

          hi mark : Any update on the nits? excited about re-rolling this patch and for the follow up on it, ive written BIGTOP-1233 to remove the "sleep" command (im assuming that may have been one of your nits) after we patch HDFS-6027.

          Show
          jay vyas added a comment - hi mark : Any update on the nits? excited about re-rolling this patch and for the follow up on it, ive written BIGTOP-1233 to remove the "sleep" command (im assuming that may have been one of your nits) after we patch HDFS-6027 .
          Hide
          Mark Grover added a comment -

          Jay, swamped right now but will take a look soonish. Thanks for your patience.

          Show
          Mark Grover added a comment - Jay, swamped right now but will take a look soonish. Thanks for your patience.
          Hide
          jay vyas added a comment -

          Okay sure. thanks again mark!

          Show
          jay vyas added a comment - Okay sure. thanks again mark!
          Hide
          Konstantin Boudnik added a comment - - edited

          Jay, could you remove white space changes please? Like those on lines 46, 47, 56 and others.

          Show
          Konstantin Boudnik added a comment - - edited Jay, could you remove white space changes please? Like those on lines 46, 47, 56 and others.
          Hide
          jay vyas added a comment -

          Sure. Any other changes? Would like to roll them into one patch.

          Show
          jay vyas added a comment - Sure. Any other changes? Would like to roll them into one patch.
          Hide
          Konstantin Boudnik added a comment -
          • When you change the package of a class you actually better move the class as well:
            --- a/bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hdfs/TestFuseDFS.groovy
            +++ b/bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hdfs/TestFuseDFS.groovy
            @@ -15,7 +15,7 @@
              * See the License for the specific language governing permissions and
              * limitations under the License.
              */
            -package org.apache.bigtop.itest.hadoop.hdfs
            +package org.apache.bigtop.itest.hadoop.hcfs
            
          • declare types in the testWrapper as you know them
          • remove commented out lines that seems like a leftovers from previous experiments
          • create javadoc for the testWrapper method
          • there seems to be some inconsistency in how testWrapper is called. E.g. looks like a parameter is missed here (testCd)
            +    testWrapper( 
            +	"cd ${testdir} && pwd ",
            +  	{ 
            
          • there are some overall inconsistencies in the identation: they are 2, 3, and sometimes 4. Please follow the guidelines on that.
          • not all asserts have messages
          • there seems to be a number of unnecessary empty lines
          • don't use printlns - a logger should be used instead
          Show
          Konstantin Boudnik added a comment - When you change the package of a class you actually better move the class as well: --- a/bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hdfs/TestFuseDFS.groovy +++ b/bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hdfs/TestFuseDFS.groovy @@ -15,7 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.bigtop.itest.hadoop.hdfs +package org.apache.bigtop.itest.hadoop.hcfs declare types in the testWrapper as you know them remove commented out lines that seems like a leftovers from previous experiments create javadoc for the testWrapper method there seems to be some inconsistency in how testWrapper is called. E.g. looks like a parameter is missed here (testCd) + testWrapper( + "cd ${testdir} && pwd ", + { there are some overall inconsistencies in the identation: they are 2, 3, and sometimes 4. Please follow the guidelines on that. not all asserts have messages there seems to be a number of unnecessary empty lines don't use printlns - a logger should be used instead
          Hide
          jay vyas added a comment - - edited

          Yup that is actually the wrong patch. Thanks cos l put the one with the correct package
          Up and fix the other indents . Re: inconsistent. ... The testWrPper method is polymorphic. It can run a setup function.

          Show
          jay vyas added a comment - - edited Yup that is actually the wrong patch. Thanks cos l put the one with the correct package Up and fix the other indents . Re: inconsistent. ... The testWrPper method is polymorphic. It can run a setup function.
          Hide
          jay vyas added a comment -

          Okay ! The "right" patch is attached . Also I added in the formatting stuff.
          Tests pass below:

          [root@vagrant bigtop]# groovy -classpath /usr/lib/hadoop/hadoop-common-2.0.6-alpha.jar:/root/.m2/repository/org/apache/bigtop/itest/itest-common/0.8.0-SNAPSHOT/itest-common-0.8.0-SNAPSHOT.jar:/usr/lib/hadoop/lib/guava-11.0.2.jar:/etc/hadoop/conf/ /vagrant/TestFuseHCFS.groovy
          Mar 01, 2014 4:08:55 PM org.apache.commons.logging.Log$info$0 call
          INFO: mounting dfs://vagrant.bigtop1:17020 on /tmp/hcfs-test
          Mar 01, 2014 4:08:58 PM org.apache.commons.logging.Log$info$0 call
          INFO: [/tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1]
          Mar 01, 2014 4:08:58 PM org.apache.commons.logging.Log$info$0 call
          INFO: [file1]
          Mar 01, 2014 4:08:58 PM org.apache.commons.logging.Log$info$0 call
          INFO: /bin/cp -rf /tmp/FUSETEST_bigtop /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cf2 out : []
          Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call
          INFO: cat output = [hi_bigtop, hi_bigtop] [] 0
          Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call
          INFO: After cd, pwd=/tmp/hcfs-test/user/root/TestFuseDFS-testDir
          Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call
          INFO: mkdir /tmp/hcfs-test/user/root/TestFuseDFS-testDir/targetdir && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp2 out : []
          Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call
          INFO: touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/a out : []
          Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call
          INFO: [total 8.0K, -rw-r--r--  1 root hadoop    0 Mar  1 16:09 a, drwxrwxrwx 10 root hadoop 4.0K Mar  1 16:09 .., drwxr-xr-x  3 root hadoop 4.0K Mar  1 16:09 .]
          Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call
          INFO: mkdir -p /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1/innerfile out : []
          Mar 01, 2014 4:09:01 PM org.apache.commons.logging.Log$info$0 call
          INFO: touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file-removed out : []
          JUnit 4 Runner, Tests: 8, Failures: 0, Time: 5660
          
          
          Show
          jay vyas added a comment - Okay ! The "right" patch is attached . Also I added in the formatting stuff. Tests pass below: [root@vagrant bigtop]# groovy -classpath /usr/lib/hadoop/hadoop-common-2.0.6-alpha.jar:/root/.m2/repository/org/apache/bigtop/itest/itest-common/0.8.0-SNAPSHOT/itest-common-0.8.0-SNAPSHOT.jar:/usr/lib/hadoop/lib/guava-11.0.2.jar:/etc/hadoop/conf/ /vagrant/TestFuseHCFS.groovy Mar 01, 2014 4:08:55 PM org.apache.commons.logging.Log$info$0 call INFO: mounting dfs://vagrant.bigtop1:17020 on /tmp/hcfs-test Mar 01, 2014 4:08:58 PM org.apache.commons.logging.Log$info$0 call INFO: [/tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1] Mar 01, 2014 4:08:58 PM org.apache.commons.logging.Log$info$0 call INFO: [file1] Mar 01, 2014 4:08:58 PM org.apache.commons.logging.Log$info$0 call INFO: /bin/cp -rf /tmp/FUSETEST_bigtop /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cf2 out : [] Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call INFO: cat output = [hi_bigtop, hi_bigtop] [] 0 Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call INFO: After cd, pwd=/tmp/hcfs-test/user/root/TestFuseDFS-testDir Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call INFO: mkdir /tmp/hcfs-test/user/root/TestFuseDFS-testDir/targetdir && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp2 out : [] Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call INFO: touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/a out : [] Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call INFO: [total 8.0K, -rw-r--r-- 1 root hadoop 0 Mar 1 16:09 a, drwxrwxrwx 10 root hadoop 4.0K Mar 1 16:09 .., drwxr-xr-x 3 root hadoop 4.0K Mar 1 16:09 .] Mar 01, 2014 4:09:00 PM org.apache.commons.logging.Log$info$0 call INFO: mkdir -p /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1/innerfile out : [] Mar 01, 2014 4:09:01 PM org.apache.commons.logging.Log$info$0 call INFO: touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file-removed out : [] JUnit 4 Runner, Tests: 8, Failures: 0, Time: 5660
          jay vyas made changes -
          Attachment BIGTOP-1221-cos-fixes.patch [ 12631953 ]
          Hide
          Virginia Wang added a comment -

          Just a comment on running the unaltered version of this test, the test sequence can be run out of order and that can cause it to fail. Example, the "-cat" test can occur before the "-touch". Test fuse also cannot work with a high availability solution with multiple namenodes as the fs.defaultFS will not match.

          Show
          Virginia Wang added a comment - Just a comment on running the unaltered version of this test, the test sequence can be run out of order and that can cause it to fail. Example, the "-cat" test can occur before the "-touch". Test fuse also cannot work with a high availability solution with multiple namenodes as the fs.defaultFS will not match.
          Hide
          jay vyas added a comment -

          thanks for commenting - and yes ! This patch fixes the above issue (all tests are completely stateless - see the lambda testWrapper - which recreates a brand new fuse mount and does any necessary setup file creation). FYI we've also got confirmation from the HDFS community that it also exposes a real bug in HDFS consistency : HADOOP-6072.

          Show
          jay vyas added a comment - thanks for commenting - and yes ! This patch fixes the above issue (all tests are completely stateless - see the lambda testWrapper - which recreates a brand new fuse mount and does any necessary setup file creation). FYI we've also got confirmation from the HDFS community that it also exposes a real bug in HDFS consistency : HADOOP-6072 .
          Hide
          Virginia Wang added a comment -

          Great! Thanks for the quick response.

          Show
          Virginia Wang added a comment - Great! Thanks for the quick response.
          Konstantin Boudnik made changes -
          Affects Version/s 0.7.0 [ 12324362 ]
          Konstantin Boudnik made changes -
          Component/s Tests [ 12315617 ]
          Konstantin Boudnik made changes -
          Fix Version/s 0.8.0 [ 12324841 ]
          Hide
          Virginia Wang added a comment -

          I just tested the patch, it seems that it does fix the ordering I mentioned, but still is not working with HA configurations.

          It seems to fail when it tries to mount the directory. The mount does happen but when I cd into it I'm getting "cd: /tmp/hcfs-test: Input/output error"

          Show
          Virginia Wang added a comment - I just tested the patch, it seems that it does fix the ordering I mentioned, but still is not working with HA configurations. It seems to fail when it tries to mount the directory. The mount does happen but when I cd into it I'm getting "cd: /tmp/hcfs-test: Input/output error"
          Hide
          jay vyas added a comment -

          Thanks Virginia: since that is another bug can you open a new jira for it which piggybacks off of this?

          Or else do you think it's so important that we need to fix it here.?

          This patch is already so big id be afraid of bundling more into it?

          Show
          jay vyas added a comment - Thanks Virginia: since that is another bug can you open a new jira for it which piggybacks off of this? Or else do you think it's so important that we need to fix it here.? This patch is already so big id be afraid of bundling more into it?
          Hide
          Konstantin Boudnik added a comment -

          If non-HA use-case is covered by the patch and it's all good then I am ok with addressing HA part of it separately. Jay, I will review your patch later today and if everything is in order I will commit it.

          Show
          Konstantin Boudnik added a comment - If non-HA use-case is covered by the patch and it's all good then I am ok with addressing HA part of it separately. Jay, I will review your patch later today and if everything is in order I will commit it.
          Hide
          Konstantin Boudnik added a comment -

          If non-HA use-case is covered by the patch and it's all good then I am ok with addressing HA part of it separately. Jay, I will review your patch later today and if everything is in order I will commit it.

          Show
          Konstantin Boudnik added a comment - If non-HA use-case is covered by the patch and it's all good then I am ok with addressing HA part of it separately. Jay, I will review your patch later today and if everything is in order I will commit it.
          Hide
          Virginia Wang added a comment -

          I've created a jira for the HA issue for this test: BIGTOP-1237.

          Show
          Virginia Wang added a comment - I've created a jira for the HA issue for this test: BIGTOP-1237 .
          Virginia Wang made changes -
          Link This issue relates to BIGTOP-1237 [ BIGTOP-1237 ]
          Konstantin Boudnik made changes -
          Link This issue is blocked by BIGTOP-1237 [ BIGTOP-1237 ]
          Konstantin Boudnik made changes -
          Link This issue relates to BIGTOP-1237 [ BIGTOP-1237 ]
          Hide
          Konstantin Boudnik added a comment - - edited

          Thanks Jay. Some comments (mostly cosmetic):

          • conceptually speaking: how this new test relates to the original TestFuseDFS? Will it be a replacement to the original test? If so, the old class should be gone as the part of the same patch
          • as always - indentation is wrong
          • in the testCp (and elsewhere) you use full paths inconsistently. Is there a reason for that?
          • there are some comments formatting issues that need to be addressed
          Show
          Konstantin Boudnik added a comment - - edited Thanks Jay. Some comments (mostly cosmetic): conceptually speaking: how this new test relates to the original TestFuseDFS ? Will it be a replacement to the original test? If so, the old class should be gone as the part of the same patch as always - indentation is wrong in the testCp (and elsewhere) you use full paths inconsistently. Is there a reason for that? there are some comments formatting issues that need to be addressed
          Hide
          Mark Grover added a comment -

          Sorry, it took me a little longer to get back but here is my feedback. Left some feedback on reviewboard.

          • Can you please update the reviewboard with the latest patch? I really prefer reviewboard for large patches like this and it seems like you incorporated Cos' feedback but forgot to update reviewboard.
          • I tried applying the patch on master and got some whitespace related warnings:
            Applying: BIGTOP-1221: Significant update FUSE tests to test more feature, consistency, and file contents + smoke test for HDFS-6027.
            /opt/src/bigtop/.git/rebase-apply/patch:43: trailing whitespace.
             * This is a "superset" of the hdfs/TestFuseDFS class.  
            /opt/src/bigtop/.git/rebase-apply/patch:46: trailing whitespace.
             * After all, FUSE is an interface, and it should be tested at that level, with 
            /opt/src/bigtop/.git/rebase-apply/patch:49: trailing whitespace.
             * 
            /opt/src/bigtop/.git/rebase-apply/patch:51: trailing whitespace.
             * and thus somewhat overcommented for the first iteration, we can clean comments up 
            /opt/src/bigtop/.git/rebase-apply/patch:61: trailing whitespace.
                
            warning: squelched 14 whitespace errors
            warning: 19 lines add whitespace errors.
            

            Some of them maybe from before your time, and if so, try to see if you can fix them. If it's something you are including, see if you can fix it.

          Show
          Mark Grover added a comment - Sorry, it took me a little longer to get back but here is my feedback. Left some feedback on reviewboard. Can you please update the reviewboard with the latest patch? I really prefer reviewboard for large patches like this and it seems like you incorporated Cos' feedback but forgot to update reviewboard. I tried applying the patch on master and got some whitespace related warnings: Applying: BIGTOP-1221: Significant update FUSE tests to test more feature, consistency, and file contents + smoke test for HDFS-6027. /opt/src/bigtop/.git/rebase-apply/patch:43: trailing whitespace. * This is a "superset" of the hdfs/TestFuseDFS class. /opt/src/bigtop/.git/rebase-apply/patch:46: trailing whitespace. * After all, FUSE is an interface , and it should be tested at that level, with /opt/src/bigtop/.git/rebase-apply/patch:49: trailing whitespace. * /opt/src/bigtop/.git/rebase-apply/patch:51: trailing whitespace. * and thus somewhat overcommented for the first iteration, we can clean comments up /opt/src/bigtop/.git/rebase-apply/patch:61: trailing whitespace. warning: squelched 14 whitespace errors warning: 19 lines add whitespace errors. Some of them maybe from before your time, and if so, try to see if you can fix them. If it's something you are including, see if you can fix it.
          Hide
          jay vyas added a comment -

          Thanks cos. Responses inline + a new patch.

          • mark mentioned maybe we should keep the HDFS test, so I opted to add this as a new test rather than delete it. I will attach with deleted TestHDFS test.
          • indentation: okay, ive given it another go. Looks like some of the stuff was double indented.
          • I'm using /bin/cp to avoid any issues with aliasing of cp. Since CP is an error prone command (i.e. we know it can break hadoop FUSE), we want to be very careful that we call it using exact "cp" program and not a common aliases (cp is normally aliased with overwrite interactive prompt, which we certainly dont want in a unit test).
          • Fixed up the indents and formats some more. Looks better? I still dont have a good groovy formatter. Right now im using eclipse's java one and it seems to choke on the functional stuff.
          Show
          jay vyas added a comment - Thanks cos. Responses inline + a new patch. mark mentioned maybe we should keep the HDFS test, so I opted to add this as a new test rather than delete it. I will attach with deleted TestHDFS test. indentation: okay, ive given it another go. Looks like some of the stuff was double indented. I'm using /bin/cp to avoid any issues with aliasing of cp. Since CP is an error prone command (i.e. we know it can break hadoop FUSE), we want to be very careful that we call it using exact "cp" program and not a common aliases (cp is normally aliased with overwrite interactive prompt, which we certainly dont want in a unit test). Fixed up the indents and formats some more. Looks better? I still dont have a good groovy formatter. Right now im using eclipse's java one and it seems to choke on the functional stuff.
          jay vyas made changes -
          Attachment BIGTOP-1221-tested-refined-2.patch [ 12632432 ]
          Hide
          jay vyas added a comment -

          mark + cos: I've also updated the patch on review board : https://reviews.apache.org/r/18593/ . let me know if this looks better. also mark ive never used reviewboard so is there a way i can edit the patch directly there? That would be awesome or is it just for communicating changes?

          Show
          jay vyas added a comment - mark + cos: I've also updated the patch on review board : https://reviews.apache.org/r/18593/ . let me know if this looks better. also mark ive never used reviewboard so is there a way i can edit the patch directly there? That would be awesome or is it just for communicating changes?
          Hide
          Mark Grover added a comment -

          Thanks Jay. Just communicating, upload a new diff after making and testing
          changes locally.

          Show
          Mark Grover added a comment - Thanks Jay. Just communicating, upload a new diff after making and testing changes locally.
          Hide
          jay vyas added a comment -

          Okay, heres another iteration on the patch. whew .

          • Mark I incorporated some of your comments (with exception of the HDFS one, because i think all we know for sure is that its possible HDFS might have special tests - as per your original comment on this patch - however we dont know wether that will be limited to fuse mounting). Also, other file system deployments may not want to mount the FUSE as part of the tests (remember that such mounting requires root proviliges, so , its not always a good idea to do it from the JVM, unless you really have to).
          Show
          jay vyas added a comment - Okay, heres another iteration on the patch. whew . Mark I incorporated some of your comments (with exception of the HDFS one, because i think all we know for sure is that its possible HDFS might have special tests - as per your original comment on this patch - however we dont know wether that will be limited to fuse mounting). Also, other file system deployments may not want to mount the FUSE as part of the tests (remember that such mounting requires root proviliges, so , its not always a good idea to do it from the JVM, unless you really have to).
          jay vyas made changes -
          Attachment BIGTOP-1221-tested-refined-3.patch [ 12632662 ]
          Hide
          Konstantin Boudnik added a comment -

          Jay, I don't think you have addressed my comments, though

          Show
          Konstantin Boudnik added a comment - Jay, I don't think you have addressed my comments, though
          Hide
          Mark Grover added a comment -

          I agree with Cos

          Show
          Mark Grover added a comment - I agree with Cos
          jay vyas made changes -
          Attachment BIGTOP-1221.patch [ 12631413 ]
          jay vyas made changes -
          Attachment BIGTOP-1221-cos-fixes.patch [ 12631953 ]
          jay vyas made changes -
          Attachment BIGTOP-1221-tested-refined-2.patch [ 12632432 ]
          Hide
          Konstantin Boudnik added a comment -

          Jay, not to pick up on you or something, but keeping the patches (along with the review comments) in the JIRA is helpful in case we won't to revisit some points at the later time. Just for the future, keep the consistent names of the patches (JIRA will sort it out properly), and don't delete the old versions. Hope it makes sense.

          Show
          Konstantin Boudnik added a comment - Jay, not to pick up on you or something, but keeping the patches (along with the review comments) in the JIRA is helpful in case we won't to revisit some points at the later time. Just for the future, keep the consistent names of the patches (JIRA will sort it out properly), and don't delete the old versions. Hope it makes sense.
          Hide
          jay vyas added a comment -

          okay. cos + mark.

          I'll put all the comments inline here. first ill start w/ cos: next thread ill address marks issues.

          cos's comments:

          • conceptually speaking: how this new test relates to the original TestFuseDFS? Will it be a replacement to the original test? If so, the old class should be gone as the part of the same patch

          response: I've deleted that file now.

          • as always - indentation is wrong

          response: ive manually fixed up the indents. are they still bad?

          • in the testCp (and elsewhere) you use full paths inconsistently. Is there a reason for that?

          response: This is answered above: /bin/cp is the full path your referencing, and ive used it so as to avoid aliased cp, which can result in the overwrite prompts. Best to be deterministic about how "cp" is called in a FUSE test IMO.

          • there are some comments formatting issues that need to be addressed

          response: I've manually attempted to address formating of comments. are they still messed up?

          Show
          jay vyas added a comment - okay. cos + mark. I'll put all the comments inline here. first ill start w/ cos: next thread ill address marks issues. cos's comments: conceptually speaking: how this new test relates to the original TestFuseDFS? Will it be a replacement to the original test? If so, the old class should be gone as the part of the same patch response: I've deleted that file now. as always - indentation is wrong response: ive manually fixed up the indents. are they still bad? in the testCp (and elsewhere) you use full paths inconsistently. Is there a reason for that? response: This is answered above: /bin/cp is the full path your referencing, and ive used it so as to avoid aliased cp, which can result in the overwrite prompts. Best to be deterministic about how "cp" is called in a FUSE test IMO. there are some comments formatting issues that need to be addressed response: I've manually attempted to address formating of comments. are they still messed up?
          Hide
          Konstantin Boudnik added a comment -

          Jay, the comments formatting seems to be ok. The indentation is still incorrect. It should be 2 spaces or 4 for continuous statements. I am attached what is properly formatted from Hadoop style guys perspective (there are still some long lines that are 80+ symbols which you need to address, by the indentation is correct)

          Show
          Konstantin Boudnik added a comment - Jay, the comments formatting seems to be ok. The indentation is still incorrect. It should be 2 spaces or 4 for continuous statements. I am attached what is properly formatted from Hadoop style guys perspective (there are still some long lines that are 80+ symbols which you need to address, by the indentation is correct)
          Konstantin Boudnik made changes -
          Attachment BIGTOP-1221.patch [ 12632716 ]
          Hide
          jay vyas added a comment -

          Okay, based on BIGTOP-1240 im going to try formatting in intelliJ. Eclipse is clearly failing me at least for the groovy formatting stuff. anyways. will upload a new patch shortly . also ive confirmed mark's issue that the patch doesnt apply b/c of whitespace.

          I'll let you folks know when another patch is ready.

          thanks and sorry for the churn. I'll use this as an opportunity to get my tooling right.

          Show
          jay vyas added a comment - Okay, based on BIGTOP-1240 im going to try formatting in intelliJ. Eclipse is clearly failing me at least for the groovy formatting stuff. anyways. will upload a new patch shortly . also ive confirmed mark's issue that the patch doesnt apply b/c of whitespace. I'll let you folks know when another patch is ready. thanks and sorry for the churn. I'll use this as an opportunity to get my tooling right.
          Hide
          jay vyas added a comment - - edited

          Okay. HEre is a beautified patch using my groovy new intellij setup, which i think mimics hadoop / bigtop formatting pretty closely.

          Mark ive also tested apply and it works on bigtop head and it lays over correctly now.

          Looking better ? (FYI Mark im updating the reviewboard also right now: https://reviews.apache.org/r/18593/diff/2-3/)

          Show
          jay vyas added a comment - - edited Okay. HEre is a beautified patch using my groovy new intellij setup, which i think mimics hadoop / bigtop formatting pretty closely. Mark ive also tested apply and it works on bigtop head and it lays over correctly now. Looking better ? (FYI Mark im updating the reviewboard also right now: https://reviews.apache.org/r/18593/diff/2-3/ )
          jay vyas made changes -
          Attachment BIGTOP-1221.4.intellij.patch [ 12632747 ]
          Hide
          Mark Grover added a comment -

          Hi Jay, thanks for updating the patch based on our suggestions. I'll take a look soon.

          Show
          Mark Grover added a comment - Hi Jay, thanks for updating the patch based on our suggestions. I'll take a look soon.
          Hide
          Mark Grover added a comment -

          Hi Jay, I looked at the review. It looks like you incorporated some of my feedback, thank you. However, it seems like you may have missed some of the comments. In any case, I have put them up again on the latest diff: So if you simply go to https://reviews.apache.org/r/18593/diff/#index_header, you will see the blue bubbles on the left which symbolize comments.

          It looks like a very nice re-structuring of the code, thanks for undertaking this effort. A few things:
          1. I am ok with the way you have isHDFS variable, so please feel free to keep it that way.
          2. I am ok with you renaming the test from the TestFuseDFS to TestFuseHCFS. Let's just make sure by a quick grep nothing else in the code refers to the old name.
          3. It seemed like you were using the first argument of asserts as a message about what the assert is doing e.g. "verifying ..." etc. However, my understanding is that the first string argument is printed when the assertion fails, so I'd think they should be error messages, otherwise it would be very misleading for another person like me trying to triage a test output to figure out what exactly was going on. Can you reword them to be error messages please? For example, if you were asserting that the exit code was zero, a good first string argument (i.e. error message) would be "Exit code was not zero".

          Show
          Mark Grover added a comment - Hi Jay, I looked at the review. It looks like you incorporated some of my feedback, thank you. However, it seems like you may have missed some of the comments. In any case, I have put them up again on the latest diff: So if you simply go to https://reviews.apache.org/r/18593/diff/#index_header , you will see the blue bubbles on the left which symbolize comments. It looks like a very nice re-structuring of the code, thanks for undertaking this effort. A few things: 1. I am ok with the way you have isHDFS variable, so please feel free to keep it that way. 2. I am ok with you renaming the test from the TestFuseDFS to TestFuseHCFS. Let's just make sure by a quick grep nothing else in the code refers to the old name. 3. It seemed like you were using the first argument of asserts as a message about what the assert is doing e.g. "verifying ..." etc. However, my understanding is that the first string argument is printed when the assertion fails, so I'd think they should be error messages, otherwise it would be very misleading for another person like me trying to triage a test output to figure out what exactly was going on. Can you reword them to be error messages please? For example, if you were asserting that the exit code was zero, a good first string argument (i.e. error message) would be "Exit code was not zero".
          Hide
          jay vyas added a comment -

          hiya mark. Sure, I'll iterate on it incorporating the propert assert messages and stuff.

          Thanks again for all the feedback !

          This has been a good opportunity to get my tooling in place.

          I will ping you after i upload a new patch to reviewboard.

          Show
          jay vyas added a comment - hiya mark. Sure, I'll iterate on it incorporating the propert assert messages and stuff. Thanks again for all the feedback ! This has been a good opportunity to get my tooling in place. I will ping you after i upload a new patch to reviewboard.
          Hide
          jay vyas added a comment -

          one quick thing mark, before i fix the other issues ... Sorry if i wasnt clear about why the cat test was removed : Its really because it was a very basic test which didnt really test anything. The new cat test tests exit code as well as output.

          does that make more sense? If i just rename "testZCat" to "testCat" I think we will all be happy.

          Show
          jay vyas added a comment - one quick thing mark, before i fix the other issues ... Sorry if i wasnt clear about why the cat test was removed : Its really because it was a very basic test which didnt really test anything. The new cat test tests exit code as well as output. does that make more sense? If i just rename "testZCat" to "testCat" I think we will all be happy.
          Hide
          Mark Grover added a comment -

          Sure, that'd be ok.

          Show
          Mark Grover added a comment - Sure, that'd be ok.
          Hide
          jay vyas added a comment - - edited

          oops wrong jira (remove this comment).

          Show
          jay vyas added a comment - - edited oops wrong jira (remove this comment).
          jay vyas made changes -
          Attachment BIGTOP-1200.reformatted.alice.3.patch [ 12632989 ]
          jay vyas made changes -
          Attachment BIGTOP-1200.reformatted.alice.3.patch [ 12632989 ]
          Hide
          jay vyas added a comment -

          hi mark, ... okay your comments definetly make sense, much easier to see what you were talking about now since the minor stuff is fixed.

          • i think the test0Echo and "test contents" are obsolete from an old patch.
          • Ive now update the testCp to actually test contents using a diff. thats a good idea.
          • Added an assertion that the first mkdirs passed, again goot catch . your right : it very well could fail if fuse mount is broken at the core.
          • another good point that contains "hcfs" was fragile. Better to do containes(testdir), also fixed that.
          • regarding cat. I think we are in agreement on that now as per above comment.
          • Regarding closting the file. Evidently magical groovy does it for you http://www.coderanch.com/t/541995/Groovy/close-file
          • I fixed the assert messages so that they all use Failure/Error semantics.. A bit hasty , prob error messages could be even better. but i think it will satisfy the patch for now. Im planning on iterating on this patch very soon, because FUSE mount semantics are quite complex, especially with the HA Jira just filed, so we can keep improving it.

          Now, after adding your cp verification, i found another inconsistency in HDFS, but was able to workaround it by adding better FUSE mount options.

          Im attaching the patch and will verify in the morning that its ready for review.

          Show
          jay vyas added a comment - hi mark, ... okay your comments definetly make sense, much easier to see what you were talking about now since the minor stuff is fixed. i think the test0Echo and "test contents" are obsolete from an old patch. Ive now update the testCp to actually test contents using a diff. thats a good idea. Added an assertion that the first mkdirs passed, again goot catch . your right : it very well could fail if fuse mount is broken at the core. another good point that contains "hcfs" was fragile. Better to do containes(testdir), also fixed that. regarding cat. I think we are in agreement on that now as per above comment. Regarding closting the file. Evidently magical groovy does it for you http://www.coderanch.com/t/541995/Groovy/close-file I fixed the assert messages so that they all use Failure/Error semantics.. A bit hasty , prob error messages could be even better. but i think it will satisfy the patch for now. Im planning on iterating on this patch very soon, because FUSE mount semantics are quite complex, especially with the HA Jira just filed, so we can keep improving it. Now, after adding your cp verification, i found another inconsistency in HDFS, but was able to workaround it by adding better FUSE mount options. Im attaching the patch and will verify in the morning that its ready for review.
          jay vyas made changes -
          Attachment BIGTOP-1221.5.patch [ 12633045 ]
          Hide
          jay vyas added a comment -

          okay mark ! Ready for review. Ive run tests in VMs, they pass, and also the diff looks i think good to me bye eye: https://reviews.apache.org/r/18593/diff/3-4/.

          Let me know what you think

          Show
          jay vyas added a comment - okay mark ! Ready for review. Ive run tests in VMs, they pass, and also the diff looks i think good to me bye eye: https://reviews.apache.org/r/18593/diff/3-4/ . Let me know what you think
          Hide
          jay vyas added a comment -

          some more cleanup, updating reviewboard as well

          Show
          jay vyas added a comment - some more cleanup, updating reviewboard as well
          jay vyas made changes -
          Attachment BIGTOP-1221.6.patch [ 12633159 ]
          jay vyas made changes -
          Attachment BIGTOP-1221.6.patch [ 12633159 ]
          jay vyas made changes -
          Attachment BIGTOP-1221.6.patch [ 12633160 ]
          jay vyas made changes -
          Attachment BIGTOP-1221.6.patch [ 12633160 ]
          Hide
          jay vyas added a comment -

          Okay, mark- heres some more cleanup ... I just couldnt help myself.

          https://reviews.apache.org/r/18593/diff/3-7/

          I think this should be quite close to something we can move fwd with .

          • uniform "Failed: " messages
          • cleaned up comments some more
          Show
          jay vyas added a comment - Okay, mark- heres some more cleanup ... I just couldnt help myself. https://reviews.apache.org/r/18593/diff/3-7/ I think this should be quite close to something we can move fwd with . uniform "Failed: " messages cleaned up comments some more
          jay vyas made changes -
          Attachment BIGTOP-1221.6.patch [ 12633162 ]
          Hide
          Mark Grover added a comment -

          ok, thanks Jay. Will take a look.

          Show
          Mark Grover added a comment - ok, thanks Jay. Will take a look.
          Hide
          jay vyas added a comment -

          hi folks. i know its a busy week for everyone so might take a few more days to get this commited, ... but just FYI have now run a final test of this patch to confirm it works in its existing state. Its definetly ready to roll !

           
          [root@vagrant bigtop]# groovy -classpath /usr/lib/hadoop/hadoop-common-2.0.6-alpha.jar:/root/.m2/repository/org/apache/bigtop/itest/itest-common/0.8.0-SNAPSHOT/itest-common-0.8.0-SNAPSHOT.jar:/usr/lib/hadoop/lib/guava-11.0.2.jar:/etc/hadoop/conf/ bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hcfs/TestFuseHCFS.groovy 
          Mar 11, 2014 12:37:33 AM org.apache.commons.logging.Log$info$0 call
          INFO: mounting dfs://vagrant.bigtop1:17020 on /tmp/hcfs-test
          Mar 11, 2014 12:37:35 AM org.apache.commons.logging.Log$info$0 call
          INFO: /bin/cp -rf /tmp/FUSETEST_bigtop /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cf2 out : []
          Mar 11, 2014 12:37:37 AM org.apache.commons.logging.Log$info$0 call
          INFO: cat output = [hi_bigtop, hi_bigtop] [] 0
          Mar 11, 2014 12:37:37 AM org.apache.commons.logging.Log$info$0 call
          INFO: Cleaning after sleep 2 && cat /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cf2
          Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call
          INFO: [/tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1]
          Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call
          INFO: Cleaning after mkdir /tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1 && cd /tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1 && pwd
          Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call
          INFO: [file1]
          Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call
          INFO: Cleaning after touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file1 && ls /tmp/hcfs-test/user/root/TestFuseDFS-testDir
          Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call
          INFO: After cd, pwd=/tmp/hcfs-test/user/root/TestFuseDFS-testDir
          Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call
          INFO: Cleaning after cd /tmp/hcfs-test/user/root/TestFuseDFS-testDir && pwd 
          Mar 11, 2014 12:37:39 AM org.apache.commons.logging.Log$info$0 call
          INFO: Cleaning after mkdir /tmp/hcfs-test/user/root/TestFuseDFS-testDir/targetdir &&echo ABCDEFGHIJKLMNOPZUIRPIEOF > /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp1 && echo ABCDEFGHIJKLMNOPZUIRPIEOF > /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp2 && /bin/cp -rf /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp* /tmp/hcfs-test/user/root/TestFuseDFS-testDir/targetdir/
          Mar 11, 2014 12:37:39 AM org.apache.commons.logging.Log$info$0 call
          INFO: touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/non-trivial-fn out : []
          Mar 11, 2014 12:37:40 AM org.apache.commons.logging.Log$info$0 call
          INFO: Cleaning after ls -altrh /tmp/hcfs-test/user/root/TestFuseDFS-testDir
          Mar 11, 2014 12:37:40 AM org.apache.commons.logging.Log$info$0 call
          INFO: mkdir -p /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1/innerfile out : []
          Mar 11, 2014 12:37:40 AM org.apache.commons.logging.Log$info$0 call
          INFO: Cleaning after mv /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1 /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir2
          Mar 11, 2014 12:37:41 AM org.apache.commons.logging.Log$info$0 call
          INFO: touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file-removed out : []
          Mar 11, 2014 12:37:41 AM org.apache.commons.logging.Log$info$0 call
          INFO: Cleaning after rm /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file-removed
          JUnit 4 Runner, Tests: 8, Failures: 0, Time: 8547
          [root@vagrant bigtop]# 
          
          
          Show
          jay vyas added a comment - hi folks. i know its a busy week for everyone so might take a few more days to get this commited, ... but just FYI have now run a final test of this patch to confirm it works in its existing state. Its definetly ready to roll ! [root@vagrant bigtop]# groovy -classpath /usr/lib/hadoop/hadoop-common-2.0.6-alpha.jar:/root/.m2/repository/org/apache/bigtop/itest/itest-common/0.8.0-SNAPSHOT/itest-common-0.8.0-SNAPSHOT.jar:/usr/lib/hadoop/lib/guava-11.0.2.jar:/etc/hadoop/conf/ bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hcfs/TestFuseHCFS.groovy Mar 11, 2014 12:37:33 AM org.apache.commons.logging.Log$info$0 call INFO: mounting dfs://vagrant.bigtop1:17020 on /tmp/hcfs-test Mar 11, 2014 12:37:35 AM org.apache.commons.logging.Log$info$0 call INFO: /bin/cp -rf /tmp/FUSETEST_bigtop /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cf2 out : [] Mar 11, 2014 12:37:37 AM org.apache.commons.logging.Log$info$0 call INFO: cat output = [hi_bigtop, hi_bigtop] [] 0 Mar 11, 2014 12:37:37 AM org.apache.commons.logging.Log$info$0 call INFO: Cleaning after sleep 2 && cat /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cf2 Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call INFO: [/tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1] Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call INFO: Cleaning after mkdir /tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1 && cd /tmp/hcfs-test/user/root/TestFuseDFS-testDir/dir1 && pwd Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call INFO: [file1] Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call INFO: Cleaning after touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file1 && ls /tmp/hcfs-test/user/root/TestFuseDFS-testDir Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call INFO: After cd, pwd=/tmp/hcfs-test/user/root/TestFuseDFS-testDir Mar 11, 2014 12:37:38 AM org.apache.commons.logging.Log$info$0 call INFO: Cleaning after cd /tmp/hcfs-test/user/root/TestFuseDFS-testDir && pwd Mar 11, 2014 12:37:39 AM org.apache.commons.logging.Log$info$0 call INFO: Cleaning after mkdir /tmp/hcfs-test/user/root/TestFuseDFS-testDir/targetdir &&echo ABCDEFGHIJKLMNOPZUIRPIEOF > /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp1 && echo ABCDEFGHIJKLMNOPZUIRPIEOF > /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp2 && /bin/cp -rf /tmp/hcfs-test/user/root/TestFuseDFS-testDir/cp* /tmp/hcfs-test/user/root/TestFuseDFS-testDir/targetdir/ Mar 11, 2014 12:37:39 AM org.apache.commons.logging.Log$info$0 call INFO: touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/non-trivial-fn out : [] Mar 11, 2014 12:37:40 AM org.apache.commons.logging.Log$info$0 call INFO: Cleaning after ls -altrh /tmp/hcfs-test/user/root/TestFuseDFS-testDir Mar 11, 2014 12:37:40 AM org.apache.commons.logging.Log$info$0 call INFO: mkdir -p /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1 && touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1/innerfile out : [] Mar 11, 2014 12:37:40 AM org.apache.commons.logging.Log$info$0 call INFO: Cleaning after mv /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir1 /tmp/hcfs-test/user/root/TestFuseDFS-testDir/subdir2 Mar 11, 2014 12:37:41 AM org.apache.commons.logging.Log$info$0 call INFO: touch /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file-removed out : [] Mar 11, 2014 12:37:41 AM org.apache.commons.logging.Log$info$0 call INFO: Cleaning after rm /tmp/hcfs-test/user/root/TestFuseDFS-testDir/file-removed JUnit 4 Runner, Tests: 8, Failures: 0, Time: 8547 [root@vagrant bigtop]#
          Hide
          Mark Grover added a comment -

          Hey Jay, Sorry about the delay, been super busy but will review soon'ish.

          Show
          Mark Grover added a comment - Hey Jay, Sorry about the delay, been super busy but will review soon'ish.
          Hide
          jay vyas added a comment -

          no problem mark thanks for all the feedback.!

          Show
          jay vyas added a comment - no problem mark thanks for all the feedback.!
          Hide
          jay vyas added a comment - - edited

          bump any chance i can get a quick review on that last diff Mark Grover . i think we're realllllly close this time !

          https://reviews.apache.org/r/18593/diff/3-7/

          Show
          jay vyas added a comment - - edited bump any chance i can get a quick review on that last diff Mark Grover . i think we're realllllly close this time ! https://reviews.apache.org/r/18593/diff/3-7/
          Hide
          Mark Grover added a comment -

          I agree, Jay! I will take a look this weekend.

          Show
          Mark Grover added a comment - I agree, Jay! I will take a look this weekend.
          Hide
          Mark Grover added a comment -

          Hi Jay, I really appreciate your patience and of course, your work. I finally got a chance to review the latest patch on reviewboard and it looks good to me.
          So, you have my +1.

          Konstantin Boudnik since you were reviewing this patch as well, do you have any final comments before I commit this?

          Show
          Mark Grover added a comment - Hi Jay, I really appreciate your patience and of course, your work. I finally got a chance to review the latest patch on reviewboard and it looks good to me. So, you have my +1. Konstantin Boudnik since you were reviewing this patch as well, do you have any final comments before I commit this?
          Hide
          jay vyas added a comment -

          Thanks mark!!I appreciate your attention to detail - this is my first apache project to actively contribute to so still learning the ropes ! this has allowed me to get up to speed w/ some good tooling.

          moving forward maybe you can help guide BIGTOP-1240 and BIGTOP-1249, which will both help increase the quality of code coming into bigtop and make it easier to review the "important" part of patches in the future.

          Show
          jay vyas added a comment - Thanks mark!!I appreciate your attention to detail - this is my first apache project to actively contribute to so still learning the ropes ! this has allowed me to get up to speed w/ some good tooling. moving forward maybe you can help guide BIGTOP-1240 and BIGTOP-1249 , which will both help increase the quality of code coming into bigtop and make it easier to review the "important" part of patches in the future.
          Hide
          Mark Grover added a comment -

          Sure thing, I have added myself to the watch list for those JIRAs. Thanks for your contributions.

          Show
          Mark Grover added a comment - Sure thing, I have added myself to the watch list for those JIRAs. Thanks for your contributions.
          Hide
          Konstantin Boudnik added a comment -

          Looks good from my side as well. Thanks Jay!

          Mark, please commit it or ping me if you're busy - I am doing some other commits today anyway. Thanks

          Show
          Konstantin Boudnik added a comment - Looks good from my side as well. Thanks Jay! Mark, please commit it or ping me if you're busy - I am doing some other commits today anyway. Thanks
          Hide
          Mark Grover added a comment -

          Hey Cos, I'd be happy to commit this but it may be a day or two, if you get to this before me, I'd totally appreciate you committing this!

          Show
          Mark Grover added a comment - Hey Cos, I'd be happy to commit this but it may be a day or two, if you get to this before me, I'd totally appreciate you committing this!
          Hide
          Konstantin Boudnik added a comment -

          Committed to the master. Thanks Jay!

          Show
          Konstantin Boudnik added a comment - Committed to the master. Thanks Jay!
          Konstantin Boudnik made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Mark Grover added a comment -

          Cos, I was just about to this now and noticed that you already did so, thanks so much!
          And, Jay, thank you again, for your contributing this change!

          Show
          Mark Grover added a comment - Cos, I was just about to this now and noticed that you already did so, thanks so much! And, Jay, thank you again, for your contributing this change!
          Hide
          Konstantin Boudnik added a comment -

          No worries Mark - as I said I was doing some other commits, so it wasn't a big detour

          Show
          Konstantin Boudnik added a comment - No worries Mark - as I said I was doing some other commits, so it wasn't a big detour
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          2d 39m 1 jay vyas 27/Feb/14 19:49
          Patch Available Patch Available Resolved Resolved
          17d 21h 10m 1 Konstantin Boudnik 17/Mar/14 17:00

            People

            • Assignee:
              jay vyas
              Reporter:
              jay vyas
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development