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

Fix memory leak for compression with merge chunks #5063

Merged
merged 1 commit into from Dec 13, 2022

Conversation

cevian
Copy link
Contributor

@cevian cevian commented Dec 8, 2022

The RelationInitIndexAccessInfo call leaks cache memory and seems to be unnecessary.

@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Merging #5063 (1a39a94) into main (13ac2fc) will increase coverage by 27.22%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #5063       +/-   ##
===========================================
+ Coverage   62.38%   89.60%   +27.22%     
===========================================
  Files         227      227               
  Lines       45501    51616     +6115     
===========================================
+ Hits        28385    46252    +17867     
+ Misses      17116     5364    -11752     
Impacted Files Coverage Δ
tsl/src/compression/compression.c 96.32% <ø> (+0.51%) ⬆️
tsl/test/src/test_chunk_stats.c 92.59% <0.00%> (-7.41%) ⬇️
src/uuid.c 78.57% <0.00%> (-6.05%) ⬇️
src/histogram.c 83.83% <0.00%> (-4.80%) ⬇️
tsl/src/fdw/fdw_utils.c 80.76% <0.00%> (-4.65%) ⬇️
src/time_utils.c 93.72% <0.00%> (-3.78%) ⬇️
src/time_bucket.c 95.81% <0.00%> (-2.76%) ⬇️
src/hypertable_cache.c 93.25% <0.00%> (-2.64%) ⬇️
tsl/src/dist_backup.c 92.30% <0.00%> (-2.52%) ⬇️
src/loader/bgw_message_queue.c 86.36% <0.00%> (-2.46%) ⬇️
... and 203 more

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 13ac2fc...1a39a94. Read the comment docs.

Copy link
Member

@akuzm akuzm left a comment

Choose a reason for hiding this comment

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

Yeah, not sure why we're calling it directly, it's already called by relation_open -> RelationIdGetRelation -> RelationBuildDesc.

@jnidzwetzki jnidzwetzki added this to the TimescaleDB 2.9 milestone Dec 8, 2022
Copy link
Contributor

@antekresic antekresic left a comment

Choose a reason for hiding this comment

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

Yeah, this was my bad, was trying to fill it manually because of some other issue I was dealing with and it got left in when I fixed that issue.

@SachinSetiya
Copy link
Contributor

SachinSetiya commented Dec 8, 2022

I rebased on top of the latest main (So that we can see sanitizer test results)

The RelationInitIndexAccessInfo call leaks cache memory and
seems to be unnecessary.
@jnidzwetzki jnidzwetzki enabled auto-merge (rebase) December 13, 2022 07:00
@jnidzwetzki jnidzwetzki merged commit df16815 into timescale:main Dec 13, 2022
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.

None yet

5 participants