Skip to content

More accurate memory accounting in external sort #14748

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

Open
Tracked by #16065
2010YOUY01 opened this issue Feb 18, 2025 · 4 comments
Open
Tracked by #16065

More accurate memory accounting in external sort #14748

2010YOUY01 opened this issue Feb 18, 2025 · 4 comments
Labels
enhancement New feature or request

Comments

@2010YOUY01
Copy link
Contributor

Is your feature request related to a problem or challenge?

#14644 fixed an external sorting bug. Each batch's memory overhead is estimated as 2 * batch memory size for the extra row conversion overhead. It works for common cases but still can fail. See #14644 (comment) and #14644 (comment).

To implement a more accurate memory size estimation for those edge cases, we can first do col->row conversion and measure the memory consumption. (depends on #7053)

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

@2010YOUY01
Copy link
Contributor Author

There is a small optimization can be done after we have accurate memory accounting #15017 (comment)

@alamb
Copy link
Contributor

alamb commented Mar 12, 2025

I wonder if this is ready to do now?

@2010YOUY01
Copy link
Contributor Author

I wonder if this is ready to do now?

Yes, I think so. However, this might cause a temporary slowdown due to an extra row <-> col conversion. Since the performance can be brought back by reusing converted rows between operators, and also I believe correctness is more important, this is not a hard blocker.
I plan to work on this ticket in the future, but if anyone is available to take it now, please feel free.

@rluvaton
Copy link
Contributor

This PR actually fix memory accounting: #15700

Just need to answer the comments and add tests to prove that the memory accounting is consistent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants