Uploaded image for project: 'Apache Ozone'
  1. Apache Ozone
  2. HDDS-2939 Ozone FS namespace
  3. HDDS-5097

[FSO] Cleanup integration tests and reduce the execution time

    XMLWordPrintableJSON

    Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None

      Description

      Apache organization has a shared, organization-level resource limit for Github actions. All the projects should use the available minutes fairly to avoid blocking the build of other Apache projects.

      This is discussed recently here: https://lists.apache.org/thread.html/r48d079eeff292254db22705c8ef8618f87ff7adc68d56c4e5d0b4105%40%3Cbuilds.apache.org%3E, but same mailing list has older threads, too.

      HDDS-2939 branch build time has been increased significantly. The problem with acceptance tests are tracked with HDDS-5093.

      But integration tests are also increased from ~56 minutes to 94 minutes (~158%)

      We need to avoid such increase unless we have very good reason to have it.

      If we check the execution time of the tests on the branch and the master we can see the problem in more details:

       tmp  %  grep -e 'Tests run: .* in org' "20" | awk -F '[,:\-]' '{print $6, $8, $10, $12, $14, $15}' | sed 's/s   in //g' | sort -n -k5 -r | head
      awk: warning: escape sequence `\-' treated as plain `-'
       104  0  0  0  478.951 org.apache.hadoop.fs.ozone.TestRootedOzoneFileSystem
       88  0  0  0  458.703 org.apache.hadoop.fs.ozone.TestOzoneFileSystem
       18  0  0  2  436.521 org.apache.hadoop.fs.ozone.TestOzoneFileInterfaces
       6  0  0  0  152.808 org.apache.hadoop.fs.ozone.TestOzoneFSWithObjectStoreCreate
       4  0  0  0  108.191 org.apache.hadoop.hdds.scm.pipeline.TestPipelineClose
       1  0  0  0  88.905 org.apache.hadoop.hdds.scm.pipeline.TestNodeFailure
       2  0  0  0  74.192 org.apache.hadoop.fs.ozone.TestOzoneFsHAURLs
       19  0  0  0  58.2 org.apache.hadoop.fs.ozone.contract.rooted.ITestRootedOzoneContractGetFileStatus
       8  0  0  0  57.063 org.apache.hadoop.fs.ozone.contract.rooted.ITestRootedOzoneContractDistCp
       8  0  0  0  54.612 org.apache.hadoop.fs.ozone.contract.ITestOzoneContractDistCp
       tmp  %  grep -e 'Tests run: .* in org' "18" | awk -F '[,:\-]' '{print $6, $8, $10, $12, $14, $15}' | sed 's/s   in //g' | sort -n -k5 -r | head
      awk: warning: escape sequence `\-' treated as plain `-'
       128  0  0  0  509.388 org.apache.hadoop.fs.ozone.TestRootedOzoneFileSystem
       132  0  0  0  508.585 org.apache.hadoop.fs.ozone.TestOzoneFileSystem
       140  0  0  8  479.578 org.apache.hadoop.fs.ozone.TestOzoneFileSystemV1
       18  0  0  2  475.686 org.apache.hadoop.fs.ozone.TestOzoneFileInterfaces
       128  0  0  0  470.619 org.apache.hadoop.fs.ozone.TestRootedOzoneFileSystemV1
       18  0  0  6  329.812 org.apache.hadoop.fs.ozone.TestOzoneFileInterfacesV1
       5  0  0  0  226.093 org.apache.hadoop.fs.ozone.TestDirectoryDeletingServiceWithFSOBucket
       6  0  0  0  171.861 org.apache.hadoop.fs.ozone.TestOzoneFSWithObjectStoreCreate
       4  0  0  0  139.951 org.apache.hadoop.hdds.scm.pipeline.TestPipelineClose
       3  0  0  0  82.128 org.apache.hadoop.fs.ozone.TestOzoneFileSystemPrefixParser
      

      It turned out that a lot of tests just copied with V1 prefix with small modification (or original test extended with V1 which means the methods of the old and new tests are executed) instead of improving the original test to cover both of the cases (simple/prefix-ed).

      Also: the same functionality seems to be tested on multiple levels (FileSystemInterface, OMRequest, acceptance test...)

      This copy can make the maintenance of the tests slightly harder, and the V1 prefix is quite meaningless and confusing.

      From the legendary "Clean code" book:

      >Programmers create problems for themselves when they write code solely to satisfy a compiler or interpreter. For example, because you can’t use the same name to refer to two different things in the same scope, you might be tempted to change one name in an arbitrary way. Sometimes this is done by misspelling one, leading to the surprising situation where correcting spelling errors leads to an inability to compile.2

      > [...] It is not sufficient to add number series or noise words, even though the compiler is satisfied. If names must be different, then they should also mean something different.

      > Number-series naming (a1, a2, .. aN) is the opposite of intentional naming. Such names are not disinformative—they are noninformative; they provide no clue to the author’s intention....

      I totally agree with this section, I think we should avoid using V1 prefixes everywhere.

      Suggestions:

      1. We should minimize the additional build time when possible.
      2. Similar to HDDS-5093 instead of creating a new dimension to the parametrized tests (which double the execution time) we should carefully select the meaningful sets of parameters. (When we have 3 parameters for a unit test with 2 values for each it's already 8 possible executions. Execution all of them with and without prefix may not be required, it seems to be better to select representative parameter sets and use only them).
      3. The V1 prefixes should be avoided. When possible we need to modify the original unit tests with adding additional parameters. It makes it easier to maintain the unit tests, and it requires less code and (together with the previous point) less time
      4. We don't need to repeat ALL the tests IMHO. For example if we modified only the rename part of the OzoneFileSystem, I assume that it's enough to test that part with/without prefix (especially as the prefix handling is already tested with testing om requests...)

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                rakeshr Rakesh Radhakrishnan
                Reporter:
                elek Marton Elek
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: