Set HashJoin seed #15783

ctsk · 2025-04-20T16:36:54Z

Which issue does this PR close?

Closes Questionable hash seed reuse between RepartitionExec and HashJoinExec #15620.

What changes are included in this PR?

The hash join seed is hard-coded to a different value that the RepartitionExec seed.

Are these changes tested?

Covered by existing tests.

Are there any user-facing changes?

No.

datafusion/physical-plan/src/joins/hash_join.rs

Co-authored-by: Alex Huang <[email protected]>

alamb · 2025-04-29T01:09:52Z

🤖 ./gh_compare_branch.sh Benchmark Script Running
Linux aal-dev 6.11.0-1013-gcp #13~24.04.1-Ubuntu SMP Wed Apr 2 16:34:16 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing fix/hash-join-seed (e1fc96d) to 8a193c2 diff
Benchmarks: tpch_mem clickbench_partitioned clickbench_extended
Results will be posted here when complete

alamb · 2025-04-29T01:10:17Z

datafusion/physical-plan/src/joins/hash_join.rs

@@ -86,6 +86,10 @@ use datafusion_physical_expr_common::physical_expr::fmt_sql;
 use futures::{ready, Stream, StreamExt, TryStreamExt};
 use parking_lot::Mutex;

+/// Hard-coded seed to ensure hash values from the hash join differ from `RepartitionExec`, avoiding collisions.
+const HASH_JOIN_SEED: RandomState =
+    RandomState::with_seeds('J' as u64, 'O' as u64, 'I' as u64, 'N' as u64);


alamb

Thanks @ctsk -- this makes sense to me. I am running a few benchmarks to make sure this doesn't cause any regressions, but overall it looks good to me

alamb · 2025-04-29T01:53:42Z

🤖: Benchmark completed

Details

Comparing HEAD and fix_hash-join-seed
--------------------
Benchmark clickbench_extended.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Query        ┃       HEAD ┃ fix_hash-join-seed ┃    Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ QQuery 0     │  2000.85ms │          1972.89ms │ no change │
│ QQuery 1     │   714.99ms │           712.92ms │ no change │
│ QQuery 2     │  1487.22ms │          1481.43ms │ no change │
│ QQuery 3     │   718.29ms │           698.86ms │ no change │
│ QQuery 4     │  1458.20ms │          1457.44ms │ no change │
│ QQuery 5     │ 15666.90ms │         15335.98ms │ no change │
│ QQuery 6     │  2082.26ms │          2092.29ms │ no change │
└──────────────┴────────────┴────────────────────┴───────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary                 ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)                 │ 24128.70ms │
│ Total Time (fix_hash-join-seed)   │ 23751.80ms │
│ Average Time (HEAD)               │  3446.96ms │
│ Average Time (fix_hash-join-seed) │  3393.11ms │
│ Queries Faster                    │          0 │
│ Queries Slower                    │          0 │
│ Queries with No Change            │          7 │
└───────────────────────────────────┴────────────┘
--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃       HEAD ┃ fix_hash-join-seed ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0     │     2.17ms │             2.22ms │     no change │
│ QQuery 1     │    37.42ms │            37.09ms │     no change │
│ QQuery 2     │    92.65ms │            90.26ms │     no change │
│ QQuery 3     │   103.22ms │            97.99ms │ +1.05x faster │
│ QQuery 4     │   887.43ms │           740.13ms │ +1.20x faster │
│ QQuery 5     │   897.35ms │           867.53ms │     no change │
│ QQuery 6     │     2.43ms │             2.32ms │     no change │
│ QQuery 7     │    44.13ms │            42.08ms │     no change │
│ QQuery 8     │   945.09ms │           901.37ms │     no change │
│ QQuery 9     │  1249.21ms │          1241.44ms │     no change │
│ QQuery 10    │   279.17ms │           274.20ms │     no change │
│ QQuery 11    │   315.83ms │           312.81ms │     no change │
│ QQuery 12    │   957.03ms │           916.08ms │     no change │
│ QQuery 13    │  1374.90ms │          1355.30ms │     no change │
│ QQuery 14    │   876.51ms │           853.30ms │     no change │
│ QQuery 15    │  1056.31ms │          1033.93ms │     no change │
│ QQuery 16    │  1801.98ms │          1744.01ms │     no change │
│ QQuery 17    │  1645.60ms │          1614.23ms │     no change │
│ QQuery 18    │  3144.80ms │          3093.86ms │     no change │
│ QQuery 19    │    86.32ms │            85.42ms │     no change │
│ QQuery 20    │  1161.10ms │          1155.16ms │     no change │
│ QQuery 21    │  1363.22ms │          1365.95ms │     no change │
│ QQuery 22    │  2284.72ms │          2297.40ms │     no change │
│ QQuery 23    │  8516.40ms │          8523.32ms │     no change │
│ QQuery 24    │   487.74ms │           486.22ms │     no change │
│ QQuery 25    │   412.34ms │           408.17ms │     no change │
│ QQuery 26    │   543.10ms │           550.15ms │     no change │
│ QQuery 27    │  1702.50ms │          1729.68ms │     no change │
│ QQuery 28    │ 12489.17ms │         12636.66ms │     no change │
│ QQuery 29    │   541.01ms │           535.21ms │     no change │
│ QQuery 30    │   829.92ms │           821.21ms │     no change │
│ QQuery 31    │   870.53ms │           883.44ms │     no change │
│ QQuery 32    │  2658.55ms │          2675.61ms │     no change │
│ QQuery 33    │  3386.29ms │          3381.19ms │     no change │
│ QQuery 34    │  3385.05ms │          3376.41ms │     no change │
│ QQuery 35    │  1283.08ms │          1258.65ms │     no change │
│ QQuery 36    │   129.46ms │           132.73ms │     no change │
│ QQuery 37    │    59.66ms │            58.32ms │     no change │
│ QQuery 38    │   127.07ms │           126.98ms │     no change │
│ QQuery 39    │   206.89ms │           202.78ms │     no change │
│ QQuery 40    │    52.69ms │            50.45ms │     no change │
│ QQuery 41    │    45.41ms │            47.94ms │  1.06x slower │
│ QQuery 42    │    40.69ms │            40.16ms │     no change │
└──────────────┴────────────┴────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary                 ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)                 │ 58376.12ms │
│ Total Time (fix_hash-join-seed)   │ 58049.37ms │
│ Average Time (HEAD)               │  1357.58ms │
│ Average Time (fix_hash-join-seed) │  1349.99ms │
│ Queries Faster                    │          2 │
│ Queries Slower                    │          1 │
│ Queries with No Change            │         40 │
└───────────────────────────────────┴────────────┘
--------------------
Benchmark tpch_mem_sf1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Query        ┃     HEAD ┃ fix_hash-join-seed ┃    Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ QQuery 1     │ 122.29ms │           124.02ms │ no change │
│ QQuery 2     │  24.38ms │            24.21ms │ no change │
│ QQuery 3     │  34.54ms │            35.67ms │ no change │
│ QQuery 4     │  20.92ms │            20.74ms │ no change │
│ QQuery 5     │  56.17ms │            56.45ms │ no change │
│ QQuery 6     │  12.35ms │            12.35ms │ no change │
│ QQuery 7     │ 104.86ms │           104.53ms │ no change │
│ QQuery 8     │  26.34ms │            26.19ms │ no change │
│ QQuery 9     │  61.22ms │            63.62ms │ no change │
│ QQuery 10    │  56.72ms │            57.69ms │ no change │
│ QQuery 11    │  13.01ms │            13.17ms │ no change │
│ QQuery 12    │  45.48ms │            46.51ms │ no change │
│ QQuery 13    │  29.20ms │            30.13ms │ no change │
│ QQuery 14    │  10.25ms │             9.85ms │ no change │
│ QQuery 15    │  25.11ms │            25.33ms │ no change │
│ QQuery 16    │  23.20ms │            23.88ms │ no change │
│ QQuery 17    │  95.84ms │            99.73ms │ no change │
│ QQuery 18    │ 231.30ms │           239.99ms │ no change │
│ QQuery 19    │  26.85ms │            27.91ms │ no change │
│ QQuery 20    │  40.82ms │            39.10ms │ no change │
│ QQuery 21    │ 166.73ms │           172.85ms │ no change │
│ QQuery 22    │  16.81ms │            17.12ms │ no change │
└──────────────┴──────────┴────────────────────┴───────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary                 ┃           ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD)                 │ 1244.41ms │
│ Total Time (fix_hash-join-seed)   │ 1271.05ms │
│ Average Time (HEAD)               │   56.56ms │
│ Average Time (fix_hash-join-seed) │   57.77ms │
│ Queries Faster                    │         0 │
│ Queries Slower                    │         0 │
│ Queries with No Change            │        22 │
└───────────────────────────────────┴───────────┘

alamb · 2025-04-29T02:28:26Z

│ QQuery 4 │ 887.43ms │ 740.13ms │ +1.20x faster │

Given these queries don't have joins I am not sure that is reproduceable 😬

alamb · 2025-04-29T12:37:42Z

Thanks again @ctsk -- sorry for the delay in review / merge

* Set HashJoin seed * fmt * whitespace grr * Document hash seed Co-authored-by: Alex Huang <[email protected]> --------- Co-authored-by: Alex Huang <[email protected]>

ctsk added 2 commits April 20, 2025 18:44

Set HashJoin seed

49491fa

fmt

231629a

ctsk force-pushed the fix/hash-join-seed branch from 6a8379b to 231629a Compare April 20, 2025 16:46

whitespace grr

af232c6

Weijun-H reviewed Apr 21, 2025

View reviewed changes

datafusion/physical-plan/src/joins/hash_join.rs Show resolved Hide resolved

Document hash seed

e1fc96d

Co-authored-by: Alex Huang <[email protected]>

m09526 approved these changes Apr 25, 2025

View reviewed changes

alamb reviewed Apr 29, 2025

View reviewed changes

alamb approved these changes Apr 29, 2025

View reviewed changes

alamb merged commit e1ad767 into apache:main Apr 29, 2025
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set HashJoin seed #15783

Set HashJoin seed #15783

ctsk commented Apr 20, 2025

alamb commented Apr 29, 2025

alamb Apr 29, 2025

alamb left a comment

alamb commented Apr 29, 2025

alamb commented Apr 29, 2025

alamb commented Apr 29, 2025

Set HashJoin seed #15783

Set HashJoin seed #15783

Conversation

ctsk commented Apr 20, 2025

Which issue does this PR close?

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

alamb commented Apr 29, 2025

alamb Apr 29, 2025

Choose a reason for hiding this comment

alamb left a comment

Choose a reason for hiding this comment

alamb commented Apr 29, 2025

alamb commented Apr 29, 2025

alamb commented Apr 29, 2025