Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

distsqlrun: top k disk sorter never discards rows #31901

Closed
jordanlewis opened this issue Oct 25, 2018 · 2 comments
Closed

distsqlrun: top k disk sorter never discards rows #31901

jordanlewis opened this issue Oct 25, 2018 · 2 comments
Assignees
Labels
A-sql-execution Relating to SQL execution. C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Milestone

Comments

@jordanlewis
Copy link
Member

The disk implementation of the top k sorter's MaybeReplaceMax method, which is supposed to sort the row into the top k rows and keep the number of saved rows at k, is currently implemented as just adding the row to the sorted disk map of rows.

This is quite inefficient - I think in this case it's just falling back to global sort.

I don't think it's a huge deal - how often do we really spill top-k sorts to disk? - but this is probably something that should be fixed to prevent really bad edge case performance.

@jordanlewis jordanlewis added C-performance Perf of queries or internals. Solution not expected to change functional behavior. A-sql-execution Relating to SQL execution. labels Oct 25, 2018
@jordanlewis jordanlewis added this to the Later milestone Oct 25, 2018
@jordanlewis jordanlewis added this to Triage in (DEPRECATED) SQL execution via automation Oct 25, 2018
@jordanlewis
Copy link
Member Author

cc @asubiotto

@jordanlewis jordanlewis moved this from Triage to Lower Priority Backlog in (DEPRECATED) SQL execution Feb 6, 2019
@jordanlewis jordanlewis moved this from Triage to Lower priority backlog in [DEPRECATED] Old SQLExec board. Don't move stuff here May 15, 2019
@asubiotto asubiotto moved this from Lower priority backlog to [TENT] SQL Exec in [DEPRECATED] Old SQLExec board. Don't move stuff here Apr 2, 2020
@yuzefovich
Copy link
Member

Closing in favor of #45192

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-execution Relating to SQL execution. C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Projects
No open projects
Development

No branches or pull requests

3 participants