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

Use explicit memory context with hash_create #4534

Merged
merged 1 commit into from
Jul 21, 2022

Conversation

svenklemm
Copy link
Member

This changes the hash_create calls in the function cache and in the
cagg code to use explicit memory context. Without this hash_create
will create the hash table in TopMemoryContext potentially leading
to memory leaks.

@svenklemm svenklemm requested a review from a team as a code owner July 20, 2022 21:17
@svenklemm svenklemm requested review from pmwkaa and konskov and removed request for a team July 20, 2022 21:17
@svenklemm svenklemm added this to the TimescaleDB 2.7.2 milestone Jul 20, 2022
@svenklemm svenklemm self-assigned this Jul 20, 2022
@codecov
Copy link

codecov bot commented Jul 20, 2022

Codecov Report

Merging #4534 (41d4fe7) into main (63a80ee) will decrease coverage by 0.03%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4534      +/-   ##
==========================================
- Coverage   90.23%   90.19%   -0.04%     
==========================================
  Files         222      222              
  Lines       41089    41088       -1     
==========================================
- Hits        37076    37059      -17     
- Misses       4013     4029      +16     
Impacted Files Coverage Δ
src/telemetry/functions.c 4.87% <0.00%> (+0.03%) ⬆️
tsl/src/continuous_aggs/insert.c 90.00% <ø> (ø)
src/func_cache.c 90.42% <100.00%> (ø)
tsl/src/chunk_api.c 94.91% <100.00%> (+<0.01%) ⬆️
src/bgw/scheduler.c 82.89% <0.00%> (-3.54%) ⬇️
tsl/src/remote/dist_commands.c 92.95% <0.00%> (-0.94%) ⬇️
src/bgw/job.c 93.14% <0.00%> (-0.29%) ⬇️
tsl/src/reorder.c 85.37% <0.00%> (-0.27%) ⬇️
tsl/src/nodes/data_node_dispatch.c 96.49% <0.00%> (-0.24%) ⬇️
tsl/src/bgw_policy/job.c 88.62% <0.00%> (-0.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ed116b...41d4fe7. Read the comment docs.

Copy link
Contributor

@jnidzwetzki jnidzwetzki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, LGTM. Two comments on the PR:

  • In my opinion, this PR (along with Fix memory leak in function telemetry #4532) should be mentioned in the changelog as a bug fix.

  • I was wondering if it would be possible to write a coccinelle test that automatically detects the missing memory contexts and HASH_CONTEXT flags?

@svenklemm
Copy link
Member Author

In general, LGTM. Two comments on the PR:

  • In my opinion, this PR (along with Fix memory leak in function telemetry #4532) should be mentioned in the changelog as a bug fix.
  • I was wondering if it would be possible to write a coccinelle test that automatically detects the missing memory contexts and HASH_CONTEXT flags?

They should definitely be in changelog, i'll add them in release PR #4535.
coccinelle script is outside of scope of this PR but would be useful to have

This changes the hash_create calls in the function cache and in the
cagg code to use explicit memory context. Without this hash_create
will create the hash table in TopMemoryContext potentially leading
to memory leaks.
@svenklemm svenklemm merged commit 7c841b8 into timescale:main Jul 21, 2022
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Jul 22, 2022
This release is a patch release. We recommend that you upgrade at the
next available opportunity.
Among other things this release fixes several memory leaks, handling
of TOASTed values in GapFill and parameter handling in prepared statements.

**Bugfixes**

* timescale#4517 Fix prepared statement param handling in ChunkAppend
* timescale#4522 Fix ANALYZE on dist hypertable for a set of nodes
* timescale#4526 Fix gapfill group comparison for TOASTed values
* timescale#4527 Handle stats properly for range types
* timescale#4532 Fix memory leak in function telemetry
* timescale#4534 Use explicit memory context with hash_create
* timescale#4538 Fix chunk creation on hypertables with non-default statistics

**Thanks**

* @3a6u9ka, @bgemmill , @hongquani, @stl-leonid-kalmaev and @victor-sudakov for reporting a memory leak
* @hleung2021 and @laocaixw  for reporting an issue with parameter handling in prepared statements
@svenklemm svenklemm mentioned this pull request Jul 22, 2022
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Jul 22, 2022
This release is a patch release. We recommend that you upgrade at the
next available opportunity.
Among other things this release fixes several memory leaks, handling
of TOASTed values in GapFill and parameter handling in prepared statements.

**Bugfixes**

* timescale#4517 Fix prepared statement param handling in ChunkAppend
* timescale#4522 Fix ANALYZE on dist hypertable for a set of nodes
* timescale#4526 Fix gapfill group comparison for TOASTed values
* timescale#4527 Handle stats properly for range types
* timescale#4532 Fix memory leak in function telemetry
* timescale#4534 Use explicit memory context with hash_create
* timescale#4538 Fix chunk creation on hypertables with non-default statistics

**Thanks**

* @3a6u9ka, @bgemmill , @hongquani, @stl-leonid-kalmaev and @victor-sudakov for reporting a memory leak
* @hleung2021 and @laocaixw  for reporting an issue with parameter handling in prepared statements
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Jul 22, 2022
This release is a patch release. We recommend that you upgrade at the
next available opportunity.
Among other things this release fixes several memory leaks, handling
of TOASTed values in GapFill and parameter handling in prepared statements.

**Bugfixes**

* timescale#4517 Fix prepared statement param handling in ChunkAppend
* timescale#4522 Fix ANALYZE on dist hypertable for a set of nodes
* timescale#4526 Fix gapfill group comparison for TOASTed values
* timescale#4527 Handle stats properly for range types
* timescale#4532 Fix memory leak in function telemetry
* timescale#4534 Use explicit memory context with hash_create
* timescale#4538 Fix chunk creation on hypertables with non-default statistics

**Thanks**

* @3a6u9ka, @bgemmill, @hongquan, @stl-leonid-kalmaev and @victor-sudakov for reporting a memory leak
* @hleung2021 and @laocaixw  for reporting an issue with parameter handling in prepared statements
svenklemm added a commit that referenced this pull request Jul 22, 2022
This release is a patch release. We recommend that you upgrade at the
next available opportunity.
Among other things this release fixes several memory leaks, handling
of TOASTed values in GapFill and parameter handling in prepared statements.

**Bugfixes**

* #4517 Fix prepared statement param handling in ChunkAppend
* #4522 Fix ANALYZE on dist hypertable for a set of nodes
* #4526 Fix gapfill group comparison for TOASTed values
* #4527 Handle stats properly for range types
* #4532 Fix memory leak in function telemetry
* #4534 Use explicit memory context with hash_create
* #4538 Fix chunk creation on hypertables with non-default statistics

**Thanks**

* @3a6u9ka, @bgemmill, @hongquan, @stl-leonid-kalmaev and @victor-sudakov for reporting a memory leak
* @hleung2021 and @laocaixw  for reporting an issue with parameter handling in prepared statements
svenklemm added a commit that referenced this pull request Jul 22, 2022
This release is a patch release. We recommend that you upgrade at the
next available opportunity.
Among other things this release fixes several memory leaks, handling
of TOASTed values in GapFill and parameter handling in prepared statements.

**Bugfixes**

* #4517 Fix prepared statement param handling in ChunkAppend
* #4522 Fix ANALYZE on dist hypertable for a set of nodes
* #4526 Fix gapfill group comparison for TOASTed values
* #4527 Handle stats properly for range types
* #4532 Fix memory leak in function telemetry
* #4534 Use explicit memory context with hash_create
* #4538 Fix chunk creation on hypertables with non-default statistics

**Thanks**

* @3a6u9ka, @bgemmill, @hongquan, @stl-leonid-kalmaev and @victor-sudakov for reporting a memory leak
* @hleung2021 and @laocaixw  for reporting an issue with parameter handling in prepared statements
svenklemm added a commit that referenced this pull request Jul 22, 2022
This release is a patch release. We recommend that you upgrade at the
next available opportunity.
Among other things this release fixes several memory leaks, handling
of TOASTed values in GapFill and parameter handling in prepared statements.

**Bugfixes**

* #4517 Fix prepared statement param handling in ChunkAppend
* #4522 Fix ANALYZE on dist hypertable for a set of nodes
* #4526 Fix gapfill group comparison for TOASTed values
* #4527 Handle stats properly for range types
* #4532 Fix memory leak in function telemetry
* #4534 Use explicit memory context with hash_create
* #4538 Fix chunk creation on hypertables with non-default statistics

**Thanks**

* @3a6u9ka, @bgemmill, @hongquan, @stl-leonid-kalmaev and @victor-sudakov for reporting a memory leak
* @hleung2021 and @laocaixw  for reporting an issue with parameter handling in prepared statements
svenklemm added a commit that referenced this pull request Jul 25, 2022
This release is a patch release. We recommend that you upgrade at the
next available opportunity.
Among other things this release fixes several memory leaks, handling
of TOASTed values in GapFill and parameter handling in prepared statements.

**Bugfixes**

* #4517 Fix prepared statement param handling in ChunkAppend
* #4522 Fix ANALYZE on dist hypertable for a set of nodes
* #4526 Fix gapfill group comparison for TOASTed values
* #4527 Handle stats properly for range types
* #4532 Fix memory leak in function telemetry
* #4534 Use explicit memory context with hash_create
* #4538 Fix chunk creation on hypertables with non-default statistics

**Thanks**

* @3a6u9ka, @bgemmill, @hongquan, @stl-leonid-kalmaev and @victor-sudakov for reporting a memory leak
* @hleung2021 and @laocaixw  for reporting an issue with parameter handling in prepared statements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants