Fix HashBuild unspilling stuck #8715
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for meta-velox canceled.
|
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D53589502 |
|
@Yuhta : Thanks for this fix. We noticed this slowdown internally as well. Will retry. Appreciate it. |
Yuhta
force-pushed
the
export-D53589502
branch
from
4ac6c10
to
e335ed3
Compare
Yuhta
added a commit
to Yuhta/velox
that referenced
this pull request
Feb 9, 2024
Summary: When the input of `HashBuild` is from spilling, they all come from the same partition. That means the spill partition bits are all same for the hashes from these rows. In case the hash table is large, there could be overlap between the hash bits we use to calculate bucket index and the bits for spill partitioning. These bits are fixed for all rows and because they are higher bits, we end up restricting ourselves to a smaller region of the hash table. This results in heavy hash collision and the hash build will take super long time and block driver threads. Fix this by adding a check to make sure that there will be no overlap between the spill partitioning bits and the bits used for bucket indexing, and increase the default spill start partition bit to 48. Reviewed By: oerling Differential Revision: D53589502
Summary: When the input of `HashBuild` is from spilling, they all come from the same partition. That means the spill partition bits are all same for the hashes from these rows. In case the hash table is large, there could be overlap between the hash bits we use to calculate bucket index and the bits for spill partitioning. These bits are fixed for all rows and because they are higher bits, we end up restricting ourselves to a smaller region of the hash table. This results in heavy hash collision and the hash build will take super long time and block driver threads. Fix this by adding a check to make sure that there will be no overlap between the spill partitioning bits and the bits used for bucket indexing, and increase the default spill start partition bit to 48. Reviewed By: oerling Differential Revision: D53589502
|
This pull request was exported from Phabricator. Differential Revision: D53589502 |
Yuhta
force-pushed
the
export-D53589502
branch
from
e335ed3
to
314f24c
Compare
|
This pull request was exported from Phabricator. Differential Revision: D53589502 |
|
This pull request has been merged in 9cf0ef0. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
FelixYBW
pushed a commit
to FelixYBW/velox
that referenced
this pull request
Feb 12, 2024
Summary: Pull Request resolved: facebookincubator#8715 When the input of `HashBuild` is from spilling, they all come from the same partition. That means the spill partition bits are all same for the hashes from these rows. In case the hash table is large, there could be overlap between the hash bits we use to calculate bucket index and the bits for spill partitioning. These bits are fixed for all rows and because they are higher bits, we end up restricting ourselves to a smaller region of the hash table. This results in heavy hash collision and the hash build will take super long time and block driver threads. Fix this by adding a check to make sure that there will be no overlap between the spill partitioning bits and the bits used for bucket indexing, and increase the default spill start partition bit to 48. Reviewed By: oerling Differential Revision: D53589502 fbshipit-source-id: 969fe24f09a04ea3abaa4ff750de4541e438d988
FelixYBW
pushed a commit
to FelixYBW/velox
that referenced
this pull request
Feb 12, 2024
Summary: Pull Request resolved: facebookincubator#8715 When the input of `HashBuild` is from spilling, they all come from the same partition. That means the spill partition bits are all same for the hashes from these rows. In case the hash table is large, there could be overlap between the hash bits we use to calculate bucket index and the bits for spill partitioning. These bits are fixed for all rows and because they are higher bits, we end up restricting ourselves to a smaller region of the hash table. This results in heavy hash collision and the hash build will take super long time and block driver threads. Fix this by adding a check to make sure that there will be no overlap between the spill partitioning bits and the bits used for bucket indexing, and increase the default spill start partition bit to 48. Reviewed By: oerling Differential Revision: D53589502 fbshipit-source-id: 969fe24f09a04ea3abaa4ff750de4541e438d988
FelixYBW
pushed a commit
to FelixYBW/velox
that referenced
this pull request
Feb 12, 2024
Summary: Pull Request resolved: facebookincubator#8715 When the input of `HashBuild` is from spilling, they all come from the same partition. That means the spill partition bits are all same for the hashes from these rows. In case the hash table is large, there could be overlap between the hash bits we use to calculate bucket index and the bits for spill partitioning. These bits are fixed for all rows and because they are higher bits, we end up restricting ourselves to a smaller region of the hash table. This results in heavy hash collision and the hash build will take super long time and block driver threads. Fix this by adding a check to make sure that there will be no overlap between the spill partitioning bits and the bits used for bucket indexing, and increase the default spill start partition bit to 48. Reviewed By: oerling Differential Revision: D53589502 fbshipit-source-id: 969fe24f09a04ea3abaa4ff750de4541e438d988
facebook-github-bot
pushed a commit
that referenced
this pull request
May 7, 2024
Summary: #8715 made a modification to the hash tag in the hash value. The hash tag was changed from (32,38] to (38,44]. We should also update the documentation of the hash table to reflect this change. Pull Request resolved: #9699 Reviewed By: xiaoxmeng Differential Revision: D57072452 Pulled By: bikramSingh91 fbshipit-source-id: 4048ee5e536ca90d7145efad59f85cc5c33d1d0f
Joe-Abraham
pushed a commit
to Joe-Abraham/velox
that referenced
this pull request
May 8, 2024
…9699) Summary: facebookincubator#8715 made a modification to the hash tag in the hash value. The hash tag was changed from (32,38] to (38,44]. We should also update the documentation of the hash table to reflect this change. Pull Request resolved: facebookincubator#9699 Reviewed By: xiaoxmeng Differential Revision: D57072452 Pulled By: bikramSingh91 fbshipit-source-id: 4048ee5e536ca90d7145efad59f85cc5c33d1d0f
Joe-Abraham
pushed a commit
to Joe-Abraham/velox
that referenced
this pull request
Jun 7, 2024
…9699) Summary: facebookincubator#8715 made a modification to the hash tag in the hash value. The hash tag was changed from (32,38] to (38,44]. We should also update the documentation of the hash table to reflect this change. Pull Request resolved: facebookincubator#9699 Reviewed By: xiaoxmeng Differential Revision: D57072452 Pulled By: bikramSingh91 fbshipit-source-id: 4048ee5e536ca90d7145efad59f85cc5c33d1d0f
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
When the input of
HashBuildis from spilling, they all come from thesame partition. That means the spill partition bits are all same for the hashes
from these rows. In case the hash table is large, there could be overlap between the hash bits
we use to calculate bucket index and the bits for spill partitioning. These
bits are fixed for all rows and because they are higher bits, we end up
restricting ourselves to a smaller region of the hash table. This results in
heavy hash collision and the hash build will take super long time and block
driver threads.
Fix this by adding a check to make sure that there will be no overlap between
the spill partitioning bits and the bits used for bucket indexing, and increase
the default spill start partition bit to 48.
Differential Revision: D53589502