Uploaded image for project: 'IMPALA'
  1. IMPALA
  2. IMPALA-5120

Consider defaulting to partitioned join when no stats are available.

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: Impala 2.9.0
    • Fix Version/s: Impala 2.9.0
    • Component/s: Frontend
    • Labels:
      None

      Description

      We currently default to broadcast join when no stats are available, since the code estimates are both MAX_LONG and in the case of equal costs, broadcast wins. We should consider making partitioned join the default because it will use less memory.

      The code is here: https://github.com/apache/incubator-impala/blob/master/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java#L509

          && (node.getDistributionModeHint() == DistributionMode.BROADCAST
                  || (node.getDistributionModeHint() != DistributionMode.PARTITIONED
      && broadcastCost <= partitionCost)))
      

        Issue Links

          Activity

          Hide
          mmokhtar Mostafa Mokhtar added a comment -

          I agree with Alex, we shouldn't limit planner changes/improvements to C6.
          From a resource consumption point of view this is an improvement.

          This is a plan change, doesn't change results or functionality so I wouldn't consider this to be a behavior change.

          Show
          mmokhtar Mostafa Mokhtar added a comment - I agree with Alex, we shouldn't limit planner changes/improvements to C6. From a resource consumption point of view this is an improvement. This is a plan change, doesn't change results or functionality so I wouldn't consider this to be a behavior change.
          Hide
          alex.behm Alexander Behm added a comment -

          Juan Yu, I don't think planner behavior necessarily belongs to the "public API" of Impala where we guarantee some level of stability for applications. The same SQL is accepted, but may produce a different plan. We have considered that the performance of queries without stats may regress. However, if you have no stats on your tables and if you had an acceptable plan it was due to sheer luck. If your plan turns bad, please compute stats. If your plan is still bad with stats, we will help.

          What do you think?

          Show
          alex.behm Alexander Behm added a comment - Juan Yu , I don't think planner behavior necessarily belongs to the "public API" of Impala where we guarantee some level of stability for applications. The same SQL is accepted, but may produce a different plan. We have considered that the performance of queries without stats may regress. However, if you have no stats on your tables and if you had an acceptable plan it was due to sheer luck. If your plan turns bad, please compute stats. If your plan is still bad with stats, we will help. What do you think?
          Hide
          jyu@cloudera.com Juan Yu added a comment -

          Although I agree this reduce the chance of disastrous plan, this changes Impala's behavior (in a dot release) and could cause performance regression in some scenarios
          e.g. left table large, right table small, both missing stats, plan changed from broadcast to shuffle and execution could take much longer time.
          Any consideration about such scenarios?

          Show
          jyu@cloudera.com Juan Yu added a comment - Although I agree this reduce the chance of disastrous plan, this changes Impala's behavior (in a dot release) and could cause performance regression in some scenarios e.g. left table large, right table small, both missing stats, plan changed from broadcast to shuffle and execution could take much longer time. Any consideration about such scenarios?
          Hide
          twmarshall Thomas Tauber-Marshall added a comment -

          commit aca07ee8160bbea0812dc4ba3c08dff818240d22
          Author: Thomas Tauber-Marshall <tmarshall@cloudera.com>
          Date: Thu May 4 13:51:08 2017 -0700

          IMPALA-5120: Default to partitioned join when stats are missing

          Previously, we defaulted to broadcast join when stats were
          missing, but this can lead to disastrous plans when the
          right hand side is actually large.

          Its always difficult to make good plans when stats are missing,
          but defaulting to partitioned joins should reduce the risk of
          disastrous plans.

          Testing:

          • Added a planner test that joins a table with no stats.

          Change-Id: Ie168ecfcd5e7c5d3c60d16926c151f8f134c81e0
          Reviewed-on: http://gerrit.cloudera.org:8080/6803
          Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
          Tested-by: Impala Public Jenkins

          Show
          twmarshall Thomas Tauber-Marshall added a comment - commit aca07ee8160bbea0812dc4ba3c08dff818240d22 Author: Thomas Tauber-Marshall <tmarshall@cloudera.com> Date: Thu May 4 13:51:08 2017 -0700 IMPALA-5120 : Default to partitioned join when stats are missing Previously, we defaulted to broadcast join when stats were missing, but this can lead to disastrous plans when the right hand side is actually large. Its always difficult to make good plans when stats are missing, but defaulting to partitioned joins should reduce the risk of disastrous plans. Testing: Added a planner test that joins a table with no stats. Change-Id: Ie168ecfcd5e7c5d3c60d16926c151f8f134c81e0 Reviewed-on: http://gerrit.cloudera.org:8080/6803 Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com> Tested-by: Impala Public Jenkins
          Hide
          tarmstrong Tim Armstrong added a comment -

          That's a good point, shuffling to one node would not help things at all. In one example I saw, the cardinality of one side was known and very small, while the other side was unknown, so maybe a different mitigation would have helped - if one side is known to be reasonably small, put it on the right. However, it's not clear how much effort we want to put into optimising such cases.

          Show
          tarmstrong Tim Armstrong added a comment - That's a good point, shuffling to one node would not help things at all. In one example I saw, the cardinality of one side was known and very small, while the other side was unknown, so maybe a different mitigation would have helped - if one side is known to be reasonably small, put it on the right. However, it's not clear how much effort we want to put into optimising such cases.
          Hide
          alex.behm Alexander Behm added a comment -

          Tim Armstrong, the left-most table determines the number of hosts which might be 1 (or very few nodes).

          Show
          alex.behm Alexander Behm added a comment - Tim Armstrong , the left-most table determines the number of hosts which might be 1 (or very few nodes).
          Hide
          tarmstrong Tim Armstrong added a comment -

          Hmm, so in that case we don't repartition between all of the available hosts?

          Show
          tarmstrong Tim Armstrong added a comment - Hmm, so in that case we don't repartition between all of the available hosts?
          Hide
          alex.behm Alexander Behm added a comment -

          Marcel Kornacker, Mostafa Mokhtar, Tim Armstrong, even with this change, we can end up with disaster scenarios if the left-most table has stats and resides on one host only. We have seen a few cases like that in the wild. Do you think we should worry about that scenario? Any mitigation suggestions?

          Show
          alex.behm Alexander Behm added a comment - Marcel Kornacker , Mostafa Mokhtar , Tim Armstrong , even with this change, we can end up with disaster scenarios if the left-most table has stats and resides on one host only. We have seen a few cases like that in the wild. Do you think we should worry about that scenario? Any mitigation suggestions?
          Hide
          alex.behm Alexander Behm added a comment -

          Matthew Jacobs, I increased the priority because we have been seeing several cases with disastrous plans due to missing stats. We can make some plans less disastrous and save users and us the headcache.

          Yes, the code change should be pretty trivial.

          Show
          alex.behm Alexander Behm added a comment - Matthew Jacobs , I increased the priority because we have been seeing several cases with disastrous plans due to missing stats. We can make some plans less disastrous and save users and us the headcache. Yes, the code change should be pretty trivial.
          Hide
          mjacobs Matthew Jacobs added a comment -

          It looks like the code change should be pretty trivial, is that right?

          Show
          mjacobs Matthew Jacobs added a comment - It looks like the code change should be pretty trivial, is that right?
          Hide
          mjacobs Matthew Jacobs added a comment -

          Alexander Behm why bump this to critical?

          Show
          mjacobs Matthew Jacobs added a comment - Alexander Behm why bump this to critical?
          Hide
          alex.behm Alexander Behm added a comment -

          Thomas Tauber-Marshall, can you take this one? Feel free to assign back to me if you are already swamped.

          Show
          alex.behm Alexander Behm added a comment - Thomas Tauber-Marshall , can you take this one? Feel free to assign back to me if you are already swamped.

            People

            • Assignee:
              twmarshall Thomas Tauber-Marshall
              Reporter:
              tarmstrong Tim Armstrong
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development