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

sql: make tests in builtin_mem_usage_test.go less flaky #79014

Closed
yuzefovich opened this issue Mar 30, 2022 · 0 comments · Fixed by #100550
Closed

sql: make tests in builtin_mem_usage_test.go less flaky #79014

yuzefovich opened this issue Mar 30, 2022 · 0 comments · Fixed by #100550
Assignees
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team

Comments

@yuzefovich
Copy link
Member

yuzefovich commented Mar 30, 2022

We probably want to re-evaluate a couple of tests in sql/builtin_mem_usage_test.go to make them less flaky and to make sure that they test what they intend to.

Currently, TestAggregatesMonitorMemory doesn't seem like a very useful test to me anymore because we now do memory accounting when scanning the table "with long strings", so it's likely that OOM error occurs not during the aggregation. This probably needs to be rewritten or removed.

TestEvaluatedMemoryIsChecked seems still useful since there the only memory consumer is the builtin, so we do want to make sure that builtins that might use a lot of memory do proper memory accounting. However, lowMemoryBudget value is needed to be bumped periodically due to the internal queries using more and more memory (or memory accounting being more precise).

Jira issue: CRDB-14499

@yuzefovich yuzefovich added C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. E-quick-win Likely to be a quick win for someone experienced. labels Mar 30, 2022
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Mar 30, 2022
@yuzefovich yuzefovich self-assigned this Apr 4, 2023
@craig craig bot closed this as completed in 0d15e0d Apr 4, 2023
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant