Hive
  1. Hive
  2. HIVE-1802

Encode MapReduce Shuffling Keys Differently for Single string/bigint Key

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Query Processor
    • Labels:
      None

      Description

      Delimiters are not needed if we only have one shuffling key, and in the same time escaping delimiters are not needed. We can save some CPU time on serializing and shuffle slightly less amount of data to save memory footprint and network traffic.

      Also there is a bug that for group-by, we by mistake add a -1 to the end of the key and pay one more unnecessary mem-copy. Can be easily fixed.

      1. HIVE-1802.2.patch
        102 kB
        Siying Dong
      2. HIVE-1802.1.patch
        34 kB
        Siying Dong

        Activity

        Hide
        Carl Steinbach added a comment -

        The patch doesn't apply cleanly to trunk. Can you please rebase it? Thanks.

        Show
        Carl Steinbach added a comment - The patch doesn't apply cleanly to trunk. Can you please rebase it? Thanks.
        Hide
        He Yongqiang added a comment -

        Yes. I am ok with the current approach.

        (moving forward, we still need to figure out a better way which can be more easy to maintain and extend. Like, we may want to try to separate serdes used for group-by and join. If we do that in the current approach, we need to have 4 serdes for reduce-sink.)

        Show
        He Yongqiang added a comment - Yes. I am ok with the current approach. (moving forward, we still need to figure out a better way which can be more easy to maintain and extend. Like, we may want to try to separate serdes used for group-by and join. If we do that in the current approach, we need to have 4 serdes for reduce-sink.)
        Hide
        Siying Dong added a comment -

        Yongqiang, after some face-to-face discussion, are you OK with go for this approach now?

        Show
        Siying Dong added a comment - Yongqiang, after some face-to-face discussion, are you OK with go for this approach now?
        Hide
        Siying Dong added a comment -

        I still don't get it. Isn't it what this patch is doing?

        Show
        Siying Dong added a comment - I still don't get it. Isn't it what this patch is doing?
        Hide
        He Yongqiang added a comment -

        >>For any Group by, we needed 2 mem-copies. One from Text objects to buffer, one add an extra tag to the end of the buffer.
        I think for Join we will need array copy and put a tag at the end.

        I mean optimize BinarySortableSerDe might be a better idea to optimize cases when need array copy.
        The code can be cleaner and simpler if only optimize the one Text key case in Group by, and put other optimizations in BinarySortableSerDe.

        Show
        He Yongqiang added a comment - >>For any Group by, we needed 2 mem-copies. One from Text objects to buffer, one add an extra tag to the end of the buffer. I think for Join we will need array copy and put a tag at the end. I mean optimize BinarySortableSerDe might be a better idea to optimize cases when need array copy. The code can be cleaner and simpler if only optimize the one Text key case in Group by, and put other optimizations in BinarySortableSerDe.
        Hide
        Siying Dong added a comment -

        For any Group by, we needed 2 mem-copies. One from Text objects to buffer, one add an extra tag to the end of the buffer.
        Now, the case with single Text takes no mem-copy (except the first byte is 0) and for multiple keys it needs one (from Text object to buffer).

        For join, we needed 2 mem-copies. One from Text to buffer, one add tag.
        Now one single Text needs one copy from buffer to add a tag. Other cases we still need two copies.

        Show
        Siying Dong added a comment - For any Group by, we needed 2 mem-copies. One from Text objects to buffer, one add an extra tag to the end of the buffer. Now, the case with single Text takes no mem-copy (except the first byte is 0) and for multiple keys it needs one (from Text object to buffer). For join, we needed 2 mem-copies. One from Text to buffer, one add tag. Now one single Text needs one copy from buffer to add a tag. Other cases we still need two copies.
        Hide
        Siying Dong added a comment -

        Refactored PlanUtils a little bit. I didn't come up a straight forward way to refactor to be a factory and make it clear. I tried to break up PlanUtils to several classes, by those return TableDesc and ReduceSyncDesc, as well as others. Hope it can be better maintained.

        No function change from previous patch.

        Show
        Siying Dong added a comment - Refactored PlanUtils a little bit. I didn't come up a straight forward way to refactor to be a factory and make it clear. I tried to break up PlanUtils to several classes, by those return TableDesc and ReduceSyncDesc, as well as others. Hope it can be better maintained. No function change from previous patch.
        Hide
        He Yongqiang added a comment -

        For one Text key in join, i think in your patch you still need an array copy. For one Text key in group by, array copy is not needed.

        I mean the new code only process one Text key in Group by, which we can avoid array copy.

        For other cases, maybe we can optimize BinarySortableSerDe to use array copy instead of write?

        Show
        He Yongqiang added a comment - For one Text key in join, i think in your patch you still need an array copy. For one Text key in group by, array copy is not needed. I mean the new code only process one Text key in Group by, which we can avoid array copy. For other cases, maybe we can optimize BinarySortableSerDe to use array copy instead of write?
        Hide
        Siying Dong added a comment -

        Yongqiang, I didn't quite get it. One key applies to both of Group-by and Join. And we ARE only processing those two cases. And we are avoiding array copy in those case. It's exactly what we are doing here.

        Are you suggesting we should also optimize other cases too? It will be nice if we can. I didn't come up with a way that let BinarySortableSerDe to use array copy. The problem is that to make binary sorting order the same as key order, we need a delimiter and in order to have delimiter, strings need to be encoded to escape the delimiter. Any better idea?

        Show
        Siying Dong added a comment - Yongqiang, I didn't quite get it. One key applies to both of Group-by and Join. And we ARE only processing those two cases. And we are avoiding array copy in those case. It's exactly what we are doing here. Are you suggesting we should also optimize other cases too? It will be nice if we can. I didn't come up with a way that let BinarySortableSerDe to use array copy. The problem is that to make binary sorting order the same as key order, we need a delimiter and in order to have delimiter, strings need to be encoded to escape the delimiter. Any better idea?
        Hide
        He Yongqiang added a comment -

        I think we only need serialize here. No? Can we make it easier? I mean only processing cases where there is one key, and type is Text, and also only for group by. In this case, we can avoid an array copy.

        But if it is a join, or there are multiple keys in group by, we anyway need to do array copy. The problem of binarysortableserde is that it uses write() to write bytes. Can we make binarysortableserde to use array copy? Maybe we can use some java nio classes, like ByteBuffer?

        Show
        He Yongqiang added a comment - I think we only need serialize here. No? Can we make it easier? I mean only processing cases where there is one key, and type is Text, and also only for group by. In this case, we can avoid an array copy. But if it is a join, or there are multiple keys in group by, we anyway need to do array copy. The problem of binarysortableserde is that it uses write() to write bytes. Can we make binarysortableserde to use array copy? Maybe we can use some java nio classes, like ByteBuffer?
        Hide
        Namit Jain added a comment -

        The code looks OK, but it is not very easy to add new serde's this way.
        Can you refactor PlanUtils change into a factory - so that it is easy to add new changes

        Show
        Namit Jain added a comment - The code looks OK, but it is not very easy to add new serde's this way. Can you refactor PlanUtils change into a factory - so that it is easy to add new changes
        Hide
        Siying Dong added a comment -

        1. Two another SerDe only for encoding single string and single bigint, respectively.
        2. When generating reduce plan, identify single sting and bigint case and write the serde in the plan
        3. add a test for key as bigint as keys
        4. fix the bug of adding FF to the end of group-by keys and pay one more mem-copy.

        Show
        Siying Dong added a comment - 1. Two another SerDe only for encoding single string and single bigint, respectively. 2. When generating reduce plan, identify single sting and bigint case and write the serde in the plan 3. add a test for key as bigint as keys 4. fix the bug of adding FF to the end of group-by keys and pay one more mem-copy.

          People

          • Assignee:
            Siying Dong
            Reporter:
            Siying Dong
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:

              Development