Hive
  1. Hive
  2. HIVE-6418

MapJoinRowContainer has large memory overhead in typical cases

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.13.0
    • Component/s: None
    • Labels:
      None
    1. HIVE-6418.WIP.patch
      44 kB
      Sergey Shelukhin
    2. HIVE-6418.patch
      62 kB
      Sergey Shelukhin
    3. HIVE-6418.05.patch
      63 kB
      Sergey Shelukhin
    4. HIVE-6418.04.patch
      62 kB
      Sergey Shelukhin
    5. HIVE-6418.04.patch
      62 kB
      Sergey Shelukhin
    6. HIVE-6418.03.patch
      61 kB
      Sergey Shelukhin
    7. HIVE-6418.02.patch
      56 kB
      Sergey Shelukhin
    8. HIVE-6418.01.patch
      55 kB
      Sergey Shelukhin

      Issue Links

        Activity

        Hide
        Lefty Leverenz added a comment -

        hive.mapjoin.lazy.hashtable is documented in the wiki here:

        Show
        Lefty Leverenz added a comment - hive.mapjoin.lazy.hashtable is documented in the wiki here: Configuration Properties: hive.mapjoin.lazy.hashtable
        Hide
        Sergey Shelukhin added a comment -

        committed to trunk

        Show
        Sergey Shelukhin added a comment - committed to trunk
        Hide
        Lefty Leverenz added a comment -

        This introduces a new config parameter, hive.mapjoin.lazy.hashtable, which needs documentation.

        But don't document it in hive-default.xml.template because that's going to be overwritten after HIVE-6037 commits. You could document it in a comment in HiveConf.java but most of the parameter comments will go away with HIVE-6037 and the descriptions will be incorporated into the parameter definitions.

        So maybe the best course is to document it here in a release note, then it can be added to HiveConf.java either before or after HIVE-6037 commits. (I'll make a note there about this new parameter.)

        Show
        Lefty Leverenz added a comment - This introduces a new config parameter, hive.mapjoin.lazy.hashtable , which needs documentation. But don't document it in hive-default.xml.template because that's going to be overwritten after HIVE-6037 commits. You could document it in a comment in HiveConf.java but most of the parameter comments will go away with HIVE-6037 and the descriptions will be incorporated into the parameter definitions. So maybe the best course is to document it here in a release note, then it can be added to HiveConf.java either before or after HIVE-6037 commits. (I'll make a note there about this new parameter.)
        Hide
        Sergey Shelukhin added a comment -

        Will commit in 24 hours...

        Show
        Sergey Shelukhin added a comment - Will commit in 24 hours...
        Hide
        Gunther Hagleitner added a comment -

        Looked at the changes since last review. Still looks good +1.

        Show
        Gunther Hagleitner added a comment - Looked at the changes since last review. Still looks good +1.
        Hide
        Sergey Shelukhin added a comment -

        Cannot repro either query or test failure. It looks like the query is flaky, judging by failures in other jiras.

        Show
        Sergey Shelukhin added a comment - Cannot repro either query or test failure. It looks like the query is flaky, judging by failures in other jiras.
        Hide
        Hive QA added a comment -

        Overall: -1 at least one tests failed

        Here are the results of testing the latest attachment:
        https://issues.apache.org/jira/secure/attachment/12630604/HIVE-6418.05.patch

        ERROR: -1 due to 2 failed/errored test(s), 5170 tests executed
        Failed tests:

        org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_auto_sortmerge_join_16
        org.apache.hive.service.cli.thrift.TestThriftHttpCLIService.org.apache.hive.service.cli.thrift.TestThriftHttpCLIService
        

        Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1488/testReport
        Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1488/console

        Messages:

        Executing org.apache.hive.ptest.execution.PrepPhase
        Executing org.apache.hive.ptest.execution.ExecutionPhase
        Executing org.apache.hive.ptest.execution.ReportingPhase
        Tests exited with: TestsFailedException: 2 tests failed
        

        This message is automatically generated.

        ATTACHMENT ID: 12630604

        Show
        Hive QA added a comment - Overall : -1 at least one tests failed Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12630604/HIVE-6418.05.patch ERROR: -1 due to 2 failed/errored test(s), 5170 tests executed Failed tests: org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_auto_sortmerge_join_16 org.apache.hive.service.cli.thrift.TestThriftHttpCLIService.org.apache.hive.service.cli.thrift.TestThriftHttpCLIService Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1488/testReport Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1488/console Messages: Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 2 tests failed This message is automatically generated. ATTACHMENT ID: 12630604
        Hide
        Sergey Shelukhin added a comment -

        add changes to non-tez out file. I think someone forgot to update tez files cause there are small unrelated changes in all outputs (database name pre/post hook), but they all pass

        Show
        Sergey Shelukhin added a comment - add changes to non-tez out file. I think someone forgot to update tez files cause there are small unrelated changes in all outputs (database name pre/post hook), but they all pass
        Hide
        Hive QA added a comment -

        Overall: -1 at least one tests failed

        Here are the results of testing the latest attachment:
        https://issues.apache.org/jira/secure/attachment/12630238/HIVE-6418.04.patch

        ERROR: -1 due to 1 failed/errored test(s), 5175 tests executed
        Failed tests:

        org.apache.hadoop.hive.cli.TestCliDriver.testCliDriver_mapjoin_mapjoin
        

        Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1457/testReport
        Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1457/console

        Messages:

        Executing org.apache.hive.ptest.execution.PrepPhase
        Executing org.apache.hive.ptest.execution.ExecutionPhase
        Executing org.apache.hive.ptest.execution.ReportingPhase
        Tests exited with: TestsFailedException: 1 tests failed
        

        This message is automatically generated.

        ATTACHMENT ID: 12630238

        Show
        Hive QA added a comment - Overall : -1 at least one tests failed Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12630238/HIVE-6418.04.patch ERROR: -1 due to 1 failed/errored test(s), 5175 tests executed Failed tests: org.apache.hadoop.hive.cli.TestCliDriver.testCliDriver_mapjoin_mapjoin Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1457/testReport Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1457/console Messages: Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 1 tests failed This message is automatically generated. ATTACHMENT ID: 12630238
        Hide
        Sergey Shelukhin added a comment -

        update, remove abstractlist
        one tez test passed, will run the rest

        Show
        Sergey Shelukhin added a comment - update, remove abstractlist one tez test passed, will run the rest
        Hide
        Sergey Shelukhin added a comment -

        will update this patch

        Show
        Sergey Shelukhin added a comment - will update this patch
        Hide
        Gunther Hagleitner added a comment -

        Minor nitpicks on RB. Otherwise LGTM +1. Are you planning to provide a new patch for the 8 additional bytes?

        Show
        Gunther Hagleitner added a comment - Minor nitpicks on RB. Otherwise LGTM +1. Are you planning to provide a new patch for the 8 additional bytes?
        Hide
        Sergey Shelukhin added a comment -

        Hmm, 8 more bytes per value can be shaved, it seems that new class inherits some useless member from abstractlist

        Show
        Sergey Shelukhin added a comment - Hmm, 8 more bytes per value can be shaved, it seems that new class inherits some useless member from abstractlist
        Hide
        Sergey Shelukhin added a comment -

        Remaining writable come from the key in that case, that is addressed in separate jira

        Show
        Sergey Shelukhin added a comment - Remaining writable come from the key in that case, that is addressed in separate jira
        Hide
        Sergey Shelukhin added a comment -

        Lazy deserialization-enabled table (before any activity, so no rows are deserialized) looks like this:

        Class Objects Shallow Size Retained Size
        byte[] 1000000 49968000 49968000
        java.lang.Object[] 2000000 48000000 121968000
        java.util.HashMap$Entry 1000000 32000000 201968000
        org.apache.hadoop.hive.ql.exec.persistence.LazyFlatRowContainer 1000000 32000000 105968000
        org.apache.hadoop.hive.serde2.io.DoubleWritable 1000000 24000000 24000000
        org.apache.hadoop.hive.ql.exec.persistence.MapJoinKey 1000000 16000000 64000000
        java.util.HashMap$Entry[] 1 8388624 210356624
        java.util.HashMap 1 48 210356672

        Savings of further 23% compared to fully eager table (at the cost of byte copying)

        Show
        Sergey Shelukhin added a comment - Lazy deserialization-enabled table (before any activity, so no rows are deserialized) looks like this: Class Objects Shallow Size Retained Size byte[] 1000000 49968000 49968000 java.lang.Object[] 2000000 48000000 121968000 java.util.HashMap$Entry 1000000 32000000 201968000 org.apache.hadoop.hive.ql.exec.persistence.LazyFlatRowContainer 1000000 32000000 105968000 org.apache.hadoop.hive.serde2.io.DoubleWritable 1000000 24000000 24000000 org.apache.hadoop.hive.ql.exec.persistence.MapJoinKey 1000000 16000000 64000000 java.util.HashMap$Entry[] 1 8388624 210356624 java.util.HashMap 1 48 210356672 Savings of further 23% compared to fully eager table (at the cost of byte copying)
        Hide
        Sergey Shelukhin added a comment -

        From applying this patch on my test, I get the following savings... HashMap with 1M entries, key is double, row is double and string (strings are short); one row per container (savings with multiple rows per key will be lower).
        Lazy part is disabled.

        Before (sorted by shallow size):

        Class Objects Shallow Size Retained Size
        java.lang.Object[] 3000000 80000000 232384000
        org.apache.hadoop.hive.serde2.io.DoubleWritable 3000000 72000000 72000000
        byte[] 1000000 32384000 32384000
        java.util.HashMap$Entry 1000000 32000000 328384000
        java.util.ArrayList 1000000 24000000 208384000
        org.apache.hadoop.hive.ql.exec.persistence.MapJoinEagerRowContainer 1000000 24000000 232384000
        org.apache.hadoop.hive.ql.exec.persistence.MapJoinEagerRowContainer$NoCopyingArrayList 1000000 24000000 160384000
        org.apache.hadoop.io.Text 1000000 24000000 56384000
        org.apache.hadoop.hive.ql.exec.persistence.MapJoinKey 1000000 16000000 64000000
        java.util.HashMap$Entry[] 1 8388624 336772624
        java.util.HashMap 1 48 336772672

        After:

        Class Objects Shallow Size Retained Size
        org.apache.hadoop.hive.serde2.io.DoubleWritable 3000000 72000000 72000000
        java.lang.Object[] 2000000 56000000 184384000
        byte[] 1000000 32384000 32384000
        java.util.HashMap$Entry 1000000 32000000 264384000
        org.apache.hadoop.hive.ql.exec.persistence.LazyFlatRowContainer 1000000 32000000 168384000
        org.apache.hadoop.io.Text 1000000 24000000 56384000
        org.apache.hadoop.hive.ql.exec.persistence.MapJoinKey 1000000 16000000 64000000
        java.util.HashMap$Entry[] 1 8388624 272772624
        java.util.HashMap 1 48 272772672

        Savings of 19~%

        Show
        Sergey Shelukhin added a comment - From applying this patch on my test, I get the following savings... HashMap with 1M entries, key is double, row is double and string (strings are short); one row per container (savings with multiple rows per key will be lower). Lazy part is disabled. Before (sorted by shallow size): Class Objects Shallow Size Retained Size java.lang.Object[] 3000000 80000000 232384000 org.apache.hadoop.hive.serde2.io.DoubleWritable 3000000 72000000 72000000 byte[] 1000000 32384000 32384000 java.util.HashMap$Entry 1000000 32000000 328384000 java.util.ArrayList 1000000 24000000 208384000 org.apache.hadoop.hive.ql.exec.persistence.MapJoinEagerRowContainer 1000000 24000000 232384000 org.apache.hadoop.hive.ql.exec.persistence.MapJoinEagerRowContainer$NoCopyingArrayList 1000000 24000000 160384000 org.apache.hadoop.io.Text 1000000 24000000 56384000 org.apache.hadoop.hive.ql.exec.persistence.MapJoinKey 1000000 16000000 64000000 java.util.HashMap$Entry[] 1 8388624 336772624 java.util.HashMap 1 48 336772672 After: Class Objects Shallow Size Retained Size org.apache.hadoop.hive.serde2.io.DoubleWritable 3000000 72000000 72000000 java.lang.Object[] 2000000 56000000 184384000 byte[] 1000000 32384000 32384000 java.util.HashMap$Entry 1000000 32000000 264384000 org.apache.hadoop.hive.ql.exec.persistence.LazyFlatRowContainer 1000000 32000000 168384000 org.apache.hadoop.io.Text 1000000 24000000 56384000 org.apache.hadoop.hive.ql.exec.persistence.MapJoinKey 1000000 16000000 64000000 java.util.HashMap$Entry[] 1 8388624 272772624 java.util.HashMap 1 48 272772672 Savings of 19~%
        Hide
        Sergey Shelukhin added a comment -

        Added config setting

        Show
        Sergey Shelukhin added a comment - Added config setting
        Hide
        Hive QA added a comment -

        Overall: +1 all checks pass

        Here are the results of testing the latest attachment:
        https://issues.apache.org/jira/secure/attachment/12628945/HIVE-6418.02.patch

        SUCCESS: +1 5095 tests passed

        Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1321/testReport
        Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1321/console

        Messages:

        Executing org.apache.hive.ptest.execution.PrepPhase
        Executing org.apache.hive.ptest.execution.ExecutionPhase
        Executing org.apache.hive.ptest.execution.ReportingPhase
        

        This message is automatically generated.

        ATTACHMENT ID: 12628945

        Show
        Hive QA added a comment - Overall : +1 all checks pass Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12628945/HIVE-6418.02.patch SUCCESS: +1 5095 tests passed Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1321/testReport Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1321/console Messages: Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase This message is automatically generated. ATTACHMENT ID: 12628945
        Hide
        Sergey Shelukhin added a comment -

        tiny fix to serde to make last tez tests pass

        Show
        Sergey Shelukhin added a comment - tiny fix to serde to make last tez tests pass
        Hide
        Sergey Shelukhin added a comment -

        Added correct handling on 0-length rows (main issue), got rid of some array allocations when deserializing, other minor changes and fixes. I have run minitez tests and they passed except for mapjoin_mapjoin; with this patch, that passes, I am rerunning all tests now.

        Show
        Sergey Shelukhin added a comment - Added correct handling on 0-length rows (main issue), got rid of some array allocations when deserializing, other minor changes and fixes. I have run minitez tests and they passed except for mapjoin_mapjoin; with this patch, that passes, I am rerunning all tests now.
        Show
        Sergey Shelukhin added a comment - https://reviews.apache.org/r/18061/
        Hide
        Sergey Shelukhin added a comment -

        please ignore q file changes

        Show
        Sergey Shelukhin added a comment - please ignore q file changes
        Hide
        Sergey Shelukhin added a comment -

        Fix some issues. Need to figure out whether the "lazy" part provides benefits, now that it requires byte array copy. It would really depend on how many entries actually get used, perhaps it could be configurable

        Show
        Sergey Shelukhin added a comment - Fix some issues. Need to figure out whether the "lazy" part provides benefits, now that it requires byte array copy. It would really depend on how many entries actually get used, perhaps it could be configurable
        Hide
        Sergey Shelukhin added a comment -

        oh, MJRC became an interface and I renamed the original one into "Eager" because I can't come up with a good interface name otherwise

        Show
        Sergey Shelukhin added a comment - oh, MJRC became an interface and I renamed the original one into "Eager" because I can't come up with a good interface name otherwise
        Hide
        Sergey Shelukhin added a comment -
        Show
        Sergey Shelukhin added a comment - Gopal V Ashutosh Chauhan fyi
        Hide
        Sergey Shelukhin added a comment -

        First cut.
        Introduces an alternative container that basically has an array. Initially that just stores context and all the un-serialized writables.
        On access, it deserializes the writables. It knows the row count at that point and can determine row length from the first deserialized row (assumes its the same), so array represents a matrix with this row length.
        For simple case of one row, it also serves as a list, so it can return itself as that "row". Otherwise it returns a readonly sublist.
        Works for Tez, because Tez doesn't have to serialize/deserialize the hashtable. I am not sure the lazy part can be made to work for MR with its extra stage, probably not, so MR uses old container.

        WIP:
        Need to get rid of index stored in each row, since unless rowCount is made short it will round to 8 bytes I presume and it's really useless.
        Also need to run more tests, I ran some tez tests

        Show
        Sergey Shelukhin added a comment - First cut. Introduces an alternative container that basically has an array. Initially that just stores context and all the un-serialized writables. On access, it deserializes the writables. It knows the row count at that point and can determine row length from the first deserialized row (assumes its the same), so array represents a matrix with this row length. For simple case of one row, it also serves as a list, so it can return itself as that "row". Otherwise it returns a readonly sublist. Works for Tez, because Tez doesn't have to serialize/deserialize the hashtable. I am not sure the lazy part can be made to work for MR with its extra stage, probably not, so MR uses old container. WIP: Need to get rid of index stored in each row, since unless rowCount is made short it will round to 8 bytes I presume and it's really useless. Also need to run more tests, I ran some tez tests

          People

          • Assignee:
            Sergey Shelukhin
            Reporter:
            Sergey Shelukhin
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development